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 2083cc7b167e1..5e4f531817068 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java +++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java @@ -136,9 +136,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)); } } @@ -312,12 +311,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); } }); } @@ -377,6 +375,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 0727c5af0dc78..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; @@ -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 - 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); - if (icon == null) { - removeAllIconsForSlot(slotName); - return; + private final CommandQueue.Callbacks mCommandQueueCallbacks = new CommandQueue.Callbacks() { + @Override + public void setIcon(String slot, StatusBarIcon icon) { + // Icons that come from CommandQueue are from external services. + setExternalIcon(slot, icon); } + @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 - 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 3bc288a2f8234..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 @@ -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() + 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 - underTest.removeIcon(slotName) + assertThat(iconList.slots).hasSize(2) + // 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 - 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) @@ -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) - 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) @@ -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() { - 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") + } + + @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") 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) { }