Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Sb refactor] Ignore old pipeline callbacks to removeIcon*

Browse Source 
StatusBarIconController was already ignoring calls to set icon when the
new pipeline was enabled, but was still allowing calls to `removeIcon`
and `removeAllIconsForSlot` externally. Therefore, it was possible for
the new pipeline to add an icon for a mobile connection during system
startup, only to have it later removed by `StatusBarSignalPolicy`, which
later came on line and tried to remove existing subscriptions.

This change ignores external calls to remove icons if the slot is
something controlled under the new pipeline.

Fixes: 266960478
Test: StatusBarIconControllerTest
Change-Id: I570fe9ea2e8c3cf09c89a100a68dce73e00febf7
This commit is contained in:
Evan Laird
2023-02-07 15:35:38 -05:00
parent b488504cf3
commit 754e1951fb
3 changed files with 104 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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;

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

@@ -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()
}

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

@@ -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);