[Status bar] Clarify the API between TileServices and status bar icons. This CL re-names the StatusBarIconController APIs that are only used by TileServices to specifically have "tile" in them. Then, it's obvious why the implementation of the method considers those icons external instead of internal. This also fixes two bugs with the tile status bar icons. See first two linked bug fixes. Fixes: 273536393 Fixes: 273540833 Bug: 265307726 Test: Add tile service that has status icon -> verify icon is added (see b/265307726 for demo) Test: Remove tile service that has status icon -> verify icon is rmeoved (see b/265307726 for demo) Test: verify tile status icon is correct size Test: atest StatusBarIconControllerImplTest Change-Id: Ieac39be633e85ce5076272f9afc12f953839bedc
diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java b/packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java index adc7165..91ecaea 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java +++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java
@@ -132,9 +132,8 @@ mServices.remove(tile); mTokenMap.remove(service.getToken()); mTiles.remove(tile.getComponent()); - final String slot = tile.getComponent().getClassName(); - // TileServices doesn't know how to add more than 1 icon per slot, so remove all - mMainHandler.post(() -> mStatusBarIconController.removeAllIconsForSlot(slot)); + final String slot = getStatusBarIconSlotName(tile.getComponent()); + mMainHandler.post(() -> mStatusBarIconController.removeIconForTile(slot)); } } @@ -308,12 +307,11 @@ ? 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; - iconController.setIcon(componentName.getClassName(), statusIcon); - iconController.setExternalIcon(componentName.getClassName()); + mStatusBarIconController.setIconFromTile(slot, statusIcon); } }); } @@ -373,6 +371,12 @@ 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) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java index 04cc8ce..30d2295 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java
@@ -81,28 +81,22 @@ 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 - * much more clearly distinct from that method. + * If the icon is null, the icon will be removed. */ - 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 - * {@link #setIcon(String, StatusBarIcon)} method. + * TODO(b/265307726): Re-name this to `setInternalIcon`. */ 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 @@ */ void removeIcon(String slot, int tag); - /** */ - void removeAllIconsForSlot(String slot); - /** - * Removes all the icons for the given slot. - * - * Only use this for icons that have come from **an external process**. + * TODO(b/265307726): Re-name this to `removeAllIconsForInternalSlot`. */ - 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 @@ 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);
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java index 0422d69..3a18423 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java
@@ -32,7 +32,6 @@ 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; @@ -350,33 +349,6 @@ } } - // TODO(b/265307726): Determine why we have two [setExternalIcon] methods and why they're - // different. - @Override - 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 - public void setIcon(String slot, StatusBarIcon icon) { - setExternalIcon(slot, icon); - } - - private void setExternalIcon(String slot, StatusBarIcon icon) { - String slotName = createExternalSlotName(slot); - if (icon == null) { - removeAllIconsForSlot(slotName); - return; - } - - StatusBarIconHolder holder = StatusBarIconHolder.fromIcon(icon); - setIcon(slotName, holder); - } - private final CommandQueue.Callbacks mCommandQueueCallbacks = new CommandQueue.Callbacks() { @Override public void setIcon(String slot, StatusBarIcon icon) { @@ -390,6 +362,26 @@ } }; + @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); + } + private void setIcon(String slot, @NonNull StatusBarIconHolder holder) { boolean isNew = mStatusBarIconList.getIconHolder(slot, holder.getTag()) == null; mStatusBarIconList.setIcon(slot, holder); @@ -452,8 +444,7 @@ mIconGroups.forEach(l -> l.onRemoveIcon(viewIndex)); } - @Override - public void removeAllIconsForExternalSlot(String slotName) { + private void removeAllIconsForExternalSlot(String slotName) { removeAllIconsForSlot(createExternalSlotName(slotName)); }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImplTest.kt index 5bde883..08e89fb 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImplTest.kt
@@ -66,7 +66,7 @@ /** Regression test for b/255428281. */ @Test - fun internalAndExternalIconWithSameName_bothDisplayed() { + fun internalAndExternalIconWithSameName_externalFromTile_bothDisplayed() { val slotName = "mute" // Internal @@ -82,7 +82,7 @@ /* number= */ 0, "contentDescription", ) - underTest.setIcon(slotName, externalIcon) + underTest.setIconFromTile(slotName, externalIcon) assertThat(iconList.slots).hasSize(2) // Whichever was added last comes first @@ -148,17 +148,17 @@ /** 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 - underTest.removeAllIconsForExternalSlot(slotName) + // WHEN the external icon is removed via #removeIconForTile + underTest.removeIconForTile(slotName) // THEN the external icon is removed but the internal icon remains assertThat(iconList.slots).hasSize(2) @@ -172,17 +172,17 @@ /** 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) - underTest.setIcon(slotName, /* icon= */ null) + // WHEN the external icon is removed via a #setIconFromTile(null) + underTest.setIconFromTile(slotName, /* icon= */ null) // THEN the external icon is removed but the internal icon remains assertThat(iconList.slots).hasSize(2) @@ -203,12 +203,12 @@ 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) @@ -227,12 +227,12 @@ 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) @@ -260,7 +260,7 @@ /* number= */ 0, "externalDescription", ) - underTest.setIcon(slotName, startingExternalIcon) + underTest.setIconFromTile(slotName, startingExternalIcon) // WHEN the internal icon is updated underTest.setIcon(slotName, /* resourceId= */ 11, "newContentDescription") @@ -282,7 +282,7 @@ /** Regression test for b/255428281. */ @Test - fun internalAndExternalIconWithSameName_externalUpdatedIndependently() { + fun internalAndExternalIconWithSameName_fromTile_externalUpdatedIndependently() { val slotName = "mute" // Internal @@ -298,7 +298,7 @@ /* number= */ 0, "externalDescription", ) - underTest.setIcon(slotName, startingExternalIcon) + underTest.setIconFromTile(slotName, startingExternalIcon) // WHEN the external icon is updated val newExternalIcon = @@ -310,7 +310,7 @@ /* number= */ 0, "newExternalDescription", ) - underTest.setIcon(slotName, newExternalIcon) + underTest.setIconFromTile(slotName, newExternalIcon) // THEN only the external slot gets the updates val externalSlot = iconList.slots[0] @@ -375,8 +375,8 @@ } @Test - fun externalSlot_alreadyEndsWithSuffix_suffixNotAddedTwice() { - underTest.setIcon("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon()) + fun externalSlot_fromTile_alreadyEndsWithSuffix_suffixNotAddedTwice() { + underTest.setIconFromTile("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon()) assertThat(iconList.slots).hasSize(1) assertThat(iconList.slots[0].name).isEqualTo("myslot$EXTERNAL_SLOT_SUFFIX")
diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/utils/leaks/FakeStatusBarIconController.java b/packages/SystemUI/tests/utils/src/com/android/systemui/utils/leaks/FakeStatusBarIconController.java index 926c6c5..c664c99 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/utils/leaks/FakeStatusBarIconController.java +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/utils/leaks/FakeStatusBarIconController.java
@@ -47,7 +47,12 @@ } @Override - public void setExternalIcon(String slot) { + public void setIconFromTile(String slot, StatusBarIcon icon) { + + } + + @Override + public void removeIconForTile(String slot) { } @@ -57,11 +62,6 @@ } @Override - public void setIcon(String slot, StatusBarIcon icon) { - - } - - @Override public void setWifiIcon(String slot, WifiIconState state) { } @@ -98,10 +98,6 @@ } @Override - public void removeAllIconsForExternalSlot(String slot) { - } - - @Override public void setIconAccessibilityLiveRegion(String slot, int mode) { }