Merge "Add support for SystemProperty flags to SysUI." into tm-dev

This commit is contained in:
Dave Mankoff
2022-02-23 22:20:48 +00:00
committed by Android (Google) Code Review
9 changed files with 133 additions and 28 deletions

View File

@@ -35,6 +35,11 @@ interface ResourceFlag<T> : Flag<T> {
val resourceId: Int
}
interface SysPropFlag<T> : Flag<T> {
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<Boolean>
data class SysPropBooleanFlag constructor(
override val id: Int,
override val name: String,
override val default: Boolean = false
) : SysPropFlag<Boolean>
data class StringFlag @JvmOverloads constructor(
override val id: Int,
override val default: String = ""

View File

@@ -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<Boolean>? = null
var onSettingsChangedAction: Consumer<Boolean>? = null
var clearCacheAction: Consumer<Int>? = null
private val listeners: MutableSet<PerFlagListener> = 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<Boolean>?) {
val filteredListeners: List<FlagListenable.Listener> = synchronized(listeners) {
listeners.mapNotNull { if (it.id == id) it.listener else null }
}

View File

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

View File

@@ -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<Map<Integer, Flag<?>>> mFlagsCollector;
private final Map<Integer, Boolean> mBooleanFlagCache = new TreeMap<>();
private final Map<Integer, String> 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<Map<Integer, Flag<?>>> 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 <T> void eraseFlag(Flag<T> flag) {
if (flag instanceof SysPropFlag) {
mSystemProperties.erase(((SysPropFlag<T>) 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<Boolean> 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.");

View File

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

View File

@@ -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, "")
}
}

View File

@@ -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<String?>(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<String>(eq(3), any())).thenReturn("foo")
@@ -259,7 +279,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
verify(mFlagManager, times(numReads)).readFlagValue(eq(id), any<FlagSerializer<*>>())
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)
}

View File

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

View File

@@ -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<FlagListenable.FlagEvent> {
verify(listener1).onFlagChanged(capture())
}
assertThat(flagEvent1.flagId).isEqualTo(1)
verifyNoMoreInteractions(listener1, listener10)
mFlagManager.dispatchListenersAndMaybeRestart(10)
mFlagManager.dispatchListenersAndMaybeRestart(10, null)
val flagEvent10 = withArgCaptor<FlagListenable.FlagEvent> {
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<FlagListenable.FlagEvent> {
verify(listener).onFlagChanged(capture())
}
assertThat(flagEvent1.flagId).isEqualTo(1)
verifyNoMoreInteractions(listener)
mFlagManager.dispatchListenersAndMaybeRestart(10)
mFlagManager.dispatchListenersAndMaybeRestart(10, null)
val flagEvent10 = withArgCaptor<FlagListenable.FlagEvent> {
verify(listener, times(2)).onFlagChanged(capture())
}
@@ -169,8 +169,7 @@ class FlagManagerTest : SysuiTestCase() {
@Test
fun testRestartWithNoListeners() {
val restartAction = mock<Consumer<Boolean>>()
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<Consumer<Boolean>>()
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<Consumer<Boolean>>()
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<Consumer<Boolean>>()
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)
}