From 52d938d31f9ed1d96f3884345291089a9b7c3585 Mon Sep 17 00:00:00 2001
From: Winson Chung <winsonc@google.com>
Date: Wed, 25 Sep 2019 16:50:05 -0700
Subject: [PATCH] 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
---
 .../view/CompositionSamplingListener.java     | 22 ++++++++++++++-----
 .../phone/NavigationBarFragment.java          |  3 +++
 .../statusbar/phone/RegionSamplingHelper.java |  9 +++++---
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/core/java/android/view/CompositionSamplingListener.java b/core/java/android/view/CompositionSamplingListener.java
index 368445cde72c5..677a559cd3b0c 100644
--- a/core/java/android/view/CompositionSamplingListener.java
+++ b/core/java/android/view/CompositionSamplingListener.java
@@ -28,7 +28,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) {
@@ -36,13 +36,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();
         }
@@ -58,6 +64,9 @@ public abstract class CompositionSamplingListener {
      */
     public static void register(CompositionSamplingListener listener,
             int displayId, SurfaceControl stopLayer, Rect samplingArea) {
+        if (listener.mNativeListener == 0) {
+            return;
+        }
         Preconditions.checkArgument(displayId == Display.DEFAULT_DISPLAY,
                 "default display only for now");
         long nativeStopLayerObject = stopLayer != null ? stopLayer.mNativeObject : 0;
@@ -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/NavigationBarFragment.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java
index b87140dddec39..6be5e7aa87912 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java
@@ -139,6 +139,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;
 
@@ -260,6 +261,7 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback
         mSysUiFlagsContainer = sysUiFlagsContainer;
         mAssistantAvailable = mAssistManager.getAssistInfoForUser(UserHandle.USER_CURRENT) != null;
         mOverviewProxyService = overviewProxyService;
+        mNavigationModeController = navigationModeController;
         mNavBarMode = navigationModeController.addListener(this);
     }
 
@@ -299,6 +301,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 c1ff572bb210e..1a6b415f87db8 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