Merge "[Status Bar] Append "__external" to external icons." into tm-qpr-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
9cf44a2aa0
@@ -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));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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";
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -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) {
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user