Merge "[Status Bar] Append "__external" to external icons." into tm-qpr-dev

This commit is contained in:
TreeHugger Robot
2023-01-13 18:29:30 +00:00
committed by Android (Google) Code Review
5 changed files with 375 additions and 10 deletions

View File

@@ -132,7 +132,7 @@ public class TileServices extends IQSService.Stub {
final String slot = tile.getComponent().getClassName();
// TileServices doesn't know how to add more than 1 icon per slot, so remove all
mMainHandler.post(() -> mHost.getIconController()
.removeAllIconsForSlot(slot));
.removeAllIconsForExternalSlot(slot));
}
}

View File

@@ -79,12 +79,30 @@ public interface StatusBarIconController {
/** Refresh the state of an IconManager by recreating the views */
void refreshIconGroup(IconManager iconManager);
/** */
/**
* Adds or updates an icon for a given slot for a **tile service icon**.
*
* TODO(b/265307726): Merge with {@link #setIcon(String, StatusBarIcon)} or make this method
* much more clearly distinct from that method.
*/
void setExternalIcon(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.
*/
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);
@@ -133,9 +151,17 @@ public interface StatusBarIconController {
* TAG_PRIMARY to refer to the first icon at a given slot.
*/
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**.
*/
void removeAllIconsForExternalSlot(String slot);
// TODO: See if we can rename this tunable name.
String ICON_HIDE_LIST = "icon_blacklist";

View File

@@ -28,6 +28,8 @@ import android.util.ArraySet;
import android.util.Log;
import android.view.ViewGroup;
import androidx.annotation.VisibleForTesting;
import com.android.internal.statusbar.StatusBarIcon;
import com.android.systemui.Dumpable;
import com.android.systemui.R;
@@ -63,6 +65,10 @@ public class StatusBarIconControllerImpl implements Tunable,
ConfigurationListener, Dumpable, CommandQueue.Callbacks, StatusBarIconController, DemoMode {
private static final String TAG = "StatusBarIconController";
// Use this suffix to prevent external icon slot names from unintentionally overriding our
// internal, system-level slot names. See b/255428281.
@VisibleForTesting
protected static final String EXTERNAL_SLOT_SUFFIX = "__external";
private final StatusBarIconList mStatusBarIconList;
private final ArrayList<IconManager> mIconGroups = new ArrayList<>();
@@ -346,21 +352,26 @@ public class StatusBarIconControllerImpl implements Tunable,
@Override
public void setExternalIcon(String slot) {
int viewIndex = mStatusBarIconList.getViewIndex(slot, 0);
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));
}
//TODO: remove this (used in command queue and for 3rd party tiles?)
// 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(slot);
removeAllIconsForSlot(slotName);
return;
}
StatusBarIconHolder holder = StatusBarIconHolder.fromIcon(icon);
setIcon(slot, holder);
setIcon(slotName, holder);
}
private void setIcon(String slot, @NonNull StatusBarIconHolder holder) {
@@ -406,10 +417,12 @@ 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) {
removeAllIconsForSlot(slot);
removeAllIconsForExternalSlot(slot);
}
/** */
@@ -423,6 +436,11 @@ public class StatusBarIconControllerImpl implements Tunable,
mIconGroups.forEach(l -> l.onRemoveIcon(viewIndex));
}
@Override
public void removeAllIconsForExternalSlot(String slotName) {
removeAllIconsForSlot(createExternalSlotName(slotName));
}
/** */
@Override
public void removeAllIconsForSlot(String slotName) {
@@ -506,4 +524,12 @@ public class StatusBarIconControllerImpl implements Tunable,
public void onDensityOrFontScaleChanged() {
refreshIconGroups();
}
private String createExternalSlotName(String slot) {
if (slot.endsWith(EXTERNAL_SLOT_SUFFIX)) {
return slot;
} else {
return slot + EXTERNAL_SLOT_SUFFIX;
}
}
}

View File

@@ -0,0 +1,309 @@
/*
* Copyright (C) 2023 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.systemui.statusbar.phone
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.phone.StatusBarIconController.TAG_PRIMARY
import com.android.systemui.statusbar.phone.StatusBarIconControllerImpl.EXTERNAL_SLOT_SUFFIX
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.Mockito.verify
@SmallTest
class StatusBarIconControllerImplTest : SysuiTestCase() {
private lateinit var underTest: StatusBarIconControllerImpl
private lateinit var iconList: StatusBarIconList
private val iconGroup: StatusBarIconController.IconManager = mock()
@Before
fun setUp() {
iconList = StatusBarIconList(arrayOf())
underTest =
StatusBarIconControllerImpl(
context,
mock(),
mock(),
mock(),
mock(),
mock(),
iconList,
mock(),
)
underTest.addIconGroup(iconGroup)
}
/** Regression test for b/255428281. */
@Test
fun internalAndExternalIconWithSameName_bothDisplayed() {
val slotName = "mute"
// Internal
underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
// External
val externalIcon =
StatusBarIcon(
"external.package",
UserHandle.ALL,
/* iconId= */ 2,
/* iconLevel= */ 0,
/* number= */ 0,
"contentDescription",
)
underTest.setIcon(slotName, externalIcon)
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_viaRemoveIcon_internalStays() {
val slotName = "mute"
// Internal
underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
// External
underTest.setIcon(slotName, createExternalIcon())
// WHEN the external icon is removed via #removeIcon
underTest.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
// the slot to null when an icon is removed.
assertThat(iconList.slots).hasSize(2)
assertThat(iconList.slots[0].name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
assertThat(iconList.slots[1].name).isEqualTo(slotName)
assertThat(iconList.slots[0].hasIconsInSlot()).isFalse() // Indicates removal
assertThat(iconList.slots[1].hasIconsInSlot()).isTrue()
verify(iconGroup).onRemoveIcon(0)
}
/** Regression test for b/255428281. */
@Test
fun internalAndExternalIconWithSameName_externalRemoved_viaRemoveAll_internalStays() {
val slotName = "mute"
// Internal
underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
// External
underTest.setIcon(slotName, createExternalIcon())
// WHEN the external icon is removed via #removeAllIconsForExternalSlot
underTest.removeAllIconsForExternalSlot(slotName)
// THEN the external icon is removed but the internal icon remains
assertThat(iconList.slots).hasSize(2)
assertThat(iconList.slots[0].name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
assertThat(iconList.slots[1].name).isEqualTo(slotName)
assertThat(iconList.slots[0].hasIconsInSlot()).isFalse() // Indicates removal
assertThat(iconList.slots[1].hasIconsInSlot()).isTrue()
verify(iconGroup).onRemoveIcon(0)
}
/** Regression test for b/255428281. */
@Test
fun internalAndExternalIconWithSameName_externalRemoved_viaSetNull_internalStays() {
val slotName = "mute"
// Internal
underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
// External
underTest.setIcon(slotName, createExternalIcon())
// WHEN the external icon is removed via a #setIcon(null)
underTest.setIcon(slotName, /* icon= */ null)
// THEN the external icon is removed but the internal icon remains
assertThat(iconList.slots).hasSize(2)
assertThat(iconList.slots[0].name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
assertThat(iconList.slots[1].name).isEqualTo(slotName)
assertThat(iconList.slots[0].hasIconsInSlot()).isFalse() // Indicates removal
assertThat(iconList.slots[1].hasIconsInSlot()).isTrue()
verify(iconGroup).onRemoveIcon(0)
}
/** Regression test for b/255428281. */
@Test
fun internalAndExternalIconWithSameName_internalRemoved_viaRemove_externalStays() {
val slotName = "mute"
// Internal
underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
// External
underTest.setIcon(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
assertThat(iconList.slots).hasSize(2)
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()).isFalse() // Indicates removal
verify(iconGroup).onRemoveIcon(1)
}
/** Regression test for b/255428281. */
@Test
fun internalAndExternalIconWithSameName_internalRemoved_viaRemoveAll_externalStays() {
val slotName = "mute"
// Internal
underTest.setIcon(slotName, /* resourceId= */ 10, "contentDescription")
// External
underTest.setIcon(slotName, createExternalIcon())
// WHEN the internal icon is removed via #removeAllIconsForSlot
underTest.removeAllIconsForSlot(slotName)
// THEN the external icon is removed but the internal icon remains
assertThat(iconList.slots).hasSize(2)
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()).isFalse() // Indicates removal
verify(iconGroup).onRemoveIcon(1)
}
/** Regression test for b/255428281. */
@Test
fun internalAndExternalIconWithSameName_internalUpdatedIndependently() {
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",
)
underTest.setIcon(slotName, startingExternalIcon)
// WHEN the internal icon is updated
underTest.setIcon(slotName, /* resourceId= */ 11, "newContentDescription")
// THEN only the internal slot gets the updates
val internalSlot = iconList.slots[1]
val internalHolder = internalSlot.getHolderForTag(TAG_PRIMARY)!!
assertThat(internalSlot.name).isEqualTo(slotName)
assertThat(internalHolder.icon!!.contentDescription).isEqualTo("newContentDescription")
assertThat(internalHolder.icon!!.icon.resId).isEqualTo(11)
// And the external slot has its own values
val externalSlot = iconList.slots[0]
val externalHolder = externalSlot.getHolderForTag(TAG_PRIMARY)!!
assertThat(externalSlot.name).isEqualTo(slotName + EXTERNAL_SLOT_SUFFIX)
assertThat(externalHolder.icon!!.contentDescription).isEqualTo("externalDescription")
assertThat(externalHolder.icon!!.icon.resId).isEqualTo(20)
}
/** Regression test for b/255428281. */
@Test
fun internalAndExternalIconWithSameName_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",
)
underTest.setIcon(slotName, startingExternalIcon)
// WHEN the external icon is updated
val newExternalIcon =
StatusBarIcon(
"external.package",
UserHandle.ALL,
/* iconId= */ 21,
/* iconLevel= */ 0,
/* number= */ 0,
"newExternalDescription",
)
underTest.setIcon(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)
}
@Test
fun externalSlot_alreadyEndsWithSuffix_suffixNotAddedTwice() {
underTest.setIcon("myslot$EXTERNAL_SLOT_SUFFIX", createExternalIcon())
assertThat(iconList.slots).hasSize(1)
assertThat(iconList.slots[0].name).isEqualTo("myslot$EXTERNAL_SLOT_SUFFIX")
}
private fun createExternalIcon(): StatusBarIcon {
return StatusBarIcon(
"external.package",
UserHandle.ALL,
/* iconId= */ 2,
/* iconLevel= */ 0,
/* number= */ 0,
"contentDescription",
)
}
}

View File

@@ -97,6 +97,10 @@ 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) {
}