Take a wrench to the pipes

 - Track extensions in garbage tracking because they could be big
 - Fix volume dialog cleanup to not leak
 - Fix QSFragment cleanup to not leak
 - Make DockedStackExistsListener static because it is going to leak
   because /Binder/
 - Add a secure setting to force on notifications for garbage monitoring
   for those that really really want it.

Test: adb shell dumpsys activity service SystemUI | grep -A 12 LEAK
Change-Id: Id5689febd65eb8a22dbb4a0a15ec051fcdbbf8be
Fixes: 38461559

diff --git a/packages/SystemUI/src/com/android/systemui/DockedStackExistsListener.java b/packages/SystemUI/src/com/android/systemui/DockedStackExistsListener.java
index c382882..6296297 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/DockedStackExistsListener.java
+++ b/packages/SystemUI/src/com/android/systemui/DockedStackExistsListener.java
@@ -14,56 +14,74 @@
 
 package com.android.systemui;
 
+import android.os.IBinder;
 import android.os.RemoteException;
 import android.util.Log;
 import android.view.IDockedStackListener;
 import android.view.WindowManagerGlobal;
 
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
 import java.util.function.Consumer;
 
 /**
  * Utility wrapper to listen for whether or not a docked stack exists, to be
  * used for things like the different overview icon in that mode.
  */
-public class DockedStackExistsListener extends IDockedStackListener.Stub {
+public class DockedStackExistsListener {
 
     private static final String TAG = "DockedStackExistsListener";
 
-    private final Consumer<Boolean> mCallback;
+    private static ArrayList<WeakReference<Consumer<Boolean>>> sCallbacks = new ArrayList<>();
 
-    private DockedStackExistsListener(Consumer<Boolean> callback) {
-        mCallback = callback;
-    }
+    static {
+        try {
+            WindowManagerGlobal.getWindowManagerService().registerDockedStackListener(
+                    new IDockedStackListener.Stub() {
+                        @Override
+                        public void onDividerVisibilityChanged(boolean b) throws RemoteException {
 
-    @Override
-    public void onDividerVisibilityChanged(boolean visible) throws RemoteException {
-    }
+                        }
 
-    @Override
-    public void onDockedStackExistsChanged(final boolean exists) throws RemoteException {
-        mCallback.accept(exists);
-    }
+                        @Override
+                        public void onDockedStackExistsChanged(boolean exists)
+                                throws RemoteException {
+                            DockedStackExistsListener.onDockedStackExistsChanged(exists);
+                        }
 
-    @Override
-    public void onDockedStackMinimizedChanged(boolean minimized, long animDuration,
-                                              boolean isHomeStackResizable) throws RemoteException {
-    }
+                        @Override
+                        public void onDockedStackMinimizedChanged(boolean b, long l, boolean b1)
+                                throws RemoteException {
+
+                        }
+
+                        @Override
+                        public void onAdjustedForImeChanged(boolean b, long l)
+                                throws RemoteException {
+
+                        }
+
+                        @Override
+                        public void onDockSideChanged(int i) throws RemoteException {
 
-    @Override
-    public void onAdjustedForImeChanged(boolean adjustedForIme, long animDuration)
-            throws RemoteException {
+                        }
+                    });
+        } catch (RemoteException e) {
+            Log.e(TAG, "Failed registering docked stack exists listener", e);
+        }
     }
 
-    @Override
-    public void onDockSideChanged(int newDockSide) throws RemoteException {
+
+    private static void onDockedStackExistsChanged(boolean exists) {
+        synchronized (sCallbacks) {
+            sCallbacks.removeIf(wf -> wf.get() == null);
+            sCallbacks.forEach(wf -> wf.get().accept(exists));
+        }
     }
 
     public static void register(Consumer<Boolean> callback) {
-        try {
-            WindowManagerGlobal.getWindowManagerService().registerDockedStackListener(
-                    new DockedStackExistsListener(callback));
-        } catch (RemoteException e) {
-            Log.e(TAG, "Failed registering docked stack exists listener", e);
+        synchronized (sCallbacks) {
+            sCallbacks.add(new WeakReference<>(callback));
         }
     }
 }

diff --git a/packages/SystemUI/src/com/android/systemui/fragments/ExtensionFragmentListener.java b/packages/SystemUI/src/com/android/systemui/fragments/ExtensionFragmentListener.java
index 4ff10e9..18fb423 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/fragments/ExtensionFragmentListener.java
+++ b/packages/SystemUI/src/com/android/systemui/fragments/ExtensionFragmentListener.java
@@ -45,6 +45,7 @@ public class ExtensionFragmentListener<T extends FragmentBase> implements Consum
         mFragmentHostManager.getFragmentManager().beginTransaction()
                 .replace(id, (Fragment) mExtension.get(), mTag)
                 .commit();
+        mExtension.clearItem(false);
     }
 
     @Override
@@ -57,6 +58,7 @@ public class ExtensionFragmentListener<T extends FragmentBase> implements Consum
         } catch (ClassCastException e) {
             Log.e(TAG, extension.getClass().getName() + " must be a Fragment", e);
         }
+        mExtension.clearItem(true);
     }
 
     public static <T> void attachExtensonToFragment(View view, String tag, int id,

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionController.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionController.java
index 40e3806..ede8411 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionController.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionController.java
@@ -38,6 +38,13 @@ public interface ExtensionController {
          * (like configuration) may have changed.
          */
         T reload();
+
+        /**
+         * Null out the cached item for the purpose of memory saving, should only be done
+         * when any other references are already gotten.
+         * @param isDestroyed
+         */
+        void clearItem(boolean isDestroyed);
     }
 
     interface ExtensionBuilder<T> {

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionControllerImpl.java
index c2618cd..6df2051 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/ExtensionControllerImpl.java
@@ -26,6 +26,7 @@ import com.android.systemui.plugins.PluginManager;
 import com.android.systemui.statusbar.policy.ConfigurationController.ConfigurationListener;
 import com.android.systemui.tuner.TunerService;
 import com.android.systemui.tuner.TunerService.Tunable;
+import com.android.systemui.util.leak.LeakDetector;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -146,7 +147,18 @@ public class ExtensionControllerImpl implements ExtensionController {
             return get();
         }
 
+        @Override
+        public void clearItem(boolean isDestroyed) {
+            if (isDestroyed && mItem != null) {
+                Dependency.get(LeakDetector.class).trackGarbage(mItem);
+            }
+            mItem = null;
+        }
+
         private void notifyChanged() {
+            if (mItem != null) {
+                Dependency.get(LeakDetector.class).trackGarbage(mItem);
+            }
             mItem = null;
             for (int i = 0; i < mProducers.size(); i++) {
                 final T item = mProducers.get(i).get();
@@ -169,7 +181,7 @@ public class ExtensionControllerImpl implements ExtensionController {
         }
 
         public void addTunerFactory(TunerFactory<T> factory, String[] keys) {
-            mProducers.add(new TunerItem(factory, factory.keys()));
+            mProducers.add(new TunerItem(factory, keys));
         }
 
         public void addUiMode(int uiMode, Supplier<T> mode) {

diff --git a/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java b/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java
index ba9e60a..021f9c4 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java
+++ b/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java
@@ -21,6 +21,7 @@ import android.os.Build;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.SystemProperties;
+import android.provider.Settings;
 import android.support.annotation.VisibleForTesting;
 
 import com.android.systemui.Dependency;
@@ -84,12 +85,15 @@ public class GarbageMonitor {
         // TODO(b/35345376): Turn this back on for debuggable builds after known leak fixed.
         private static final boolean ENABLED = Build.IS_DEBUGGABLE
                 && SystemProperties.getBoolean("debug.enable_leak_reporting", false);
+        private static final String FORCE_ENABLE = "sysui_force_garbage_monitor";
 
         private GarbageMonitor mGarbageMonitor;
 
         @Override
         public void start() {
-            if (!ENABLED) {
+            boolean forceEnable = Settings.Secure.getInt(mContext.getContentResolver(),
+                    FORCE_ENABLE, 0) != 0;
+            if (!ENABLED && !forceEnable) {
                 return;
             }
             mGarbageMonitor = Dependency.get(GarbageMonitor.class);

diff --git a/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java b/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java
index 32fd3e0..7e9c865 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java
@@ -180,7 +180,13 @@ public class VolumeDialogImpl implements VolumeDialog, TunerService.Tunable {
 
     @Override
     public void destroy() {
+        mAccessibility.destroy();
         mController.removeCallback(mControllerCallbackH);
+        if (mZenFooter != null) {
+            mZenFooter.cleanup();
+        }
+        Dependency.get(TunerService.class).removeTunable(this);
+        mHandler.removeCallbacksAndMessages(null);
     }
 
     private void initDialog() {
@@ -1241,16 +1247,14 @@ public class VolumeDialogImpl implements VolumeDialog, TunerService.Tunable {
                 }
             });
             mDialogView.setAccessibilityDelegate(this);
-            mAccessibilityMgr.addAccessibilityStateChangeListener(
-                    new AccessibilityStateChangeListener() {
-                        @Override
-                        public void onAccessibilityStateChanged(boolean enabled) {
-                            updateFeedbackEnabled();
-                        }
-                    });
+            mAccessibilityMgr.addAccessibilityStateChangeListener(mListener);
             updateFeedbackEnabled();
         }
 
+        public void destroy() {
+            mAccessibilityMgr.removeAccessibilityStateChangeListener(mListener);
+        }
+
         @Override
         public boolean onRequestSendAccessibilityEvent(ViewGroup host, View child,
                 AccessibilityEvent event) {
@@ -1273,6 +1277,9 @@ public class VolumeDialogImpl implements VolumeDialog, TunerService.Tunable {
             }
             return false;
         }
+
+        private final AccessibilityStateChangeListener mListener =
+                enabled -> updateFeedbackEnabled();
     }
 
     private static class VolumeRow {

diff --git a/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeExtensionController.java b/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeExtensionController.java
index daf7547..586a424 100644 (file)
--- a/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeExtensionController.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeExtensionController.java
@@ -100,6 +100,11 @@ public class FakeExtensionController implements ExtensionController {
         }
 
         @Override
+        public void clearItem(boolean isDestroyed) {
+
+        }
+
+        @Override
         public Context getContext() {
             return null;
         }