commit 3cc2222d1a0db83a7950807bba0749393664a96f
author Robert Snoeberger <snoeberger@google.com> Wed Mar 25 15:36:31 2020 -0400
committer Robert Snoeberger <snoeberger@google.com> Thu Mar 26 14:14:58 2020 -0400
tree 5cec6561d39c5ee32f8fd9377cced27c26c35252
parent ef7b53424b6794d7167a151d11d0c58540b7c0e3

Unregister MediaListener on view detached

Fixes: 152406740
Test: manual - toggle dark mode and check heap dump
Change-Id: I49deb7a42cfab25807cbff7011f824d0ce1463f8

packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java

diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
index e208ee2..9873d24 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
@@ -37,6 +37,7 @@
 import android.view.KeyEvent;
 import android.view.LayoutInflater;
 import android.view.View;
+import android.view.View.OnAttachStateChangeListener;
 import android.view.ViewGroup;
 import android.widget.ImageButton;
 import android.widget.ImageView;
@@ -53,6 +54,8 @@
 import com.android.systemui.R;
 import com.android.systemui.plugins.ActivityStarter;
 import com.android.systemui.statusbar.NotificationMediaManager;
+import com.android.systemui.statusbar.NotificationMediaManager.MediaListener;
+import com.android.systemui.util.Assert;
 
 import java.util.List;
 import java.util.concurrent.Executor;
@@ -60,7 +63,7 @@
 /**
  * Base media control panel for System UI
  */
-public class MediaControlPanel implements NotificationMediaManager.MediaListener {
+public class MediaControlPanel {
     private static final String TAG = "MediaControlPanel";
     private final NotificationMediaManager mMediaManager;
     private final Executor mForegroundExecutor;
@@ -74,6 +77,7 @@
     private int mForegroundColor;
     private int mBackgroundColor;
     protected ComponentName mRecvComponent;
+    private boolean mIsRegistered = false;
 
     private final int[] mActionIds;
 
@@ -86,12 +90,34 @@
             com.android.internal.R.id.action4
     };
 
-    private MediaController.Callback mSessionCallback = new MediaController.Callback() {
+    private final MediaController.Callback mSessionCallback = new MediaController.Callback() {
         @Override
         public void onSessionDestroyed() {
             Log.d(TAG, "session destroyed");
             mController.unregisterCallback(mSessionCallback);
             clearControls();
+            makeInactive();
+        }
+    };
+
+    private final MediaListener mMediaListener = new MediaListener() {
+        @Override
+        public void onMetadataOrStateChanged(MediaMetadata metadata, int state) {
+            if (state == PlaybackState.STATE_NONE) {
+                clearControls();
+                makeInactive();
+            }
+        }
+    };
+
+    private final OnAttachStateChangeListener mStateListener = new OnAttachStateChangeListener() {
+        @Override
+        public void onViewAttachedToWindow(View unused) {
+            makeActive();
+        }
+        @Override
+        public void onViewDetachedFromWindow(View unused) {
+            makeInactive();
         }
     };
 
@@ -111,6 +137,12 @@
         mContext = context;
         LayoutInflater inflater = LayoutInflater.from(mContext);
         mMediaNotifView = (LinearLayout) inflater.inflate(layoutId, parent, false);
+        // TODO(b/150854549): removeOnAttachStateChangeListener when this doesn't inflate views
+        // mStateListener shouldn't need to be unregistered since this object shares the same
+        // lifecycle with the inflated view. It would be better, however, if this controller used an
+        // attach/detach of views instead of inflating them in the constructor, which would allow
+        // mStateListener to be unregistered in detach.
+        mMediaNotifView.addOnAttachStateChangeListener(mStateListener);
         mMediaManager = manager;
         mActionIds = actionIds;
         mForegroundExecutor = foregroundExecutor;
@@ -236,9 +268,7 @@
             });
         }
 
-        // Ensure is only added once
-        mMediaManager.removeCallback(this);
-        mMediaManager.addCallback(this);
+        makeActive();
     }
 
     /**
@@ -422,11 +452,20 @@
         btn.setVisibility(View.VISIBLE);
     }
 
-    @Override
-    public void onMetadataOrStateChanged(MediaMetadata metadata, int state) {
-        if (state == PlaybackState.STATE_NONE) {
-            clearControls();
-            mMediaManager.removeCallback(this);
+    private void makeActive() {
+        Assert.isMainThread();
+        if (!mIsRegistered) {
+            mMediaManager.addCallback(mMediaListener);
+            mIsRegistered = true;
         }
     }
+
+    private void makeInactive() {
+        Assert.isMainThread();
+        if (mIsRegistered) {
+            mMediaManager.removeCallback(mMediaListener);
+            mIsRegistered = false;
+        }
+    }
+
 }