From 00cfbeb7e5d6a90953e206070f6a2862b88a89b6 Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Wed, 25 Sep 2019 16:50:05 -0700 Subject: Fix nav bar leak in SystemUI - Ensure that the composition sampling listener is destroyed when the nav bar is destroyed instead of waiting for the finalize callback. That callback would never be made since the sampling listener has a reference to the outer class and has a strong ref from SF, and the other class has itself a reference to the listener. - Always unregister the nav bar fragment from the mode change callbacks Bug: 141473489 Test: Switch navigation modes a couple times, take hprof dump and verify there aren't leaking classes Change-Id: Ic389a559a3d430af495365102854d531f7d1966d Merged-In: Ic389a559a3d430af495365102854d531f7d1966d --- .../android/view/CompositionSamplingListener.java | 22 +++++++++++++++++----- .../statusbar/phone/NavBarTintController.java | 8 ++++++-- .../statusbar/phone/NavigationBarFragment.java | 3 +++ .../statusbar/phone/RegionSamplingHelper.java | 9 ++++++--- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/core/java/android/view/CompositionSamplingListener.java b/core/java/android/view/CompositionSamplingListener.java index e4309a6d9aa4..f23bb40278ac 100644 --- a/core/java/android/view/CompositionSamplingListener.java +++ b/core/java/android/view/CompositionSamplingListener.java @@ -29,7 +29,7 @@ import java.util.concurrent.Executor; */ public abstract class CompositionSamplingListener { - private final long mNativeListener; + private long mNativeListener; private final Executor mExecutor; public CompositionSamplingListener(Executor executor) { @@ -37,13 +37,19 @@ public abstract class CompositionSamplingListener { mNativeListener = nativeCreate(this); } + public void destroy() { + if (mNativeListener == 0) { + return; + } + unregister(this); + nativeDestroy(mNativeListener); + mNativeListener = 0; + } + @Override protected void finalize() throws Throwable { try { - if (mNativeListener != 0) { - unregister(this); - nativeDestroy(mNativeListener); - } + destroy(); } finally { super.finalize(); } @@ -59,6 +65,9 @@ public abstract class CompositionSamplingListener { */ public static void register(CompositionSamplingListener listener, int displayId, IBinder stopLayer, Rect samplingArea) { + if (listener.mNativeListener == 0) { + return; + } Preconditions.checkArgument(displayId == Display.DEFAULT_DISPLAY, "default display only for now"); nativeRegister(listener.mNativeListener, stopLayer, samplingArea.left, samplingArea.top, @@ -69,6 +78,9 @@ public abstract class CompositionSamplingListener { * Unregisters a sampling listener. */ public static void unregister(CompositionSamplingListener listener) { + if (listener.mNativeListener == 0) { + return; + } nativeUnregister(listener.mNativeListener); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavBarTintController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavBarTintController.java index bfd17b9abc72..68ea81dfce55 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavBarTintController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavBarTintController.java @@ -107,6 +107,11 @@ public class NavBarTintController implements View.OnAttachStateChangeListener, requestUpdateSamplingListener(); } + void stopAndDestroy() { + stop(); + mSamplingListener.destroy(); + } + @Override public void onViewAttachedToWindow(View view) { requestUpdateSamplingListener(); @@ -114,8 +119,7 @@ public class NavBarTintController implements View.OnAttachStateChangeListener, @Override public void onViewDetachedFromWindow(View view) { - // Defer calling updateSamplingListener the attach info has not yet been reset - requestUpdateSamplingListener(); + stopAndDestroy(); } @Override diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java index e9731c521308..9a5f35a979eb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java @@ -137,6 +137,7 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback private final MetricsLogger mMetricsLogger; private final DeviceProvisionedController mDeviceProvisionedController; private final StatusBarStateController mStatusBarStateController; + private final NavigationModeController mNavigationModeController; protected NavigationBarView mNavigationBarView = null; @@ -253,6 +254,7 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback mAssistManager = assistManager; mAssistantAvailable = mAssistManager.getAssistInfoForUser(UserHandle.USER_CURRENT) != null; mOverviewProxyService = overviewProxyService; + mNavigationModeController = navigationModeController; mNavBarMode = navigationModeController.addListener(this); } @@ -292,6 +294,7 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback @Override public void onDestroy() { super.onDestroy(); + mNavigationModeController.removeListener(this); mAccessibilityManagerWrapper.removeCallback(mAccessibilityListener); mContentResolver.unregisterContentObserver(mMagnificationObserver); mContentResolver.unregisterContentObserver(mAssistContentObserver); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/RegionSamplingHelper.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/RegionSamplingHelper.java index 2b0bb21c6560..26cc74d16ccf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/RegionSamplingHelper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/RegionSamplingHelper.java @@ -127,6 +127,11 @@ public class RegionSamplingHelper implements View.OnAttachStateChangeListener, updateSamplingListener(); } + void stopAndDestroy() { + stop(); + mSamplingListener.destroy(); + } + @Override public void onViewAttachedToWindow(View view) { updateSamplingListener(); @@ -134,9 +139,7 @@ public class RegionSamplingHelper implements View.OnAttachStateChangeListener, @Override public void onViewDetachedFromWindow(View view) { - // isAttachedToWindow is only changed after this call to the listeners, so let's post it - // instead - postUpdateSamplingListener(); + stopAndDestroy(); } @Override -- cgit v1.2.3-59-g8ed1b