Revert "[Status Bar] Append "__external" to external icons." (1add163b) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java

+1 −1

Original line number	Diff line number	Diff line
		@@ -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()
		.removeAllIconsForExternalSlot(slot));
		.removeAllIconsForSlot(slot));
		}
		}

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java

+3 −29

Original line number	Diff line number	Diff line
		@@ -79,30 +79,12 @@ 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);

		@@ -151,17 +133,9 @@ 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";

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java

+6 −32

Original line number	Diff line number	Diff line
		@@ -28,8 +28,6 @@ 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;
		@@ -65,10 +63,6 @@ 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<>();
		@@ -352,26 +346,21 @@ public class StatusBarIconControllerImpl implements Tunable,

		@Override
		public void setExternalIcon(String slot) {
		String slotName = createExternalSlotName(slot);
		int viewIndex = mStatusBarIconList.getViewIndex(slotName, 0);
		int viewIndex = mStatusBarIconList.getViewIndex(slot, 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
		//TODO: remove this (used in command queue and for 3rd party tiles?)
		public void setIcon(String slot, StatusBarIcon icon) {
		String slotName = createExternalSlotName(slot);
		if (icon == null) {
		removeAllIconsForSlot(slotName);
		removeAllIconsForSlot(slot);
		return;
		}

		StatusBarIconHolder holder = StatusBarIconHolder.fromIcon(icon);
		setIcon(slotName, holder);
		setIcon(slot, holder);
		}

		private void setIcon(String slot, @NonNull StatusBarIconHolder holder) {
		@@ -417,12 +406,10 @@ 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);
		removeAllIconsForSlot(slot);
		}

		/** */
		@@ -436,11 +423,6 @@ 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) {
		@@ -524,12 +506,4 @@ 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;
		}
		}
		}

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImplTest.kt

deleted100644 → 0

+0 −309

Original line number	Diff line number	Diff line
		/*
		* 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",
		)
		}
		}

packages/SystemUI/tests/utils/src/com/android/systemui/utils/leaks/FakeStatusBarIconController.java

+0 −4

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