Merge "Manually manage memory of native objects" into tm-qpr-dev am: 2971d9f3e0

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20554503

Change-Id: Ib65f7f146134f70e4e84efa0816d62c54c100ad0
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

core/java/android/window/TransitionInfo.java

@@ -414,6 +414,53 @@ public final class TransitionInfo implements Parcelable {
         return false;
     }
 
+    /**
+     * Releases temporary-for-animation surfaces referenced by this to potentially free up memory.
+     * This includes root-leash and snapshots.
+     */
+    public void releaseAnimSurfaces() {
+        for (int i = mChanges.size() - 1; i >= 0; --i) {
+            final Change c = mChanges.get(i);
+            if (c.mSnapshot != null) {
+                c.mSnapshot.release();
+                c.mSnapshot = null;
+            }
+        }
+        if (mRootLeash != null) {
+            mRootLeash.release();
+        }
+    }
+
+    /**
+     * Releases ALL the surfaces referenced by this to potentially free up memory. Do NOT use this
+     * if the surface-controls get stored and used elsewhere in the process. To just release
+     * temporary-for-animation surfaces, use {@link #releaseAnimSurfaces}.
+     */
+    public void releaseAllSurfaces() {
+        releaseAnimSurfaces();
+        for (int i = mChanges.size() - 1; i >= 0; --i) {
+            mChanges.get(i).getLeash().release();
+        }
+    }
+
+    /**
+     * Makes a copy of this as if it were parcel'd and unparcel'd. This implies that surfacecontrol
+     * refcounts are incremented which allows the "remote" receiver to release them without breaking
+     * the caller's references. Use this only if you need to "send" this to a local function which
+     * assumes it is being called from a remote caller.
+     */
+    public TransitionInfo localRemoteCopy() {
+        final TransitionInfo out = new TransitionInfo(mType, mFlags);
+        for (int i = 0; i < mChanges.size(); ++i) {
+            out.mChanges.add(mChanges.get(i).localRemoteCopy());
+        }
+        out.mRootLeash = mRootLeash != null ? new SurfaceControl(mRootLeash, "localRemote") : null;
+        // Doesn't have any native stuff, so no need for actual copy
+        out.mOptions = mOptions;
+        out.mRootOffset.set(mRootOffset);
+        return out;
+    }
+
     /** Represents the change a WindowContainer undergoes during a transition */
     public static final class Change implements Parcelable {
         private final WindowContainerToken mContainer;
@@ -466,6 +513,27 @@ public final class TransitionInfo implements Parcelable {
             mSnapshotLuma = in.readFloat();
         }
 
+        private Change localRemoteCopy() {
+            final Change out = new Change(mContainer, new SurfaceControl(mLeash, "localRemote"));
+            out.mParent = mParent;
+            out.mLastParent = mLastParent;
+            out.mMode = mMode;
+            out.mFlags = mFlags;
+            out.mStartAbsBounds.set(mStartAbsBounds);
+            out.mEndAbsBounds.set(mEndAbsBounds);
+            out.mEndRelOffset.set(mEndRelOffset);
+            out.mTaskInfo = mTaskInfo;
+            out.mAllowEnterPip = mAllowEnterPip;
+            out.mStartRotation = mStartRotation;
+            out.mEndRotation = mEndRotation;
+            out.mEndFixedRotation = mEndFixedRotation;
+            out.mRotationAnimation = mRotationAnimation;
+            out.mBackgroundColor = mBackgroundColor;
+            out.mSnapshot = mSnapshot != null ? new SurfaceControl(mSnapshot, "localRemote") : null;
+            out.mSnapshotLuma = mSnapshotLuma;
+            return out;
+        }
+
         /** Sets the parent of this change's container. The parent must be a participant or null. */
         public void setParent(@Nullable WindowContainerToken parent) {
             mParent = parent;

libs/WindowManager/Shell/src/com/android/wm/shell/transition/OneShotRemoteHandler.java

@@ -77,10 +77,10 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler {
                 if (mRemote.asBinder() != null) {
                     mRemote.asBinder().unlinkToDeath(remoteDied, 0 /* flags */);
                 }
+                if (sct != null) {
+                    finishTransaction.merge(sct);
+                }
                 mMainExecutor.execute(() -> {
-                    if (sct != null) {
-                        finishTransaction.merge(sct);
-                    }
                     finishCallback.onTransitionFinished(wct, null /* wctCB */);
                 });
             }
@@ -90,7 +90,13 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler {
             if (mRemote.asBinder() != null) {
                 mRemote.asBinder().linkToDeath(remoteDied, 0 /* flags */);
             }
-            mRemote.getRemoteTransition().startAnimation(transition, info, startTransaction, cb);
+            // If the remote is actually in the same process, then make a copy of parameters since
+            // remote impls assume that they have to clean-up native references.
+            final SurfaceControl.Transaction remoteStartT = RemoteTransitionHandler.copyIfLocal(
+                    startTransaction, mRemote.getRemoteTransition());
+            final TransitionInfo remoteInfo =
+                    remoteStartT == startTransaction ? info : info.localRemoteCopy();
+            mRemote.getRemoteTransition().startAnimation(transition, remoteInfo, remoteStartT, cb);
             // assume that remote will apply the start transaction.
             startTransaction.clear();
         } catch (RemoteException e) {
@@ -124,7 +130,13 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler {
             }
         };
         try {
-            mRemote.getRemoteTransition().mergeAnimation(transition, info, t, mergeTarget, cb);
+            // If the remote is actually in the same process, then make a copy of parameters since
+            // remote impls assume that they have to clean-up native references.
+            final SurfaceControl.Transaction remoteT =
+                    RemoteTransitionHandler.copyIfLocal(t, mRemote.getRemoteTransition());
+            final TransitionInfo remoteInfo = remoteT == t ? info : info.localRemoteCopy();
+            mRemote.getRemoteTransition().mergeAnimation(
+                    transition, remoteInfo, remoteT, mergeTarget, cb);
         } catch (RemoteException e) {
             Log.e(Transitions.TAG, "Error merging remote transition.", e);
         }

libs/WindowManager/Shell/src/com/android/wm/shell/transition/RemoteTransitionHandler.java

@@ -19,6 +19,7 @@ package com.android.wm.shell.transition;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.os.IBinder;
+import android.os.Parcel;
 import android.os.RemoteException;
 import android.util.ArrayMap;
 import android.util.Log;
@@ -120,10 +121,10 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler {
             public void onTransitionFinished(WindowContainerTransaction wct,
                     SurfaceControl.Transaction sct) {
                 unhandleDeath(remote.asBinder(), finishCallback);
+                if (sct != null) {
+                    finishTransaction.merge(sct);
+                }
                 mMainExecutor.execute(() -> {
-                    if (sct != null) {
-                        finishTransaction.merge(sct);
-                    }
                     mRequestedRemotes.remove(transition);
                     finishCallback.onTransitionFinished(wct, null /* wctCB */);
                 });
@@ -131,8 +132,14 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler {
         };
         Transitions.setRunningRemoteTransitionDelegate(remote.getAppThread());
         try {
+            // If the remote is actually in the same process, then make a copy of parameters since
+            // remote impls assume that they have to clean-up native references.
+            final SurfaceControl.Transaction remoteStartT =
+                    copyIfLocal(startTransaction, remote.getRemoteTransition());
+            final TransitionInfo remoteInfo =
+                    remoteStartT == startTransaction ? info : info.localRemoteCopy();
             handleDeath(remote.asBinder(), finishCallback);
-            remote.getRemoteTransition().startAnimation(transition, info, startTransaction, cb);
+            remote.getRemoteTransition().startAnimation(transition, remoteInfo, remoteStartT, cb);
             // assume that remote will apply the start transaction.
             startTransaction.clear();
         } catch (RemoteException e) {
@@ -145,6 +152,28 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler {
         return true;
     }
 
+    static SurfaceControl.Transaction copyIfLocal(SurfaceControl.Transaction t,
+            IRemoteTransition remote) {
+        // We care more about parceling than local (though they should be the same); so, use
+        // queryLocalInterface since that's what Binder uses to decide if it needs to parcel.
+        if (remote.asBinder().queryLocalInterface(IRemoteTransition.DESCRIPTOR) == null) {
+            // No local interface, so binder itself will parcel and thus we don't need to.
+            return t;
+        }
+        // Binder won't be parceling; however, the remotes assume they have their own native
+        // objects (and don't know if caller is local or not), so we need to make a COPY here so
+        // that the remote can clean it up without clearing the original transaction.
+        // Since there's no direct `copy` for Transaction, we have to parcel/unparcel instead.
+        final Parcel p = Parcel.obtain();
+        try {
+            t.writeToParcel(p, 0);
+            p.setDataPosition(0);
+            return SurfaceControl.Transaction.CREATOR.createFromParcel(p);
+        } finally {
+            p.recycle();
+        }
+    }
+
     @Override
     public void mergeAnimation(@NonNull IBinder transition, @NonNull TransitionInfo info,
             @NonNull SurfaceControl.Transaction t, @NonNull IBinder mergeTarget,
@@ -175,7 +204,11 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler {
             }
         };
         try {
-            remote.mergeAnimation(transition, info, t, mergeTarget, cb);
+            // If the remote is actually in the same process, then make a copy of parameters since
+            // remote impls assume that they have to clean-up native references.
+            final SurfaceControl.Transaction remoteT = copyIfLocal(t, remote);
+            final TransitionInfo remoteInfo = remoteT == t ? info : info.localRemoteCopy();
+            remote.mergeAnimation(transition, remoteInfo, remoteT, mergeTarget, cb);
         } catch (RemoteException e) {
             Log.e(Transitions.TAG, "Error attempting to merge remote transition.", e);
         }

libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java

@@ -500,6 +500,7 @@ public class Transitions implements RemoteCallable<Transitions> {
             // Treat this as an abort since we are bypassing any merge logic and effectively
             // finishing immediately.
             onAbort(transitionToken);
+            releaseSurfaces(info);
             return;
         }
 
@@ -604,6 +605,15 @@ public class Transitions implements RemoteCallable<Transitions> {
         onFinish(transition, wct, wctCB, false /* abort */);
     }
 
+    /**
+     * Releases an info's animation-surfaces. These don't need to persist and we need to release
+     * them asap so that SF can free memory sooner.
+     */
+    private void releaseSurfaces(@Nullable TransitionInfo info) {
+        if (info == null) return;
+        info.releaseAnimSurfaces();
+    }
+
     private void onFinish(IBinder transition,
             @Nullable WindowContainerTransaction wct,
             @Nullable WindowContainerTransactionCallback wctCB,
@@ -642,6 +652,11 @@ public class Transitions implements RemoteCallable<Transitions> {
         }
         ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS,
                 "Transition animation finished (abort=%b), notifying core %s", abort, transition);
+        if (active.mStartT != null) {
+            // Applied by now, so close immediately. Do not set to null yet, though, since nullness
+            // is used later to disambiguate malformed transitions.
+            active.mStartT.close();
+        }
         // Merge all relevant transactions together
         SurfaceControl.Transaction fullFinish = active.mFinishT;
         for (int iA = activeIdx + 1; iA < mActiveTransitions.size(); ++iA) {
@@ -661,12 +676,14 @@ public class Transitions implements RemoteCallable<Transitions> {
             fullFinish.apply();
         }
         // Now perform all the finishes.
+        releaseSurfaces(active.mInfo);
         mActiveTransitions.remove(activeIdx);
         mOrganizer.finishTransition(transition, wct, wctCB);
         while (activeIdx < mActiveTransitions.size()) {
             if (!mActiveTransitions.get(activeIdx).mMerged) break;
             ActiveTransition merged = mActiveTransitions.remove(activeIdx);
             mOrganizer.finishTransition(merged.mToken, null /* wct */, null /* wctCB */);
+            releaseSurfaces(merged.mInfo);
         }
         // sift through aborted transitions
         while (mActiveTransitions.size() > activeIdx
@@ -679,8 +696,9 @@ public class Transitions implements RemoteCallable<Transitions> {
             }
             mOrganizer.finishTransition(aborted.mToken, null /* wct */, null /* wctCB */);
             for (int i = 0; i < mObservers.size(); ++i) {
-                mObservers.get(i).onTransitionFinished(active.mToken, true);
+                mObservers.get(i).onTransitionFinished(aborted.mToken, true);
             }
+            releaseSurfaces(aborted.mInfo);
         }
         if (mActiveTransitions.size() <= activeIdx) {
             ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, "All active transition animations "

packages/SystemUI/animation/src/com/android/systemui/animation/RemoteTransitionAdapter.kt

@@ -320,9 +320,7 @@ class RemoteTransitionAdapter {
                                 counterWallpaper.cleanUp(finishTransaction)
                                 // Release surface references now. This is apparently to free GPU
                                 // memory while doing quick operations (eg. during CTS).
-                                for (i in info.changes.indices.reversed()) {
+                                info.releaseAllSurfaces()
-                                    info.changes[i].leash.release()
-                                }
                                 for (i in leashMap.size - 1 downTo 0) {
                                     leashMap.valueAt(i).release()
                                 }
@@ -331,6 +329,7 @@ class RemoteTransitionAdapter {
                                         null /* wct */,
                                         finishTransaction
                                     )
+                                    finishTransaction.close()
                                 } catch (e: RemoteException) {
                                     Log.e(
                                         "ActivityOptionsCompat",
@@ -364,6 +363,9 @@ class RemoteTransitionAdapter {
                 ) {
                     // TODO: hook up merge to recents onTaskAppeared if applicable. Until then,
                     //       ignore any incoming merges.
+                    // Clean up stuff though cuz GC takes too long for benchmark tests.
+                    t.close()
+                    info.releaseAllSurfaces()
                 }
             }
         }

packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationRunnerCompat.java

@@ -166,15 +166,14 @@ public abstract class RemoteAnimationRunnerCompat extends IRemoteAnimationRunner
                     counterLauncher.cleanUp(finishTransaction);
                     counterWallpaper.cleanUp(finishTransaction);
                     // Release surface references now. This is apparently to free GPU memory
-                    // while doing quick operations (eg. during CTS).
+                    // before GC would.
-                    for (int i = info.getChanges().size() - 1; i >= 0; --i) {
+                    info.releaseAllSurfaces();
-                        info.getChanges().get(i).getLeash().release();
-                    }
                     // Don't release here since launcher might still be using them. Instead
                     // let launcher release them (eg. via RemoteAnimationTargets)
                     leashMap.clear();
                     try {
                         finishCallback.onTransitionFinished(null /* wct */, finishTransaction);
+                        finishTransaction.close();
                     } catch (RemoteException e) {
                         Log.e("ActivityOptionsCompat", "Failed to call app controlled animation"
                                 + " finished callback", e);
@@ -203,10 +202,13 @@ public abstract class RemoteAnimationRunnerCompat extends IRemoteAnimationRunner
                 synchronized (mFinishRunnables) {
                     finishRunnable = mFinishRunnables.remove(mergeTarget);
                 }
+                // Since we're not actually animating, release native memory now
+                t.close();
+                info.releaseAllSurfaces();
                 if (finishRunnable == null) return;
                 onAnimationCancelled(false /* isKeyguardOccluded */);
                 finishRunnable.run();
             }
         };
     }
-}
+}

packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteTransitionCompat.java

@@ -126,15 +126,18 @@ public class RemoteTransitionCompat {
             public void mergeAnimation(IBinder transition, TransitionInfo info,
                     SurfaceControl.Transaction t, IBinder mergeTarget,
                     IRemoteTransitionFinishedCallback finishedCallback) {
-                if (!mergeTarget.equals(mToken)) return;
+                if (mergeTarget.equals(mToken) && mRecentsSession.merge(info, t, recents)) {
-                if (!mRecentsSession.merge(info, t, recents)) return;
+                    try {
-                try {
+                        finishedCallback.onTransitionFinished(null /* wct */, null /* sct */);
-                    finishedCallback.onTransitionFinished(null /* wct */, null /* sct */);
+                    } catch (RemoteException e) {
-                } catch (RemoteException e) {
+                        Log.e(TAG, "Error merging transition.", e);
-                    Log.e(TAG, "Error merging transition.", e);
+                    }
+                    // commit taskAppeared after merge transition finished.
+                    mRecentsSession.commitTasksAppearedIfNeeded(recents);
+                } else {
+                    t.close();
+                    info.releaseAllSurfaces();
                 }
-                // commit taskAppeared after merge transition finished.
-                mRecentsSession.commitTasksAppearedIfNeeded(recents);
             }
         };
         return new RemoteTransition(remote, appThread);
@@ -248,6 +251,8 @@ public class RemoteTransitionCompat {
                 }
                 // In this case, we are "returning" to an already running app, so just consume
                 // the merge and do nothing.
+                info.releaseAllSurfaces();
+                t.close();
                 return true;
             }
             final int layer = mInfo.getChanges().size() * 3;
@@ -264,6 +269,8 @@ public class RemoteTransitionCompat {
                 t.setLayer(targets[i].leash, layer);
             }
             t.apply();
+            // not using the incoming anim-only surfaces
+            info.releaseAnimSurfaces();
             mAppearedTargets = targets;
             return true;
         }
@@ -380,9 +387,7 @@ public class RemoteTransitionCompat {
             }
             // Only release the non-local created surface references. The animator is responsible
             // for releasing the leashes created by local.
-            for (int i = 0; i < mInfo.getChanges().size(); ++i) {
+            mInfo.releaseAllSurfaces();
-                mInfo.getChanges().get(i).getLeash().release();
-            }
             // Reset all members.
             mWrapped = null;
             mFinishCB = null;

packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java

@@ -249,6 +249,7 @@ public class KeyguardService extends Service {
                                 synchronized (mFinishCallbacks) {
                                     if (mFinishCallbacks.remove(transition) == null) return;
                                 }
+                                info.releaseAllSurfaces();
                                 Slog.d(TAG, "Finish IRemoteAnimationRunner.");
                                 finishCallback.onTransitionFinished(null /* wct */, null /* t */);
                             }
@@ -264,6 +265,8 @@ public class KeyguardService extends Service {
                     synchronized (mFinishCallbacks) {
                         origFinishCB = mFinishCallbacks.remove(transition);
                     }
+                    info.releaseAllSurfaces();
+                    t.close();
                     if (origFinishCB == null) {
                         // already finished (or not started yet), so do nothing.
                         return;
@@ -459,12 +462,15 @@ public class KeyguardService extends Service {
             t.apply();
             mBinder.setOccluded(true /* isOccluded */, true /* animate */);
             finishCallback.onTransitionFinished(null /* wct */, null /* wctCB */);
+            info.releaseAllSurfaces();
         }
 
         @Override
         public void mergeAnimation(IBinder transition, TransitionInfo info,
                 SurfaceControl.Transaction t, IBinder mergeTarget,
                 IRemoteTransitionFinishedCallback finishCallback) {
+            t.close();
+            info.releaseAllSurfaces();
         }
     };
 
@@ -476,12 +482,15 @@ public class KeyguardService extends Service {
             t.apply();
             mBinder.setOccluded(false /* isOccluded */, true /* animate */);
             finishCallback.onTransitionFinished(null /* wct */, null /* wctCB */);
+            info.releaseAllSurfaces();
         }
 
         @Override
         public void mergeAnimation(IBinder transition, TransitionInfo info,
                 SurfaceControl.Transaction t, IBinder mergeTarget,
                 IRemoteTransitionFinishedCallback finishCallback) {
+            t.close();
+            info.releaseAllSurfaces();
         }
     };
 

services/core/java/com/android/server/wm/Transition.java

@@ -751,6 +751,11 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
             Trace.asyncTraceEnd(TRACE_TAG_WINDOW_MANAGER, TRACE_NAME_PLAY_TRANSITION,
                     System.identityHashCode(this));
         }
+        // Close the transactions now. They were originally copied to Shell in case we needed to
+        // apply them due to a remote failure. Since we don't need to apply them anymore, free them
+        // immediately.
+        if (mStartTransaction != null) mStartTransaction.close();
+        if (mFinishTransaction != null) mFinishTransaction.close();
         mStartTransaction = mFinishTransaction = null;
         if (mState < STATE_PLAYING) {
             throw new IllegalStateException("Can't finish a non-playing transition " + mSyncId);
@@ -892,6 +897,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
             mController.mAtm.mWindowManager.updateRotation(false /* alwaysSendConfiguration */,
                     false /* forceRelayout */);
         }
+        cleanUpInternal();
     }
 
     void abort() {
@@ -934,6 +940,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
             dc.getPendingTransaction().merge(transaction);
             mSyncId = -1;
             mOverrideOptions = null;
+            cleanUpInternal();
             return;
         }
         // Ensure that wallpaper visibility is updated with the latest wallpaper target.
@@ -1053,6 +1060,8 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
                         "Calling onTransitionReady: %s", info);
                 mController.getTransitionPlayer().onTransitionReady(
                         mToken, info, transaction, mFinishTransaction);
+                // Since we created root-leash but no longer reference it from core, release it now
+                info.releaseAnimSurfaces();
                 if (Trace.isTagEnabled(TRACE_TAG_WINDOW_MANAGER)) {
                     Trace.asyncTraceBegin(TRACE_TAG_WINDOW_MANAGER, TRACE_NAME_PLAY_TRANSITION,
                             System.identityHashCode(this));
@@ -1088,6 +1097,16 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         mController.finishTransition(mToken);
     }
 
+    private void cleanUpInternal() {
+        // Clean-up any native references.
+        for (int i = 0; i < mChanges.size(); ++i) {
+            final ChangeInfo ci = mChanges.valueAt(i);
+            if (ci.mSnapshot != null) {
+                ci.mSnapshot.release();
+            }
+        }
+    }
+
     /** @see RecentsAnimationController#attachNavigationBarToApp */
     private void handleLegacyRecentsStartBehavior(DisplayContent dc, TransitionInfo info) {
         if ((mFlags & TRANSIT_FLAG_IS_RECENTS) == 0) {