From e180de6b20512862bfdf3ea1d7d795396dd54dd9 Mon Sep 17 00:00:00 2001 From: Fengjiang Li Date: Tue, 29 Aug 2023 15:34:59 -0700 Subject: [PATCH] Fix memory leak of Launcher activityfrom QuickstepTransitionManager and LauncherBackAnimationController We should also avoid using non-static inner class that extends IOnBackInvokedCallback.Stub and IRemoteAnimationRunner.Stub inside LauncherBackAnimationController, which references the entire LauncherBackAnimationController object. 1. When launcher is created, a Runnable is posted to ShellExecutor to call BackAnimationController#registerAnimation 2. When launcher is later destroyed, another Runnable is posted to same ShellExecturo to call BackAnimationController#unregisterAnimation 3. If the execturo queued the 1st runnable, then we have leaked LauncherBackAnimationController object, including Launcher activity. This CL fixes the leak by making the Stub static inner classes, and use weak reference hold reference to launcher activity. Bug: 297806573 Test: Grab a heap dump and this reference no longer exist Flag: N/A Change-Id: I78853e900a98399b02682ba2d9179e544a4030d5 --- .../launcher3/QuickstepTransitionManager.java | 42 ++--- .../LauncherBackAnimationController.java | 153 +++++++++++------- 2 files changed, 119 insertions(+), 76 deletions(-) diff --git a/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java b/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java index c6c4dde6b7..f8ea9320c4 100644 --- a/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java +++ b/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java @@ -159,6 +159,7 @@ import com.android.systemui.shared.system.RemoteAnimationRunnerCompat; import com.android.wm.shell.startingsurface.IStartingWindowListener; import java.io.PrintWriter; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -225,7 +226,8 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener private final float mClosingWindowTransY; private final float mMaxShadowRadius; - private final StartingWindowListener mStartingWindowListener = new StartingWindowListener(); + private final StartingWindowListener mStartingWindowListener = + new StartingWindowListener(this); private DeviceProfile mDeviceProfile; @@ -278,7 +280,6 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener } }; - mStartingWindowListener.setTransitionManager(this); SystemUiProxy.INSTANCE.get(mLauncher).setStartingWindowListener( mStartingWindowListener); } @@ -310,8 +311,8 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener mAppLaunchRunner = new AppLaunchAnimationRunner(v, onEndCallback); ItemInfo tag = (ItemInfo) v.getTag(); if (tag != null && tag.shouldUseBackgroundAnimation()) { - ContainerAnimationRunner containerAnimationRunner = - ContainerAnimationRunner.from(v, mStartingWindowListener, onEndCallback); + ContainerAnimationRunner containerAnimationRunner = ContainerAnimationRunner.from( + v, mLauncher, mStartingWindowListener, onEndCallback); if (containerAnimationRunner != null) { mAppLaunchRunner = containerAnimationRunner; } @@ -1152,7 +1153,6 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener public void onActivityDestroyed() { unregisterRemoteAnimations(); unregisterRemoteTransitions(); - mStartingWindowListener.setTransitionManager(null); SystemUiProxy.INSTANCE.get(mLauncher).setStartingWindowListener(null); } @@ -1775,8 +1775,8 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener } @Nullable - private static ContainerAnimationRunner from( - View v, StartingWindowListener startingWindowListener, RunnableList onEndCallback) { + private static ContainerAnimationRunner from(View v, Launcher launcher, + StartingWindowListener startingWindowListener, RunnableList onEndCallback) { View viewToUse = findViewWithBackground(v); if (viewToUse == null) { viewToUse = v; @@ -1801,8 +1801,13 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener } }; - ActivityLaunchAnimator.Callback callback = task -> ColorUtils.setAlphaComponent( - startingWindowListener.getBackgroundColor(), 255); + ActivityLaunchAnimator.Callback callback = task -> { + final int backgroundColor = + startingWindowListener.mBackgroundColor == Color.TRANSPARENT + ? launcher.getScrimView().getBackgroundColor() + : startingWindowListener.mBackgroundColor; + return ColorUtils.setAlphaComponent(backgroundColor, 255); + }; ActivityLaunchAnimator.Listener listener = new ActivityLaunchAnimator.Listener() { @Override @@ -1912,25 +1917,22 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener } } - private class StartingWindowListener extends IStartingWindowListener.Stub { - private QuickstepTransitionManager mTransitionManager; + private static class StartingWindowListener extends IStartingWindowListener.Stub { + private final WeakReference mTransitionManagerRef; private int mBackgroundColor; - public void setTransitionManager(QuickstepTransitionManager transitionManager) { - mTransitionManager = transitionManager; + private StartingWindowListener(QuickstepTransitionManager transitionManager) { + mTransitionManagerRef = new WeakReference<>(transitionManager); } @Override public void onTaskLaunching(int taskId, int supportedType, int color) { - mTransitionManager.mTaskStartParams.put(taskId, Pair.create(supportedType, color)); + QuickstepTransitionManager transitionManager = mTransitionManagerRef.get(); + if (transitionManager != null) { + transitionManager.mTaskStartParams.put(taskId, Pair.create(supportedType, color)); + } mBackgroundColor = color; } - - public int getBackgroundColor() { - return mBackgroundColor == Color.TRANSPARENT - ? mLauncher.getScrimView().getBackgroundColor() - : mBackgroundColor; - } } /** diff --git a/quickstep/src/com/android/quickstep/LauncherBackAnimationController.java b/quickstep/src/com/android/quickstep/LauncherBackAnimationController.java index f1660ee858..857c83106b 100644 --- a/quickstep/src/com/android/quickstep/LauncherBackAnimationController.java +++ b/quickstep/src/com/android/quickstep/LauncherBackAnimationController.java @@ -58,6 +58,8 @@ import com.android.launcher3.uioverrides.QuickstepLauncher; import com.android.quickstep.util.RectFSpringAnim; import com.android.systemui.shared.system.QuickStepContract; +import java.lang.ref.WeakReference; + /** * Controls the animation of swiping back and returning to launcher. * @@ -105,7 +107,7 @@ public class LauncherBackAnimationController { private boolean mAnimatorSetInProgress = false; private float mBackProgress = 0; private boolean mBackInProgress = false; - private IOnBackInvokedCallback mBackCallback; + private OnBackInvokedCallbackStub mBackCallback; private IRemoteAnimationFinishedCallback mAnimationFinishedCallback; private BackProgressAnimator mProgressAnimator = new BackProgressAnimator(); private SurfaceControl mScrimLayer; @@ -137,65 +139,104 @@ public class LauncherBackAnimationController { * @param handler Handler to the thread to run the animations on. */ public void registerBackCallbacks(Handler handler) { - mBackCallback = new IOnBackInvokedCallback.Stub() { - @Override - public void onBackCancelled() { - handler.post(() -> { + mBackCallback = new OnBackInvokedCallbackStub(handler, mProgressAnimator, this); + SystemUiProxy.INSTANCE.get(mLauncher).setBackToLauncherCallback(mBackCallback, + new RemoteAnimationRunnerStub(this)); + } + + private static class OnBackInvokedCallbackStub extends IOnBackInvokedCallback.Stub { + private Handler mHandler; + private BackProgressAnimator mProgressAnimator; + // LauncherBackAnimationController has strong reference to Launcher activity, the binder + // callback should not hold strong reference to it to avoid memory leak. + private WeakReference mControllerRef; + + private OnBackInvokedCallbackStub( + Handler handler, + BackProgressAnimator progressAnimator, + LauncherBackAnimationController controller) { + mHandler = handler; + mProgressAnimator = progressAnimator; + mControllerRef = new WeakReference<>(controller); + } + + @Override + public void onBackCancelled() { + mHandler.post(() -> { + LauncherBackAnimationController controller = mControllerRef.get(); + if (controller != null) { mProgressAnimator.onBackCancelled( - LauncherBackAnimationController.this::resetPositionAnimated); - }); - } - - @Override - public void onBackInvoked() { - handler.post(() -> { - startTransition(); - mProgressAnimator.reset(); - }); - } - - @Override - public void onBackProgressed(BackMotionEvent backEvent) { - handler.post(() -> { - mProgressAnimator.onBackProgressed(backEvent); - }); - } - - @Override - public void onBackStarted(BackMotionEvent backEvent) { - handler.post(() -> { - startBack(backEvent); - mProgressAnimator.onBackStarted(backEvent, event -> { - mBackProgress = event.getProgress(); - // TODO: Update once the interpolation curve spec is finalized. - mBackProgress = - 1 - (1 - mBackProgress) * (1 - mBackProgress) * (1 - - mBackProgress); - updateBackProgress(mBackProgress, event); - }); - }); - } - }; - - final IRemoteAnimationRunner runner = new IRemoteAnimationRunner.Stub() { - @Override - public void onAnimationStart(int transit, RemoteAnimationTarget[] apps, - RemoteAnimationTarget[] wallpapers, RemoteAnimationTarget[] nonApps, - IRemoteAnimationFinishedCallback finishedCallback) { - for (final RemoteAnimationTarget target : apps) { - if (MODE_CLOSING == target.mode) { - mBackTarget = target; - break; - } + controller::resetPositionAnimated); } - mAnimationFinishedCallback = finishedCallback; + }); + } + + @Override + public void onBackInvoked() { + mHandler.post(() -> { + LauncherBackAnimationController controller = mControllerRef.get(); + if (controller != null) { + controller.startTransition(); + } + mProgressAnimator.reset(); + }); + } + + @Override + public void onBackProgressed(BackMotionEvent backMotionEvent) { + mHandler.post(() -> { + mProgressAnimator.onBackProgressed(backMotionEvent); + }); + } + + @Override + public void onBackStarted(BackMotionEvent backEvent) { + mHandler.post(() -> { + LauncherBackAnimationController controller = mControllerRef.get(); + if (controller != null) { + controller.startBack(backEvent); + mProgressAnimator.onBackStarted(backEvent, event -> { + float backProgress = event.getProgress(); + // TODO: Update once the interpolation curve spec is finalized. + controller.mBackProgress = + 1 - (1 - backProgress) * (1 - backProgress) * (1 + - backProgress); + controller.updateBackProgress(controller.mBackProgress, event); + }); + } + }); + } + } + + private static class RemoteAnimationRunnerStub extends IRemoteAnimationRunner.Stub { + + // LauncherBackAnimationController has strong reference to Launcher activity, the binder + // callback should not hold strong reference to it to avoid memory leak. + private WeakReference mControllerRef; + + private RemoteAnimationRunnerStub(LauncherBackAnimationController controller) { + mControllerRef = new WeakReference<>(controller); + } + + @Override + public void onAnimationStart(int transit, RemoteAnimationTarget[] apps, + RemoteAnimationTarget[] wallpapers, RemoteAnimationTarget[] nonApps, + IRemoteAnimationFinishedCallback finishedCallback) { + LauncherBackAnimationController controller = mControllerRef.get(); + if (controller == null) { + return; } + for (final RemoteAnimationTarget target : apps) { + if (MODE_CLOSING == target.mode) { + controller.mBackTarget = target; + break; + } + } + controller.mAnimationFinishedCallback = finishedCallback; + } - @Override - public void onAnimationCancelled() {} - }; - - SystemUiProxy.INSTANCE.get(mLauncher).setBackToLauncherCallback(mBackCallback, runner); + @Override + public void onAnimationCancelled() {} } private void resetPositionAnimated() {