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 adc71657e6803..91ecaea02c476 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 @@ public class TileServices extends IQSService.Stub { 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 @@ 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; - iconController.setIcon(componentName.getClassName(), statusIcon); - iconController.setExternalIcon(componentName.getClassName()); + mStatusBarIconController.setIconFromTile(slot, statusIcon); } }); } @@ -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) { 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 04cc8ce792d12..30d2295206d84 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 @@ 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 - * 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 @@ public interface StatusBarIconController { */ 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 @@ 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); 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 0422d694b70d4..3a184239ac436 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 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; @@ -350,33 +349,6 @@ public class StatusBarIconControllerImpl implements Tunable, } } - // 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 @@ public class StatusBarIconControllerImpl implements Tunable, } }; + @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 @@ public class StatusBarIconControllerImpl implements Tunable, 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 5bde88300f960..08e89fbef486d 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 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { /** Regression test for b/255428281. */ @Test - fun internalAndExternalIconWithSameName_bothDisplayed() { + fun internalAndExternalIconWithSameName_externalFromTile_bothDisplayed() { val slotName = "mute" // Internal @@ -82,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 @@ -148,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 - 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 @@ 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) - 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 @@ 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) @@ -227,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) @@ -260,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") @@ -282,7 +282,7 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { /** Regression test for b/255428281. */ @Test - fun internalAndExternalIconWithSameName_externalUpdatedIndependently() { + fun internalAndExternalIconWithSameName_fromTile_externalUpdatedIndependently() { val slotName = "mute" // Internal @@ -298,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 = @@ -310,7 +310,7 @@ 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] @@ -375,8 +375,8 @@ class StatusBarIconControllerImplTest : SysuiTestCase() { } @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 926c6c56a8620..c664c99cf2a77 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 @@ public class FakeStatusBarIconController extends BaseLeakChecker } @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 } - @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 public void removeAllIconsForSlot(String slot) { } - @Override - public void removeAllIconsForExternalSlot(String slot) { - } - @Override public void setIconAccessibilityLiveRegion(String slot, int mode) { }