diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt index b611c9659db89..6ad91612f99b6 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt @@ -35,6 +35,11 @@ interface ResourceFlag : Flag { val resourceId: Int } +interface SysPropFlag : Flag { + val name: String + val default: T +} + // Consider using the "parcelize" kotlin library. data class BooleanFlag @JvmOverloads constructor( @@ -66,6 +71,12 @@ data class ResourceBooleanFlag constructor( @BoolRes override val resourceId: Int ) : ResourceFlag +data class SysPropBooleanFlag constructor( + override val id: Int, + override val name: String, + override val default: Boolean = false +) : SysPropFlag + data class StringFlag @JvmOverloads constructor( override val id: Int, override val default: String = "" diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt index ec619dd6eea3d..149f6e8e7e9ed 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt @@ -54,7 +54,7 @@ class FlagManager constructor( * An action called on restart which takes as an argument whether the listeners requested * that the restart be suppressed */ - var restartAction: Consumer? = null + var onSettingsChangedAction: Consumer? = null var clearCacheAction: Consumer? = null private val listeners: MutableSet = mutableSetOf() private val settingsObserver: ContentObserver = SettingsObserver() @@ -154,11 +154,11 @@ class FlagManager constructor( val idStr = parts[parts.size - 1] val id = try { idStr.toInt() } catch (e: NumberFormatException) { return } clearCacheAction?.accept(id) - dispatchListenersAndMaybeRestart(id) + dispatchListenersAndMaybeRestart(id, onSettingsChangedAction) } } - fun dispatchListenersAndMaybeRestart(id: Int) { + fun dispatchListenersAndMaybeRestart(id: Int, restartAction: Consumer?) { val filteredListeners: List = synchronized(listeners) { listeners.mapNotNull { if (it.id == id) it.listener else null } } diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt index 96a90dfc7fe95..9d6e3c2951009 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt @@ -28,6 +28,9 @@ interface FeatureFlags : FlagListenable { /** Returns a boolean value for the given flag. */ fun isEnabled(flag: ResourceBooleanFlag): Boolean + /** Returns a boolean value for the given flag. */ + fun isEnabled(flag: SysPropBooleanFlag): Boolean + /** Returns a string value for the given flag. */ fun getString(flag: StringFlag): String diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java index df605990fa81a..677990437f3df 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java @@ -49,6 +49,7 @@ import java.util.ArrayList; import java.util.Map; import java.util.Objects; import java.util.TreeMap; +import java.util.function.Consumer; import java.util.function.Supplier; import javax.inject.Inject; @@ -69,6 +70,7 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { private final FlagManager mFlagManager; private final SecureSettings mSecureSettings; private final Resources mResources; + private final SystemPropertiesHelper mSystemProperties; private final Supplier>> mFlagsCollector; private final Map mBooleanFlagCache = new TreeMap<>(); private final Map mStringFlagCache = new TreeMap<>(); @@ -79,6 +81,7 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { FlagManager flagManager, Context context, SecureSettings secureSettings, + SystemPropertiesHelper systemProperties, @Main Resources resources, DumpManager dumpManager, @Nullable Supplier>> flagsCollector, @@ -86,11 +89,12 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { mFlagManager = flagManager; mSecureSettings = secureSettings; mResources = resources; + mSystemProperties = systemProperties; mFlagsCollector = flagsCollector != null ? flagsCollector : Flags::collectFlags; IntentFilter filter = new IntentFilter(); filter.addAction(ACTION_SET_FLAG); filter.addAction(ACTION_GET_FLAGS); - flagManager.setRestartAction(this::restartSystemUI); + flagManager.setOnSettingsChangedAction(this::restartSystemUI); flagManager.setClearCacheAction(this::removeFromCache); context.registerReceiver(mReceiver, filter, null, null, Context.RECEIVER_EXPORTED_UNAUDITED); @@ -121,6 +125,17 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { return mBooleanFlagCache.get(id); } + @Override + public boolean isEnabled(@NonNull SysPropBooleanFlag flag) { + int id = flag.getId(); + if (!mBooleanFlagCache.containsKey(id)) { + mBooleanFlagCache.put( + id, mSystemProperties.getBoolean(flag.getName(), flag.getDefault())); + } + + return mBooleanFlagCache.get(id); + } + @NonNull @Override public String getString(@NonNull StringFlag flag) { @@ -180,16 +195,28 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { mSecureSettings.putString(mFlagManager.idToSettingsKey(id), data); Log.i(TAG, "Set id " + id + " to " + value); removeFromCache(id); - mFlagManager.dispatchListenersAndMaybeRestart(id); + mFlagManager.dispatchListenersAndMaybeRestart(id, this::restartSystemUI); + } + + private void eraseFlag(Flag flag) { + if (flag instanceof SysPropFlag) { + mSystemProperties.erase(((SysPropFlag) flag).getName()); + dispatchListenersAndMaybeRestart(flag.getId(), this::restartAndroid); + } else { + eraseFlag(flag.getId()); + } } /** Erase a flag's overridden value if there is one. */ - public void eraseFlag(int id) { + private void eraseFlag(int id) { eraseInternal(id); removeFromCache(id); - mFlagManager.dispatchListenersAndMaybeRestart(id); + dispatchListenersAndMaybeRestart(id, this::restartSystemUI); } + private void dispatchListenersAndMaybeRestart(int id, Consumer restartAction) { + mFlagManager.dispatchListenersAndMaybeRestart(id, restartAction); + } /** Works just like {@link #eraseFlag(int)} except that it doesn't restart SystemUI. */ private void eraseInternal(int id) { // We can't actually "erase" things from sysprops, but we can set them to empty! @@ -217,7 +244,11 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { System.exit(0); } - private void restartAndroid() { + private void restartAndroid(boolean requestSuppress) { + if (requestSuppress) { + Log.i(TAG, "Android Restart Suppressed"); + return; + } Log.i(TAG, "Restarting Android"); try { mBarService.restart(); @@ -273,7 +304,7 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { Flag flag = flagMap.get(id); if (!extras.containsKey(EXTRA_VALUE)) { - eraseFlag(id); + eraseFlag(flag); return; } @@ -282,6 +313,10 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { setFlagValue(id, (Boolean) value, BooleanFlagSerializer.INSTANCE); } else if (flag instanceof ResourceBooleanFlag && value instanceof Boolean) { setFlagValue(id, (Boolean) value, BooleanFlagSerializer.INSTANCE); + } else if (flag instanceof SysPropBooleanFlag && value instanceof Boolean) { + // Store SysProp flags in SystemProperties where they can read by outside parties. + mSystemProperties.setBoolean( + ((SysPropBooleanFlag) flag).getName(), (Boolean) value); } else if (flag instanceof StringFlag && value instanceof String) { setFlagValue(id, (String) value, StringFlagSerializer.INSTANCE); } else if (flag instanceof ResourceStringFlag && value instanceof String) { @@ -306,6 +341,9 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { if (f instanceof ResourceBooleanFlag) { return new BooleanFlag(f.getId(), isEnabled((ResourceBooleanFlag) f)); } + if (f instanceof SysPropBooleanFlag) { + return new BooleanFlag(f.getId(), isEnabled((SysPropBooleanFlag) f)); + } // TODO: add support for other flag types. Log.w(TAG, "Unsupported Flag Type. Please file a bug."); diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java index 348a8e20122ea..1fb1acfae1cb3 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java @@ -43,11 +43,17 @@ import javax.inject.Inject; @SysUISingleton public class FeatureFlagsRelease implements FeatureFlags, Dumpable { private final Resources mResources; + private final SystemPropertiesHelper mSystemProperties; SparseBooleanArray mBooleanCache = new SparseBooleanArray(); SparseArray mStringCache = new SparseArray<>(); + @Inject - public FeatureFlagsRelease(@Main Resources resources, DumpManager dumpManager) { + public FeatureFlagsRelease( + @Main Resources resources, + SystemPropertiesHelper systemProperties, + DumpManager dumpManager) { mResources = resources; + mSystemProperties = systemProperties; dumpManager.registerDumpable("SysUIFlags", this); } @@ -72,6 +78,17 @@ public class FeatureFlagsRelease implements FeatureFlags, Dumpable { return mBooleanCache.valueAt(cacheIndex); } + @Override + public boolean isEnabled(SysPropBooleanFlag flag) { + int cacheIndex = mBooleanCache.indexOfKey(flag.getId()); + if (cacheIndex < 0) { + return isEnabled( + flag.getId(), mSystemProperties.getBoolean(flag.getName(), flag.getDefault())); + } + + return mBooleanCache.valueAt(cacheIndex); + } + private boolean isEnabled(int key, boolean defaultValue) { mBooleanCache.append(key, defaultValue); return defaultValue; diff --git a/packages/SystemUI/src/com/android/systemui/flags/SystemPropertiesHelper.kt b/packages/SystemUI/src/com/android/systemui/flags/SystemPropertiesHelper.kt index 1dc5a9f3adc5a..6c160975587b3 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/SystemPropertiesHelper.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/SystemPropertiesHelper.kt @@ -34,6 +34,10 @@ open class SystemPropertiesHelper @Inject constructor() { return SystemProperties.getBoolean(name, default) } + fun setBoolean(name: String, value: Boolean) { + SystemProperties.set(name, if (value) "1" else "0") + } + fun set(name: String, value: String) { SystemProperties.set(name, value) } @@ -41,4 +45,8 @@ open class SystemPropertiesHelper @Inject constructor() { fun set(name: String, value: Int) { set(name, value.toString()) } + + fun erase(name: String) { + set(name, "") + } } \ No newline at end of file diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt index 4cc5673eaf978..23a5b2b2c5b7c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt @@ -20,7 +20,7 @@ import android.content.Context import android.content.Intent import android.content.pm.PackageManager.NameNotFoundException import android.content.res.Resources -import androidx.test.filters.SmallTest +import android.test.suitebuilder.annotation.SmallTest import com.android.internal.statusbar.IStatusBarService import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager @@ -35,6 +35,8 @@ import org.junit.Assert import org.junit.Before import org.junit.Test import org.mockito.Mock +import org.mockito.Mockito.anyBoolean +import org.mockito.Mockito.anyString import org.mockito.Mockito.inOrder import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -57,6 +59,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Mock private lateinit var mFlagManager: FlagManager @Mock private lateinit var mMockContext: Context @Mock private lateinit var mSecureSettings: SecureSettings + @Mock private lateinit var mSystemProperties: SystemPropertiesHelper @Mock private lateinit var mResources: Resources @Mock private lateinit var mDumpManager: DumpManager @Mock private lateinit var mBarService: IStatusBarService @@ -71,12 +74,13 @@ class FeatureFlagsDebugTest : SysuiTestCase() { mFlagManager, mMockContext, mSecureSettings, + mSystemProperties, mResources, mDumpManager, { mFlagMap }, mBarService ) - verify(mFlagManager).restartAction = any() + verify(mFlagManager).onSettingsChangedAction = any() mBroadcastReceiver = withArgCaptor { verify(mMockContext).registerReceiver(capture(), any(), nullable(), nullable(), any()) @@ -122,6 +126,22 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } } + @Test + fun testReadSysPropBooleanFlag() { + whenever(mSystemProperties.getBoolean(anyString(), anyBoolean())).thenAnswer { + if ("b".equals(it.getArgument(0))) { + return@thenAnswer true + } + return@thenAnswer it.getArgument(1) + } + + assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a"))).isFalse() + assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b"))).isTrue() + assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", true))).isTrue() + assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(4, "d", false))).isFalse() + assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(5, "e"))).isFalse() + } + @Test fun testReadStringFlag() { whenever(mFlagManager.readFlagValue(eq(3), any())).thenReturn("foo") @@ -259,7 +279,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { verify(mFlagManager, times(numReads)).readFlagValue(eq(id), any>()) verify(mFlagManager).idToSettingsKey(eq(id)) verify(mSecureSettings).putString(eq("key-$id"), eq(data)) - verify(mFlagManager).dispatchListenersAndMaybeRestart(eq(id)) + verify(mFlagManager).dispatchListenersAndMaybeRestart(eq(id), any()) }.verifyNoMoreInteractions() verifyNoMoreInteractions(mFlagManager, mSecureSettings) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt index b5e6602594eb4..ad304c49bd411 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt @@ -17,7 +17,7 @@ package com.android.systemui.flags import android.content.pm.PackageManager.NameNotFoundException import android.content.res.Resources -import androidx.test.filters.SmallTest +import android.test.suitebuilder.annotation.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager import com.android.systemui.util.mockito.any @@ -44,12 +44,13 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { private lateinit var mFeatureFlagsRelease: FeatureFlagsRelease @Mock private lateinit var mResources: Resources + @Mock private lateinit var mSystemProperties: SystemPropertiesHelper @Mock private lateinit var mDumpManager: DumpManager @Before fun setup() { MockitoAnnotations.initMocks(this) - mFeatureFlagsRelease = FeatureFlagsRelease(mResources, mDumpManager) + mFeatureFlagsRelease = FeatureFlagsRelease(mResources, mSystemProperties, mDumpManager) } @After @@ -86,6 +87,17 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { } } + @Test + fun testSysPropBooleanFlag() { + val flagId = 213 + val flagName = "sys_prop_flag" + val flagDefault = true + + val flag = SysPropBooleanFlag(flagId, flagName, flagDefault) + whenever(mSystemProperties.getBoolean(flagName, flagDefault)).thenReturn(flagDefault) + assertThat(mFeatureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault) + } + @Test fun testDump() { val flag1 = BooleanFlag(1, true) diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt index 644bd21476116..a2eca81b04ed1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt @@ -19,7 +19,7 @@ import android.content.Context import android.database.ContentObserver import android.net.Uri import android.os.Handler -import androidx.test.filters.SmallTest +import android.test.suitebuilder.annotation.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.eq @@ -130,14 +130,14 @@ class FlagManagerTest : SysuiTestCase() { mFlagManager.addListener(BooleanFlag(1, true), listener1) mFlagManager.addListener(BooleanFlag(10, true), listener10) - mFlagManager.dispatchListenersAndMaybeRestart(1) + mFlagManager.dispatchListenersAndMaybeRestart(1, null) val flagEvent1 = withArgCaptor { verify(listener1).onFlagChanged(capture()) } assertThat(flagEvent1.flagId).isEqualTo(1) verifyNoMoreInteractions(listener1, listener10) - mFlagManager.dispatchListenersAndMaybeRestart(10) + mFlagManager.dispatchListenersAndMaybeRestart(10, null) val flagEvent10 = withArgCaptor { verify(listener10).onFlagChanged(capture()) } @@ -151,14 +151,14 @@ class FlagManagerTest : SysuiTestCase() { mFlagManager.addListener(BooleanFlag(1, true), listener) mFlagManager.addListener(BooleanFlag(10, true), listener) - mFlagManager.dispatchListenersAndMaybeRestart(1) + mFlagManager.dispatchListenersAndMaybeRestart(1, null) val flagEvent1 = withArgCaptor { verify(listener).onFlagChanged(capture()) } assertThat(flagEvent1.flagId).isEqualTo(1) verifyNoMoreInteractions(listener) - mFlagManager.dispatchListenersAndMaybeRestart(10) + mFlagManager.dispatchListenersAndMaybeRestart(10, null) val flagEvent10 = withArgCaptor { verify(listener, times(2)).onFlagChanged(capture()) } @@ -169,8 +169,7 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testRestartWithNoListeners() { val restartAction = mock>() - mFlagManager.restartAction = restartAction - mFlagManager.dispatchListenersAndMaybeRestart(1) + mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) verify(restartAction).accept(eq(false)) verifyNoMoreInteractions(restartAction) } @@ -178,11 +177,10 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testListenerCanSuppressRestart() { val restartAction = mock>() - mFlagManager.restartAction = restartAction mFlagManager.addListener(BooleanFlag(1, true)) { event -> event.requestNoRestart() } - mFlagManager.dispatchListenersAndMaybeRestart(1) + mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) verify(restartAction).accept(eq(true)) verifyNoMoreInteractions(restartAction) } @@ -190,11 +188,10 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testListenerOnlySuppressesRestartForOwnFlag() { val restartAction = mock>() - mFlagManager.restartAction = restartAction mFlagManager.addListener(BooleanFlag(10, true)) { event -> event.requestNoRestart() } - mFlagManager.dispatchListenersAndMaybeRestart(1) + mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) verify(restartAction).accept(eq(false)) verifyNoMoreInteractions(restartAction) } @@ -202,14 +199,13 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testRestartWhenNotAllListenersRequestSuppress() { val restartAction = mock>() - mFlagManager.restartAction = restartAction mFlagManager.addListener(BooleanFlag(10, true)) { event -> event.requestNoRestart() } mFlagManager.addListener(BooleanFlag(10, true)) { // do not request } - mFlagManager.dispatchListenersAndMaybeRestart(1) + mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) verify(restartAction).accept(eq(false)) verifyNoMoreInteractions(restartAction) }