From 8f126e8e660bd3306e41c48343d977650dee778f Mon Sep 17 00:00:00 2001 From: Shamali P Date: Wed, 12 Mar 2025 23:00:40 +0000 Subject: [PATCH 1/2] Change the studio build specific onEndCallback exception to a log.e Since developers have been relying on detecting leaks from studio build and this exception can cause rest of on destroy to not run, to avoid false positives, we could just rely on Log.e instead. See bug for more details. Bug: 400793700 Flag: EXEMPT BUGFIX Test: Leak canary and studio heap dump Change-Id: Ieea35ecfcc4b0f048b47c204809bbbf39e367b06 --- .../src/com/android/launcher3/QuickstepTransitionManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java b/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java index 7cf06050fa..2af72c91d8 100644 --- a/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java +++ b/quickstep/src/com/android/launcher3/QuickstepTransitionManager.java @@ -92,6 +92,7 @@ import android.os.Looper; import android.os.SystemProperties; import android.os.UserHandle; import android.provider.Settings; +import android.util.Log; import android.util.Pair; import android.util.Size; import android.view.CrossWindowBlurListeners; @@ -182,6 +183,7 @@ import java.util.Map.Entry; * Manages the opening and closing app transitions from Launcher */ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener { + private static final String TAG = "QuickstepTransitionManager"; private static final boolean ENABLE_SHELL_STARTING_SURFACE = SystemProperties.getBoolean("persist.debug.shell_starting_surface", true); @@ -1207,7 +1209,7 @@ public class QuickstepTransitionManager implements OnDeviceProfileChangeListener mLauncher.removeOnDeviceProfileChangeListener(this); SystemUiProxy.INSTANCE.get(mLauncher).setStartingWindowListener(null); if (BuildConfig.IS_STUDIO_BUILD && !mRegisteredTaskStackChangeListener.isEmpty()) { - throw new IllegalStateException("Failed to run onEndCallback created from" + Log.e(TAG, "IllegalState: Failed to run onEndCallback created from" + " getActivityLaunchOptions()"); } mRegisteredTaskStackChangeListener.forEach(TaskRestartedDuringLaunchListener::unregister); From 9cf28d1772c94141599f643c508309d84f915973 Mon Sep 17 00:00:00 2001 From: Shamali P Date: Wed, 12 Mar 2025 23:03:44 +0000 Subject: [PATCH 2/2] Improve reliability of cleanup done in onDestroy. As seen in the bug, an exception caused rest of onDestroy to not clean up the other objects - which could cause other leaks. So, for somewhat non-trivial clean ups, I've wrapped them in try-catch. This way, with Log.e we still will know about them, but not crash destruction of activity and other clean up. Bug: 400793700 Flag: EXEMPT BUGFIX Test: Leak canary and studio heap dump Change-Id: I45e7298394eafff182a2c800eebfa5f772fbe2de --- .../uioverrides/QuickstepLauncher.java | 46 ++++++++++++++----- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java b/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java index 1a42d21b6f..14a99e147b 100644 --- a/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java +++ b/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java @@ -86,6 +86,7 @@ import android.os.Trace; import android.os.UserHandle; import android.text.TextUtils; import android.util.AttributeSet; +import android.util.Log; import android.view.Display; import android.view.HapticFeedbackConstants; import android.view.KeyEvent; @@ -226,6 +227,7 @@ import java.util.stream.Stream; public class QuickstepLauncher extends Launcher implements RecentsViewContainer, SystemShortcut.BubbleActivityStarter { + private static final String TAG = "QuickstepLauncher"; private static final boolean TRACE_LAYOUTS = SystemProperties.getBoolean("persist.debug.trace_layouts", false); private static final String TRACE_RELAYOUT_CLASS = @@ -561,20 +563,35 @@ public class QuickstepLauncher extends Launcher implements RecentsViewContainer, @Override public void onDestroy() { + // wrap non-trivial clean up blocks in try-catch to avoid stopping clean up of rest of + // objects + if (mAppTransitionManager != null) { - mAppTransitionManager.onActivityDestroyed(); + try { + mAppTransitionManager.onActivityDestroyed(); + } catch (Exception e) { + Log.e(TAG, "Failed to destroy mAppTransitionManager", e); + } } mAppTransitionManager = null; mIsPredictiveBackToHomeInProgress = false; if (mUnfoldTransitionProgressProvider != null) { - SystemUiProxy.INSTANCE.get(this).setUnfoldAnimationListener(null); - mUnfoldTransitionProgressProvider.destroy(); + try { + SystemUiProxy.INSTANCE.get(this).setUnfoldAnimationListener(null); + mUnfoldTransitionProgressProvider.destroy(); + } catch (Exception e) { + Log.e(TAG, "Failed to destroy mUnfoldTransitionProgressProvider", e); + } } OverviewComponentObserver.INSTANCE.get(this) .removeOverviewChangeListener(mOverviewChangeListener); - mTISBindHelper.onDestroy(); + try { + mTISBindHelper.onDestroy(); + } catch (Exception e) { + Log.e(TAG, "Failed to destroy mTISBindHelper", e); + } if (mLauncherUnfoldAnimationController != null) { mLauncherUnfoldAnimationController.onDestroy(); @@ -584,15 +601,22 @@ public class QuickstepLauncher extends Launcher implements RecentsViewContainer, mSplitSelectStateController.onDestroy(); } - RecentsView recentsView = getOverviewPanel(); - if (recentsView != null) { - recentsView.destroy(); + try { + RecentsView recentsView = getOverviewPanel(); + if (recentsView != null) { + recentsView.destroy(); + } + } catch (Exception e) { + Log.e(TAG, "Failed to destroy RecentsView", e); } - super.onDestroy(); - mHotseatPredictionController.destroy(); - if (mViewCapture != null) mViewCapture.close(); - removeBackAnimationCallback(mSplitSelectStateController.getSplitBackHandler()); + try { + super.onDestroy(); + } finally { // trivial close operations in finally. + mHotseatPredictionController.destroy(); + if (mViewCapture != null) mViewCapture.close(); + removeBackAnimationCallback(mSplitSelectStateController.getSplitBackHandler()); + } } @Override