Merge "[Sb refactor] Ignore old pipeline callbacks to removeIcon*" into tm-qpr-dev am: ff1e610f6f am: e4d4907f3e

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21323046

Change-Id: I81b2d82dae701de95f2c007f03c0fcbc2891c105
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 104 insertions, 3 deletions

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 416bc7141eeb..5408afb71547 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java
@@ -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;
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/StatusBarPipelineFlags.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/StatusBarPipelineFlags.kt
index 15fed3244d97..4a684d9f8e36 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/StatusBarPipelineFlags.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/StatusBarPipelineFlags.kt
@@ -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()
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java
index 6fb68938b00d..8aaa57ffe2cb 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java
@@ -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);