Merge changes Ieac39be6,Id3c9a088 into tm-qpr-dev

* changes:
  [Status bar] Clarify the API between TileServices and status bar icons.
  [Status Bar] Move icon controller's CommandQueue.Callbacks into object.

packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java

@@ -132,9 +132,8 @@ public class TileServices extends IQSService.Stub {
             mServices.remove(tile);
             mTokenMap.remove(service.getToken());
             mTiles.remove(tile.getComponent());
-            final String slot = tile.getComponent().getClassName();
+            final String slot = getStatusBarIconSlotName(tile.getComponent());
-            // TileServices doesn't know how to add more than 1 icon per slot, so remove all
+            mMainHandler.post(() -> mStatusBarIconController.removeIconForTile(slot));
-            mMainHandler.post(() -> mStatusBarIconController.removeAllIconsForSlot(slot));
         }
     }
 
@@ -308,12 +307,11 @@ public class TileServices extends IQSService.Stub {
                             ? new StatusBarIcon(userHandle, packageName, icon, 0, 0,
                                     contentDescription)
                             : null;
+                    final String slot = getStatusBarIconSlotName(componentName);
                     mMainHandler.post(new Runnable() {
                         @Override
                         public void run() {
-                            StatusBarIconController iconController = mStatusBarIconController;
+                            mStatusBarIconController.setIconFromTile(slot, statusIcon);
-                            iconController.setIcon(componentName.getClassName(), statusIcon);
-                            iconController.setExternalIcon(componentName.getClassName());
                         }
                     });
                 }
@@ -373,6 +371,12 @@ public class TileServices extends IQSService.Stub {
         mCommandQueue.removeCallback(mRequestListeningCallback);
     }
 
+    /** Returns the slot name that should be used when adding or removing status bar icons. */
+    private String getStatusBarIconSlotName(ComponentName componentName) {
+        return componentName.getClassName();
+    }
+
+
     private final CommandQueue.Callbacks mRequestListeningCallback = new CommandQueue.Callbacks() {
         @Override
         public void requestTileServiceListeningState(@NonNull ComponentName componentName) {

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java

@@ -81,28 +81,22 @@ public interface StatusBarIconController {
     void refreshIconGroup(IconManager iconManager);
 
     /**
-     * Adds or updates an icon for a given slot for a **tile service icon**.
+     * Adds or updates an icon that comes from an active tile service.
      *
-     * TODO(b/265307726): Merge with {@link #setIcon(String, StatusBarIcon)} or make this method
+     * If the icon is null, the icon will be removed.
-     *   much more clearly distinct from that method.
      */
-    void setExternalIcon(String slot);
+    void setIconFromTile(String slot, @Nullable StatusBarIcon icon);
+
+    /** Removes an icon that had come from an active tile service. */
+    void removeIconForTile(String slot);
 
     /**
      * Adds or updates an icon for the given slot for **internal system icons**.
      *
-     * TODO(b/265307726): Rename to `setInternalIcon`, or merge this appropriately with the
+     * TODO(b/265307726): Re-name this to `setInternalIcon`.
-     * {@link #setIcon(String, StatusBarIcon)} method.
      */
     void setIcon(String slot, int resourceId, CharSequence contentDescription);
 
-    /**
-     * Adds or updates an icon for the given slot for an **externally-provided icon**.
-     *
-     * TODO(b/265307726): Rename to `setExternalIcon` or something similar.
-     */
-    void setIcon(String slot, StatusBarIcon icon);
-
     /** */
     void setWifiIcon(String slot, WifiIconState state);
 
@@ -152,15 +146,10 @@ public interface StatusBarIconController {
      */
     void removeIcon(String slot, int tag);
 
-    /** */
-    void removeAllIconsForSlot(String slot);
-
     /**
-     * Removes all the icons for the given slot.
+     * TODO(b/265307726): Re-name this to `removeAllIconsForInternalSlot`.
-     *
-     * Only use this for icons that have come from **an external process**.
      */
-    void removeAllIconsForExternalSlot(String slot);
+    void removeAllIconsForSlot(String slot);
 
     // TODO: See if we can rename this tunable name.
     String ICON_HIDE_LIST = "icon_blacklist";
@@ -618,13 +607,6 @@ public interface StatusBarIconController {
             mGroup.removeAllViews();
         }
 
-        protected void onIconExternal(int viewIndex, int height) {
-            ImageView imageView = (ImageView) mGroup.getChildAt(viewIndex);
-            imageView.setScaleType(ImageView.ScaleType.FIT_CENTER);
-            imageView.setAdjustViewBounds(true);
-            setHeightAndCenter(imageView, height);
-        }
-
         protected void onDensityOrFontScaleChanged() {
             for (int i = 0; i < mGroup.getChildCount(); i++) {
                 View child = mGroup.getChildAt(i);

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java

@@ -32,7 +32,6 @@ import androidx.annotation.VisibleForTesting;
 
 import com.android.internal.statusbar.StatusBarIcon;
 import com.android.systemui.Dumpable;
-import com.android.systemui.R;
 import com.android.systemui.dagger.SysUISingleton;
 import com.android.systemui.demomode.DemoMode;
 import com.android.systemui.demomode.DemoModeController;
@@ -62,7 +61,7 @@ import javax.inject.Inject;
  */
 @SysUISingleton
 public class StatusBarIconControllerImpl implements Tunable,
-        ConfigurationListener, Dumpable, CommandQueue.Callbacks, StatusBarIconController, DemoMode {
+        ConfigurationListener, Dumpable, StatusBarIconController, DemoMode {
 
     private static final String TAG = "StatusBarIconController";
     // Use this suffix to prevent external icon slot names from unintentionally overriding our
@@ -93,7 +92,7 @@ public class StatusBarIconControllerImpl implements Tunable,
         mStatusBarPipelineFlags = statusBarPipelineFlags;
 
         configurationController.addCallback(this);
-        commandQueue.addCallback(this);
+        commandQueue.addCallback(mCommandQueueCallbacks);
         tunerService.addTunable(this, ICON_HIDE_LIST);
         demoModeController.addCallback(this);
         dumpManager.registerDumpable(getClass().getSimpleName(), this);
@@ -350,26 +349,35 @@ public class StatusBarIconControllerImpl implements Tunable,
         }
     }
 
-    @Override
+    private final CommandQueue.Callbacks mCommandQueueCallbacks = new CommandQueue.Callbacks() {
-    public void setExternalIcon(String slot) {
-        String slotName = createExternalSlotName(slot);
-        int viewIndex = mStatusBarIconList.getViewIndex(slotName, 0);
-        int height = mContext.getResources().getDimensionPixelSize(
-                R.dimen.status_bar_icon_drawing_size);
-        mIconGroups.forEach(l -> l.onIconExternal(viewIndex, height));
-    }
-
-    // Override for *both* CommandQueue.Callbacks AND StatusBarIconController.
-    // TODO(b/265307726): Pull out the CommandQueue callbacks into a member variable to
-    //  differentiate between those callback methods and StatusBarIconController methods.
         @Override
         public void setIcon(String slot, StatusBarIcon icon) {
-        String slotName = createExternalSlotName(slot);
+            // Icons that come from CommandQueue are from external services.
-        if (icon == null) {
+            setExternalIcon(slot, icon);
-            removeAllIconsForSlot(slotName);
-            return;
         }
 
+        @Override
+        public void removeIcon(String slot) {
+            removeAllIconsForExternalSlot(slot);
+        }
+    };
+
+    @Override
+    public void setIconFromTile(String slot, StatusBarIcon icon) {
+        setExternalIcon(slot, icon);
+    }
+
+    @Override
+    public void removeIconForTile(String slot) {
+        removeAllIconsForExternalSlot(slot);
+    }
+
+    private void setExternalIcon(String slot, StatusBarIcon icon) {
+        if (icon == null) {
+            removeAllIconsForExternalSlot(slot);
+            return;
+        }
+        String slotName = createExternalSlotName(slot);
         StatusBarIconHolder holder = StatusBarIconHolder.fromIcon(icon);
         setIcon(slotName, holder);
     }
@@ -417,14 +425,6 @@ public class StatusBarIconControllerImpl implements Tunable,
         }
     }
 
-    // CommandQueue.Callbacks override
-    // TODO(b/265307726): Pull out the CommandQueue callbacks into a member variable to
-    //  differentiate between those callback methods and StatusBarIconController methods.
-    @Override
-    public void removeIcon(String slot) {
-        removeAllIconsForExternalSlot(slot);
-    }
-
     /** */
     @Override
     public void removeIcon(String slot, int tag) {
@@ -444,8 +444,7 @@ public class StatusBarIconControllerImpl implements Tunable,
         mIconGroups.forEach(l -> l.onRemoveIcon(viewIndex));
     }
 
-    @Override
+    private void removeAllIconsForExternalSlot(String slotName) {
-    public void removeAllIconsForExternalSlot(String slotName) {
         removeAllIconsForSlot(createExternalSlotName(slotName));
     }
 

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImplTest.kt

@@ -20,13 +20,17 @@ import android.os.UserHandle
 import androidx.test.filters.SmallTest
 import com.android.internal.statusbar.StatusBarIcon
 import com.android.systemui.SysuiTestCase
+import com.android.systemui.statusbar.CommandQueue
 import com.android.systemui.statusbar.phone.StatusBarIconController.TAG_PRIMARY
 import com.android.systemui.statusbar.phone.StatusBarIconControllerImpl.EXTERNAL_SLOT_SUFFIX
+import com.android.systemui.util.mockito.kotlinArgumentCaptor
 import com.android.systemui.util.mockito.mock
 import com.google.common.truth.Truth.assertThat
 import org.junit.Before
 import org.junit.Test
+import org.mockito.Mock
 import org.mockito.Mockito.verify
+import org.mockito.MockitoAnnotations
 
 @SmallTest
 class StatusBarIconControllerImplTest : SysuiTestCase() {
@@ -34,15 +38,19 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
     private lateinit var underTest: StatusBarIconControllerImpl
 
     private lateinit var iconList: StatusBarIconList
+    private lateinit var commandQueueCallbacks: CommandQueue.Callbacks
     private val iconGroup: StatusBarIconController.IconManager = mock()
 
+    @Mock private lateinit var commandQueue: CommandQueue
+
     @Before
     fun setUp() {
+        MockitoAnnotations.initMocks(this)
         iconList = StatusBarIconList(arrayOf())
         underTest =
             StatusBarIconControllerImpl(
                 context,
-                mock(),
+                commandQueue,
                 mock(),
                 mock(),
                 mock(),
@@ -51,11 +59,14 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
                 mock(),
             )
         underTest.addIconGroup(iconGroup)
+        val commandQueueCallbacksCaptor = kotlinArgumentCaptor<CommandQueue.Callbacks>()
+        verify(commandQueue).addCallback(commandQueueCallbacksCaptor.capture())
+        commandQueueCallbacks = commandQueueCallbacksCaptor.value
     }
 
     /** Regression test for b/255428281. */
     @Test
-    fun internalAndExternalIconWithSameName_bothDisplayed() {
+    fun internalAndExternalIconWithSameName_externalFromTile_bothDisplayed() {
         val slotName = "mute"
 
         // Internal
@@ -71,7 +82,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
                 /* number= */ 0,
                 "contentDescription",
             )
-        underTest.setIcon(slotName, externalIcon)
+        underTest.setIconFromTile(slotName, externalIcon)
 
         assertThat(iconList.slots).hasSize(2)
         // Whichever was added last comes first
@@ -83,17 +94,45 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
 
     /** Regression test for b/255428281. */
     @Test
-    fun internalAndExternalIconWithSameName_externalRemoved_viaRemoveIcon_internalStays() {
+    fun internalAndExternalIconWithSameName_externalFromCommandQueue_bothDisplayed() {
         val slotName = "mute"
 
         // Internal
         underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
 
         // External
-        underTest.setIcon(slotName, createExternalIcon())
+        val externalIcon =
+            StatusBarIcon(
+                "external.package",
+                UserHandle.ALL,
+                /* iconId= */ 2,
+                /* iconLevel= */ 0,
+                /* number= */ 0,
+                "contentDescription",
+            )
+        commandQueueCallbacks.setIcon(slotName, externalIcon)
 
-        // WHEN the external icon is removed via #removeIcon
+        assertThat(iconList.slots).hasSize(2)
-        underTest.removeIcon(slotName)
+        // Whichever was added last comes first
+        assertThat(iconList.slots[0].name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
+        assertThat(iconList.slots[1].name).isEqualTo(slotName)
+        assertThat(iconList.slots[0].hasIconsInSlot()).isTrue()
+        assertThat(iconList.slots[1].hasIconsInSlot()).isTrue()
+    }
+
+    /** Regression test for b/255428281. */
+    @Test
+    fun internalAndExternalIconWithSameName_externalRemoved_fromCommandQueue_internalStays() {
+        val slotName = "mute"
+
+        // Internal
+        underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
+
+        // External
+        commandQueueCallbacks.setIcon(slotName, createExternalIcon())
+
+        // WHEN the external icon is removed via CommandQueue.Callbacks#removeIcon
+        commandQueueCallbacks.removeIcon(slotName)
 
         // THEN the external icon is removed but the internal icon remains
         // Note: [StatusBarIconList] never removes slots from its list, it just sets the holder for
@@ -109,17 +148,17 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
 
     /** Regression test for b/255428281. */
     @Test
-    fun internalAndExternalIconWithSameName_externalRemoved_viaRemoveAll_internalStays() {
+    fun internalAndExternalIconWithSameName_externalRemoved_fromTileRemove_internalStays() {
         val slotName = "mute"
 
         // Internal
         underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
 
         // External
-        underTest.setIcon(slotName, createExternalIcon())
+        underTest.setIconFromTile(slotName, createExternalIcon())
 
-        // WHEN the external icon is removed via #removeAllIconsForExternalSlot
+        // WHEN the external icon is removed via #removeIconForTile
-        underTest.removeAllIconsForExternalSlot(slotName)
+        underTest.removeIconForTile(slotName)
 
         // THEN the external icon is removed but the internal icon remains
         assertThat(iconList.slots).hasSize(2)
@@ -133,17 +172,17 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
 
     /** Regression test for b/255428281. */
     @Test
-    fun internalAndExternalIconWithSameName_externalRemoved_viaSetNull_internalStays() {
+    fun internalAndExternalIconWithSameName_externalRemoved_fromTileSetNull_internalStays() {
         val slotName = "mute"
 
         // Internal
         underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
 
         // External
-        underTest.setIcon(slotName, createExternalIcon())
+        underTest.setIconFromTile(slotName, createExternalIcon())
 
-        // WHEN the external icon is removed via a #setIcon(null)
+        // WHEN the external icon is removed via a #setIconFromTile(null)
-        underTest.setIcon(slotName, /* icon= */ null)
+        underTest.setIconFromTile(slotName, /* icon= */ null)
 
         // THEN the external icon is removed but the internal icon remains
         assertThat(iconList.slots).hasSize(2)
@@ -164,12 +203,12 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
         underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
 
         // External
-        underTest.setIcon(slotName, createExternalIcon())
+        underTest.setIconFromTile(slotName, createExternalIcon())
 
         // WHEN the internal icon is removed via #removeIcon
         underTest.removeIcon(slotName, /* tag= */ 0)
 
-        // THEN the external icon is removed but the internal icon remains
+        // THEN the internal icon is removed but the external icon remains
         assertThat(iconList.slots).hasSize(2)
         assertThat(iconList.slots[0].name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
         assertThat(iconList.slots[1].name).isEqualTo(slotName)
@@ -188,12 +227,12 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
         underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
 
         // External
-        underTest.setIcon(slotName, createExternalIcon())
+        underTest.setIconFromTile(slotName, createExternalIcon())
 
         // WHEN the internal icon is removed via #removeAllIconsForSlot
         underTest.removeAllIconsForSlot(slotName)
 
-        // THEN the external icon is removed but the internal icon remains
+        // THEN the internal icon is removed but the external icon remains
         assertThat(iconList.slots).hasSize(2)
         assertThat(iconList.slots[0].name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
         assertThat(iconList.slots[1].name).isEqualTo(slotName)
@@ -221,7 +260,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
                 /* number= */ 0,
                 "externalDescription",
             )
-        underTest.setIcon(slotName, startingExternalIcon)
+        underTest.setIconFromTile(slotName, startingExternalIcon)
 
         // WHEN the internal icon is updated
         underTest.setIcon(slotName, /* resourceId= */ 11, "newContentDescription")
@@ -243,7 +282,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
 
     /** Regression test for b/255428281. */
     @Test
-    fun internalAndExternalIconWithSameName_externalUpdatedIndependently() {
+    fun internalAndExternalIconWithSameName_fromTile_externalUpdatedIndependently() {
         val slotName = "mute"
 
         // Internal
@@ -259,7 +298,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
                 /* number= */ 0,
                 "externalDescription",
             )
-        underTest.setIcon(slotName, startingExternalIcon)
+        underTest.setIconFromTile(slotName, startingExternalIcon)
 
         // WHEN the external icon is updated
         val newExternalIcon =
@@ -271,7 +310,54 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
                 /* number= */ 0,
                 "newExternalDescription",
             )
-        underTest.setIcon(slotName, newExternalIcon)
+        underTest.setIconFromTile(slotName, newExternalIcon)
+
+        // THEN only the external slot gets the updates
+        val externalSlot = iconList.slots[0]
+        val externalHolder = externalSlot.getHolderForTag(TAG_PRIMARY)!!
+        assertThat(externalSlot.name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
+        assertThat(externalHolder.icon!!.contentDescription).isEqualTo("newExternalDescription")
+        assertThat(externalHolder.icon!!.icon.resId).isEqualTo(21)
+
+        // And the internal slot has its own values
+        val internalSlot = iconList.slots[1]
+        val internalHolder = internalSlot.getHolderForTag(TAG_PRIMARY)!!
+        assertThat(internalSlot.name).isEqualTo(slotName)
+        assertThat(internalHolder.icon!!.contentDescription).isEqualTo("contentDescription")
+        assertThat(internalHolder.icon!!.icon.resId).isEqualTo(10)
+    }
+
+    /** Regression test for b/255428281. */
+    @Test
+    fun internalAndExternalIconWithSameName_fromCommandQueue_externalUpdatedIndependently() {
+        val slotName = "mute"
+
+        // Internal
+        underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
+
+        // External
+        val startingExternalIcon =
+            StatusBarIcon(
+                "external.package",
+                UserHandle.ALL,
+                /* iconId= */ 20,
+                /* iconLevel= */ 0,
+                /* number= */ 0,
+                "externalDescription",
+            )
+        commandQueueCallbacks.setIcon(slotName, startingExternalIcon)
+
+        // WHEN the external icon is updated
+        val newExternalIcon =
+            StatusBarIcon(
+                "external.package",
+                UserHandle.ALL,
+                /* iconId= */ 21,
+                /* iconLevel= */ 0,
+                /* number= */ 0,
+                "newExternalDescription",
+            )
+        commandQueueCallbacks.setIcon(slotName, newExternalIcon)
 
         // THEN only the external slot gets the updates
         val externalSlot = iconList.slots[0]
@@ -289,8 +375,16 @@ class StatusBarIconControllerImplTest : SysuiTestCase() {
     }
 
     @Test
-    fun externalSlot_alreadyEndsWithSuffix_suffixNotAddedTwice() {
+    fun externalSlot_fromTile_alreadyEndsWithSuffix_suffixNotAddedTwice() {
-        underTest.setIcon("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon())
+        underTest.setIconFromTile("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon())
+
+        assertThat(iconList.slots).hasSize(1)
+        assertThat(iconList.slots[0].name).isEqualTo("myslot$EXTERNAL_SLOT_SUFFIX")
+    }
+
+    @Test
+    fun externalSlot_fromCommandQueue_alreadyEndsWithSuffix_suffixNotAddedTwice() {
+        commandQueueCallbacks.setIcon("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon())
 
         assertThat(iconList.slots).hasSize(1)
         assertThat(iconList.slots[0].name).isEqualTo("myslot$EXTERNAL_SLOT_SUFFIX")

packages/SystemUI/tests/utils/src/com/android/systemui/utils/leaks/FakeStatusBarIconController.java

@@ -47,7 +47,12 @@ public class FakeStatusBarIconController extends BaseLeakChecker<IconManager>
     }
 
     @Override
-    public void setExternalIcon(String slot) {
+    public void setIconFromTile(String slot, StatusBarIcon icon) {
+
+    }
+
+    @Override
+    public void removeIconForTile(String slot) {
 
     }
 
@@ -56,11 +61,6 @@ public class FakeStatusBarIconController extends BaseLeakChecker<IconManager>
 
     }
 
-    @Override
-    public void setIcon(String slot, StatusBarIcon icon) {
-
-    }
-
     @Override
     public void setWifiIcon(String slot, WifiIconState state) {
     }
@@ -97,10 +97,6 @@ public class FakeStatusBarIconController extends BaseLeakChecker<IconManager>
     public void removeAllIconsForSlot(String slot) {
     }
 
-    @Override
-    public void removeAllIconsForExternalSlot(String slot) {
-    }
-
     @Override
     public void setIconAccessibilityLiveRegion(String slot, int mode) {
     }