Merge "Remove removeCallbacks when processing tiles" into udc-dev

packages/SystemUI/src/com/android/systemui/qs/QSPanel.java

@@ -522,7 +522,7 @@ public class QSPanel extends LinearLayout implements Tunable {
         return mExpanded;
     }
 
-    void addTile(QSPanelControllerBase.TileRecord tileRecord) {
+    final void addTile(QSPanelControllerBase.TileRecord tileRecord) {
         final QSTile.Callback callback = new QSTile.Callback() {
             @Override
             public void onStateChanged(QSTile.State state) {

packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java

@@ -199,7 +199,7 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr
         mMediaHost.removeVisibilityChangeListener(mMediaHostVisibilityListener);
 
         for (TileRecord record : mRecords) {
-            record.tile.removeCallbacks();
+            record.tile.removeCallback(record.callback);
         }
         mRecords.clear();
         mDumpManager.unregisterDumpable(mView.getDumpableTag());

packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractor.kt

@@ -318,7 +318,6 @@ constructor(
             // We have a handful of different cases
             qsTile !is CustomTile -> {
                 // The tile is not a custom tile. Make sure they are reset to the correct user
-                qsTile.removeCallbacks()
                 if (userChanged) {
                     qsTile.userSwitch(user)
                     logger.logTileUserChanged(tileSpec, user)
@@ -327,7 +326,6 @@ constructor(
             }
             qsTile.user == user -> {
                 // The tile is a custom tile for the same user, just return it
-                qsTile.removeCallbacks()
                 qsTile
             }
             else -> {

packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerBaseTest.java

@@ -321,4 +321,30 @@ public class QSPanelControllerBaseTest extends SysuiTestCase {
         assertThat(mController.shouldUseHorizontalLayout()).isFalse();
         verify(mHorizontalLayoutListener).run();
     }
+
+    @Test
+    public void changeTiles_callbackRemovedOnOldOnes() {
+        // Start with one tile
+        assertThat(mController.mRecords.size()).isEqualTo(1);
+        QSPanelControllerBase.TileRecord record = mController.mRecords.get(0);
+
+        assertThat(record.tile).isEqualTo(mQSTile);
+
+        // Change to a different tile
+        when(mQSHost.getTiles()).thenReturn(List.of(mOtherTile));
+        mController.setTiles();
+
+        verify(mQSTile).removeCallback(record.callback);
+        verify(mOtherTile, never()).removeCallback(any());
+        verify(mOtherTile, never()).removeCallbacks();
+    }
+
+    @Test
+    public void onViewDetached_removesJustTheAssociatedCallback() {
+        QSPanelControllerBase.TileRecord record = mController.mRecords.get(0);
+
+        mController.onViewDetached();
+        verify(mQSTile).removeCallback(record.callback);
+        verify(mQSTile, never()).removeCallbacks();
+    }
 }

packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.kt

@@ -27,6 +27,8 @@ import androidx.test.filters.SmallTest
 import com.android.systemui.R
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.plugins.qs.QSTile
+import com.android.systemui.plugins.qs.QSTileView
+import com.android.systemui.qs.QSPanelControllerBase.TileRecord
 import com.android.systemui.qs.logging.QSLogger
 import com.android.systemui.qs.tileimpl.QSIconViewImpl
 import com.android.systemui.qs.tileimpl.QSTileViewImpl
@@ -192,6 +194,18 @@ class QSPanelTest : SysuiTestCase() {
         verify(accessibilityInfo, never()).addAction(actionCollapse)
     }
 
+    @Test
+    fun addTile_callbackAdded() {
+        val tile = mock(QSTile::class.java)
+        val tileView = mock(QSTileView::class.java)
+
+        val record = TileRecord(tile, tileView)
+
+        qsPanel.addTile(record)
+
+        verify(tile).addCallback(record.callback)
+    }
+
     private infix fun View.isLeftOf(other: View): Boolean {
         val rect = Rect()
         getBoundsOnScreen(rect)

packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractorImplTest.kt

@@ -589,6 +589,26 @@ class CurrentTilesInteractorImplTest : SysuiTestCase() {
                 .isTrue()
         }
 
+    @Test
+    fun retainedTiles_callbackNotRemoved() =
+        testScope.runTest(USER_INFO_0) {
+            val tiles by collectLastValue(underTest.currentTiles)
+            tileSpecRepository.setTiles(USER_INFO_0.id, listOf(TileSpec.create("a")))
+
+            val tileA = tiles!![0].tile
+            val callback = mock<QSTile.Callback>()
+            tileA.addCallback(callback)
+
+            tileSpecRepository.setTiles(
+                USER_INFO_0.id,
+                listOf(TileSpec.create("a"), CUSTOM_TILE_SPEC)
+            )
+            val newTileA = tiles!![0].tile
+            assertThat(tileA).isSameInstanceAs(newTileA)
+
+            assertThat((tileA as FakeQSTile).callbacks).containsExactly(callback)
+        }
+
     private fun QSTile.State.fillIn(state: Int, label: CharSequence, secondaryLabel: CharSequence) {
         this.state = state
         this.label = label

packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/domain/interactor/FakeQSTile.kt

@@ -29,6 +29,7 @@ class FakeQSTile(
     private var tileSpec: String? = null
     var destroyed = false
     private val state = QSTile.State()
+    val callbacks = mutableListOf<QSTile.Callback>()
 
     override fun getTileSpec(): String? {
         return tileSpec
@@ -45,11 +46,17 @@ class FakeQSTile(
 
     override fun refreshState() {}
 
-    override fun addCallback(callback: QSTile.Callback?) {}
+    override fun addCallback(callback: QSTile.Callback) {
+        callbacks.add(callback)
+    }
 
-    override fun removeCallback(callback: QSTile.Callback?) {}
+    override fun removeCallback(callback: QSTile.Callback) {
+        callbacks.remove(callback)
+    }
 
-    override fun removeCallbacks() {}
+    override fun removeCallbacks() {
+        callbacks.clear()
+    }
 
     override fun createTileView(context: Context?): QSIconView? {
         return null