Merge "[Sb refactor] Ignore old pipeline callbacks to removeIcon*" into... (b14850bd) · Commits · e / os / android_frameworks_base

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

+23 −2

Original line number	Diff line number	Diff line
		@@ -163,7 +163,8 @@ public class StatusBarIconControllerImpl implements Tunable,
		for (int i = currentSlots.size() - 1; i >= 0; i--) {
		Slot s = currentSlots.get(i);
		slotsToReAdd.put(s, s.getHolderList());
		removeAllIconsForSlot(s.getName());
		// Don't force here because the new pipeline properly handles the tuner settings
		removeAllIconsForSlot(s.getName(), /* force */ false);
		}

		// Add them all back
		@@ -285,7 +286,7 @@ public class StatusBarIconControllerImpl implements Tunable,
		// Because of the way we cache the icon holders, we need to remove everything any time
		// we get a new set of subscriptions. This might change in the future, but is required
		// to support demo mode for now
		removeAllIconsForSlot(slotName);
		removeAllIconsForSlot(slotName, /* force */ true);

		Collections.reverse(subIds);

		@@ -428,6 +429,14 @@ public class StatusBarIconControllerImpl implements Tunable,
		/** */
		@Override
		public void removeIcon(String slot, int tag) {
		// If the new pipeline is on for this icon, don't allow removal, since the new pipeline
		// will never call this method
		if (mStatusBarPipelineFlags.isIconControlledByFlags(slot)) {
		Log.i(TAG, "Ignoring removal of (" + slot + "). "
		+ "It should be controlled elsewhere");
		return;
		}

		if (mStatusBarIconList.getIconHolder(slot, tag) == null) {
		return;
		}
		@@ -444,6 +453,18 @@ public class StatusBarIconControllerImpl implements Tunable,
		/** */
		@Override
		public void removeAllIconsForSlot(String slotName) {
		removeAllIconsForSlot(slotName, /* force */ false);
		}

		private void removeAllIconsForSlot(String slotName, Boolean force) {
		// If the new pipeline is on for this icon, don't allow removal, since the new pipeline
		// will never call this method
		if (!force && mStatusBarPipelineFlags.isIconControlledByFlags(slotName)) {
		Log.i(TAG, "Ignoring removal of (" + slotName + "). "
		+ "It should be controlled elsewhere");
		return;
		}

		Slot slot = mStatusBarIconList.getSlot(slotName);
		if (!slot.hasIconsInSlot()) {
		return;

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/StatusBarPipelineFlags.kt

+19 −1

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.systemui.statusbar.pipeline

		import android.content.Context
		import com.android.systemui.dagger.SysUISingleton
		import com.android.systemui.flags.FeatureFlags
		import com.android.systemui.flags.Flags
		@@ -23,7 +24,15 @@ import javax.inject.Inject

		/** All flagging methods related to the new status bar pipeline (see b/238425913). */
		@SysUISingleton
		class StatusBarPipelineFlags @Inject constructor(private val featureFlags: FeatureFlags) {
		class StatusBarPipelineFlags
		@Inject
		constructor(
		context: Context,
		private val featureFlags: FeatureFlags,
		) {
		private val mobileSlot = context.getString(com.android.internal.R.string.status_bar_mobile)
		private val wifiSlot = context.getString(com.android.internal.R.string.status_bar_wifi)

		/** True if we should display the mobile icons using the new status bar data pipeline. */
		fun useNewMobileIcons(): Boolean = featureFlags.isEnabled(Flags.NEW_STATUS_BAR_MOBILE_ICONS)

		@@ -54,4 +63,13 @@ class StatusBarPipelineFlags @Inject constructor(private val featureFlags: Featu
		*/
		fun useDebugColoring(): Boolean =
		featureFlags.isEnabled(Flags.NEW_STATUS_BAR_ICONS_DEBUG_COLORING)

		/**
		* For convenience in the StatusBarIconController, we want to gate some actions based on slot
		* name and the flag together.
		*
		* @return true if this icon is controlled by any of the status bar pipeline flags
		*/
		fun isIconControlledByFlags(slotName: String): Boolean =
		slotName == wifiSlot && useNewWifiIcon() \|\| slotName == mobileSlot && useNewMobileIcons()
		}

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java

+62 −0

Original line number	Diff line number	Diff line
		@@ -23,6 +23,8 @@ import static junit.framework.Assert.assertTrue;
		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.anyInt;
		import static org.mockito.Mockito.mock;
		import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.verify;
		import static org.mockito.Mockito.when;

		import android.testing.AndroidTestingRunner;
		@@ -33,7 +35,10 @@ import android.widget.LinearLayout;
		import androidx.test.filters.SmallTest;

		import com.android.internal.statusbar.StatusBarIcon;
		import com.android.systemui.demomode.DemoModeController;
		import com.android.systemui.dump.DumpManager;
		import com.android.systemui.plugins.DarkIconDispatcher;
		import com.android.systemui.statusbar.CommandQueue;
		import com.android.systemui.statusbar.StatusBarIconView;
		import com.android.systemui.statusbar.StatusBarMobileView;
		import com.android.systemui.statusbar.StatusBarWifiView;
		@@ -46,6 +51,8 @@ import com.android.systemui.statusbar.phone.StatusBarSignalPolicy.WifiIconState;
		import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags;
		import com.android.systemui.statusbar.pipeline.mobile.ui.MobileUiAdapter;
		import com.android.systemui.statusbar.pipeline.wifi.ui.WifiUiAdapter;
		import com.android.systemui.statusbar.policy.ConfigurationController;
		import com.android.systemui.tuner.TunerService;
		import com.android.systemui.utils.leaks.LeakCheckedTest;

		import org.junit.Before;
		@@ -87,6 +94,61 @@ public class StatusBarIconControllerTest extends LeakCheckedTest {
		testCallOnAdd_forManager(manager);
		}

		@Test
		public void testRemoveIcon_ignoredForNewPipeline() {
		IconManager manager = mock(IconManager.class);

		// GIVEN the new pipeline is on
		StatusBarPipelineFlags flags = mock(StatusBarPipelineFlags.class);
		when(flags.isIconControlledByFlags("test_icon")).thenReturn(true);

		StatusBarIconController iconController = new StatusBarIconControllerImpl(
		mContext,
		mock(CommandQueue.class),
		mock(DemoModeController.class),
		mock(ConfigurationController.class),
		mock(TunerService.class),
		mock(DumpManager.class),
		mock(StatusBarIconList.class),
		flags
		);

		iconController.addIconGroup(manager);

		// WHEN a request to remove a new icon is sent
		iconController.removeIcon("test_icon", 0);

		// THEN it is not removed for those icons
		verify(manager, never()).onRemoveIcon(anyInt());
		}

		@Test
		public void testRemoveAllIconsForSlot_ignoredForNewPipeline() {
		IconManager manager = mock(IconManager.class);

		// GIVEN the new pipeline is on
		StatusBarPipelineFlags flags = mock(StatusBarPipelineFlags.class);
		when(flags.isIconControlledByFlags("test_icon")).thenReturn(true);

		StatusBarIconController iconController = new StatusBarIconControllerImpl(
		mContext,
		mock(CommandQueue.class),
		mock(DemoModeController.class),
		mock(ConfigurationController.class),
		mock(TunerService.class),
		mock(DumpManager.class),
		mock(StatusBarIconList.class),
		flags
		);

		iconController.addIconGroup(manager);

		// WHEN a request to remove a new icon is sent
		iconController.removeAllIconsForSlot("test_icon");

		// THEN it is not removed for those icons
		verify(manager, never()).onRemoveIcon(anyInt());
		}

		private <T extends IconManager & TestableIconManager> void testCallOnAdd_forManager(T manager) {
		StatusBarIconHolder holder = holderForType(TYPE_ICON);