Remove removeCallbacks when processing tiles

This is not needed and can lead to the following error:
1. QSPanel added a callback for tile A
2. CurrentTilesInteractor processes tiles and removes all callbacks from
   A. After that, the list of tiles didn't change. As it's backed by a
   StateFlow, consumers don't get notified of a change (they don't need
   to, the list is the same).
3. QSPanelControllerBase doesn't get a notified so it doesn't re-add the
   callback.

The error is not usually observable, as it requires the tile setting to
change but the list of actual tiles not to change, though certain
scenarios could make it happen (like restoring from backup with tiles
that are not available, or changing tiles through adb).

Also, have QSPanelControllerBase only remove the associated callback
(and not all of them) onViewDetached.

Flag: QS_PIPELINE_NEW_HOST
Fixes: 282978588
Test: atest com.android.systemui.qs
Change-Id: I6451c80a7595bd3b3614de7b2f2870f6e97ba9d1

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