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

FeatureFlags Listeners:

Browse Source 
* Convert FlagReader interface to FlagListenable.
* Move Flag reading to FeatureFlags interface.
* Remove isEnabled(id, boolean) from all interfaces; make private in implementations where used.

Bug: 209081785
Test: atest FlagManagerTest
Change-Id: I8c90a67def242b5d2fa55d5effab8270ecafabb4
This commit is contained in:
Jeff DeCew
2021-12-03 02:55:01 +00:00
parent 8f3255ae3b
commit 595956489d
8 changed files with 374 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt → packages/SystemUI/shared/src/com/android/systemui/flags/FlagListenable.kt
View File

@@ -18,28 +18,24 @@ package com.android.systemui.flags
/**
* Plugin for loading flag values
*/
interface FlagReader {
/** Returns a boolean value for the given flag. */
fun isEnabled(flag: BooleanFlag): Boolean {
return flag.default
}
fun isEnabled(flag: ResourceBooleanFlag): Boolean
/** Returns a boolean value for the given flag. */
fun isEnabled(id: Int, def: Boolean): Boolean {
return def
}
/** Add a listener to be alerted when any flag changes. */
fun addListener(listener: Listener) {}
interface FlagListenable {
/** Add a listener to be alerted when the given flag changes. */
fun addListener(flag: Flag<*>, listener: Listener)
/** Remove a listener to be alerted when any flag changes. */
fun removeListener(listener: Listener) {}
fun removeListener(listener: Listener)
/** A simple listener to be alerted when a flag changes. */
fun interface Listener {
/** */
fun onFlagChanged(id: Int)
/** Called when the flag changes */
fun onFlagChanged(event: FlagEvent)
}
/** An event representing the change */
interface FlagEvent {
/** the id of the flag which changed */
val flagId: Int
/** if all listeners alerted invoke this method, the restart will be skipped */
fun requestNoRestart()
}
}

88
packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt
View File

@@ -24,16 +24,17 @@ import android.database.ContentObserver
import android.net.Uri
import android.os.Bundle
import android.os.Handler
import android.provider.Settings
import androidx.concurrent.futures.CallbackToFutureAdapter
import com.google.common.util.concurrent.ListenableFuture
import org.json.JSONException
import org.json.JSONObject
import java.util.function.Consumer
class FlagManager constructor(
private val context: Context,
private val settings: FlagSettingsHelper,
private val handler: Handler
) : FlagReader {
) : FlagListenable {
companion object {
const val RECEIVING_PACKAGE = "com.android.systemui"
const val ACTION_SET_FLAG = "com.android.systemui.action.SET_FLAG"
@@ -47,7 +48,19 @@ class FlagManager constructor(
private const val SETTINGS_PREFIX = "systemui/flags"
}
private val listeners: MutableSet<FlagReader.Listener> = mutableSetOf()
constructor(context: Context, handler: Handler) : this(
context,
FlagSettingsHelper(context.contentResolver),
handler
)
/**
* 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 clearCacheAction: Consumer<Int>? = null
private val listeners: MutableSet<PerFlagListener> = mutableSetOf()
private val settingsObserver: ContentObserver = SettingsObserver()
fun getFlagsFuture(): ListenableFuture<Collection<Flag<*>>> {
@@ -86,14 +99,9 @@ class FlagManager constructor(
context.sendBroadcast(intent)
}
override fun isEnabled(id: Int, def: Boolean): Boolean {
return isEnabled(id) ?: def
}
/** Returns the stored value or null if not set. */
fun isEnabled(id: Int): Boolean? {
val data: String? = Settings.Secure.getString(
context.contentResolver, keyToSettingsPrefix(id))
val data = settings.getString(keyToSettingsPrefix(id))
if (data == null || data?.isEmpty()) {
return null
}
@@ -108,27 +116,24 @@ class FlagManager constructor(
}
}
override fun isEnabled(flag: ResourceBooleanFlag): Boolean {
throw RuntimeException("Not implemented in FlagManager")
}
override fun addListener(listener: FlagReader.Listener) {
override fun addListener(flag: Flag<*>, listener: FlagListenable.Listener) {
synchronized(listeners) {
val registerNeeded = listeners.isEmpty()
listeners.add(listener)
listeners.add(PerFlagListener(flag.id, listener))
if (registerNeeded) {
context.contentResolver.registerContentObserver(
Settings.Secure.getUriFor(SETTINGS_PREFIX), true, settingsObserver)
settings.registerContentObserver(SETTINGS_PREFIX, true, settingsObserver)
}
}
}
override fun removeListener(listener: FlagReader.Listener) {
override fun removeListener(listener: FlagListenable.Listener) {
synchronized(listeners) {
val isRegistered = !listeners.isEmpty()
listeners.remove(listener)
if (isRegistered && listeners.isEmpty()) {
context.contentResolver.unregisterContentObserver(settingsObserver)
if (listeners.isEmpty()) {
return
}
listeners.removeIf { it.listener == listener }
if (listeners.isEmpty()) {
settings.unregisterContentObserver(settingsObserver)
}
}
}
@@ -142,7 +147,7 @@ class FlagManager constructor(
}
fun keyToSettingsPrefix(key: Int): String {
return SETTINGS_PREFIX + "/" + key
return "$SETTINGS_PREFIX/$key"
}
private fun assertType(json: JSONObject, type: String): Boolean {
@@ -160,14 +165,39 @@ class FlagManager constructor(
}
val parts = uri.pathSegments
val idStr = parts[parts.size - 1]
try {
val id = idStr.toInt()
listeners.forEach { l -> l.onFlagChanged(id) }
} catch (e: NumberFormatException) {
// no-op
}
val id = try { idStr.toInt() } catch (e: NumberFormatException) { return }
clearCacheAction?.accept(id)
dispatchListenersAndMaybeRestart(id)
}
}
fun dispatchListenersAndMaybeRestart(id: Int) {
val filteredListeners: List<FlagListenable.Listener> = synchronized(listeners) {
listeners.mapNotNull { if (it.id == id) it.listener else null }
}
// If there are no listeners, there's nothing to dispatch to, and nothing to suppress it.
if (filteredListeners.isEmpty()) {
restartAction?.accept(false)
return
}
// Dispatch to every listener and save whether each one called requestNoRestart.
val suppressRestartList: List<Boolean> = filteredListeners.map { listener ->
var didRequestNoRestart = false
val event = object : FlagListenable.FlagEvent {
override val flagId = id
override fun requestNoRestart() {
didRequestNoRestart = true
}
}
listener.onFlagChanged(event)
didRequestNoRestart
}
// Suppress restart only if ALL listeners request it.
val suppressRestart = suppressRestartList.all { it }
restartAction?.accept(suppressRestart)
}
private data class PerFlagListener(val id: Int, val listener: FlagListenable.Listener)
}
class InvalidFlagStorageException : Exception("Data found but is invalid")

42
packages/SystemUI/shared/src/com/android/systemui/flags/FlagSettingsHelper.kt Normal file
View File

@@ -0,0 +1,42 @@
/*
* Copyright (C) 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.systemui.flags
import android.content.ContentResolver
import android.database.ContentObserver
import android.provider.Settings
class FlagSettingsHelper(private val contentResolver: ContentResolver) {
fun getString(key: String): String? = Settings.Secure.getString(contentResolver, key)
fun registerContentObserver(
name: String,
notifyForDescendants: Boolean,
observer: ContentObserver
) {
contentResolver.registerContentObserver(
Settings.Secure.getUriFor(name),
notifyForDescendants,
observer
)
}
fun unregisterContentObserver(observer: ContentObserver) {
contentResolver.unregisterContentObserver(observer)
}
}

13
packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.java → packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt
View File

@@ -14,12 +14,17 @@
* limitations under the License.
*/
package com.android.systemui.flags;
package com.android.systemui.flags
/**
* Class to manage simple DeviceConfig-based feature flags.
*
* See {@link Flags} for instructions on defining new flags.
* See [Flags] for instructions on defining new flags.
*/
public interface FeatureFlags extends FlagReader {
}
interface FeatureFlags : FlagListenable {
/** Returns a boolean value for the given flag. */
fun isEnabled(flag: BooleanFlag): Boolean
/** Returns a boolean value for the given flag. */
fun isEnabled(flag: ResourceBooleanFlag): Boolean
}

41
packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java
View File

@@ -22,7 +22,6 @@ import static com.android.systemui.flags.FlagManager.FIELD_FLAGS;
import static com.android.systemui.flags.FlagManager.FIELD_ID;
import static com.android.systemui.flags.FlagManager.FIELD_VALUE;
import android.annotation.Nullable;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
@@ -32,6 +31,7 @@ import android.os.Bundle;
import android.util.Log;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.android.systemui.Dumpable;
import com.android.systemui.dagger.SysUISingleton;
@@ -81,6 +81,8 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
IntentFilter filter = new IntentFilter();
filter.addAction(ACTION_SET_FLAG);
filter.addAction(ACTION_GET_FLAGS);
flagManager.setRestartAction(this::restartSystemUI);
flagManager.setClearCacheAction(this::removeFromCache);
context.registerReceiver(mReceiver, filter, null, null);
dumpManager.registerDumpable(TAG, this);
}
@@ -106,9 +108,8 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
return mBooleanFlagCache.get(id);
}
/** Return a {@link BooleanFlag}'s value. */
@Override
public boolean isEnabled(int id, boolean defaultValue) {
/** Return a flag's value. */
private boolean isEnabled(int id, boolean defaultValue) {
Boolean result = isEnabledInternal(id);
return result == null ? defaultValue : result;
}
@@ -136,17 +137,19 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
json.put(FlagManager.FIELD_TYPE, FlagManager.TYPE_BOOLEAN);
json.put(FIELD_VALUE, value);
mSecureSettings.putString(mFlagManager.keyToSettingsPrefix(id), json.toString());
Log.i(TAG, "Set id " + id + " to " + value);
restartSystemUI();
} catch (JSONException e) {
// no-op
return; // ignore
}
Log.i(TAG, "Set id " + id + " to " + value);
removeFromCache(id);
mFlagManager.dispatchListenersAndMaybeRestart(id);
}
/** Erase a flag's overridden value if there is one. */
public void eraseFlag(int id) {
eraseInternal(id);
restartSystemUI();
removeFromCache(id);
mFlagManager.dispatchListenersAndMaybeRestart(id);
}
/** Works just like {@link #eraseFlag(int)} except that it doesn't restart SystemUI. */
@@ -157,16 +160,20 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
}
@Override
public void addListener(Listener run) {
mFlagManager.addListener(run);
public void addListener(@NonNull Flag<?> flag, @NonNull Listener listener) {
mFlagManager.addListener(flag, listener);
}
@Override
public void removeListener(Listener run) {
mFlagManager.removeListener(run);
public void removeListener(@NonNull Listener listener) {
mFlagManager.removeListener(listener);
}
private void restartSystemUI() {
private void restartSystemUI(boolean requestSuppress) {
if (requestSuppress) {
Log.i(TAG, "SystemUI Restart Suppressed");
return;
}
Log.i(TAG, "Restarting SystemUI");
// SysUI starts back when up exited. Is there a better way to do this?
System.exit(0);
@@ -202,6 +209,10 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
}
private void handleSetFlag(Bundle extras) {
if (extras == null) {
Log.w(TAG, "No extras");
return;
}
int id = extras.getInt(FIELD_ID);
if (id <= 0) {
Log.w(TAG, "ID not set or less than or equal to 0: " + id);
@@ -245,6 +256,10 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
}
};
private void removeFromCache(int id) {
mBooleanFlagCache.remove(id);
}
@Override
public void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter pw, @NonNull String[] args) {
pw.println("can override: true");

10
packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java
View File

@@ -48,10 +48,10 @@ public class FeatureFlagsRelease implements FeatureFlags, Dumpable {
}
@Override
public void addListener(Listener run) {}
public void addListener(@NonNull Flag<?> flag, @NonNull Listener listener) {}
@Override
public void removeListener(Listener run) {}
public void removeListener(@NonNull Listener listener) {}
@Override
public boolean isEnabled(BooleanFlag flag) {
@@ -68,8 +68,7 @@ public class FeatureFlagsRelease implements FeatureFlags, Dumpable {
return mFlagCache.valueAt(cacheIndex);
}
@Override
public boolean isEnabled(int key, boolean defaultValue) {
private boolean isEnabled(int key, boolean defaultValue) {
mFlagCache.append(key, defaultValue);
return defaultValue;
}
@@ -79,8 +78,7 @@ public class FeatureFlagsRelease implements FeatureFlags, Dumpable {
pw.println("can override: false");
int size = mFlagCache.size();
for (int i = 0; i < size; i++) {
pw.println(" sysui_flag_" + mFlagCache.keyAt(i)
+ ": " + mFlagCache.valueAt(i));
pw.println(" sysui_flag_" + mFlagCache.keyAt(i) + ": " + mFlagCache.valueAt(i));
}
}
}

6
packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.java
View File

@@ -81,21 +81,23 @@ public class FeatureFlagsReleaseTest extends SysuiTestCase {
public void testDump() {
int flagIdA = 213;
int flagIdB = 18;
int flagIdC = 1;
int flagResourceId = 3;
BooleanFlag flagA = new BooleanFlag(flagIdA, true);
ResourceBooleanFlag flagB = new ResourceBooleanFlag(flagIdB, flagResourceId);
BooleanFlag flagC = new BooleanFlag(flagIdC, false);
when(mResources.getBoolean(flagResourceId)).thenReturn(true);
// WHEN the flags have been accessed
assertThat(mFeatureFlagsRelease.isEnabled(1, false)).isFalse();
assertThat(mFeatureFlagsRelease.isEnabled(flagA)).isTrue();
assertThat(mFeatureFlagsRelease.isEnabled(flagB)).isTrue();
assertThat(mFeatureFlagsRelease.isEnabled(flagC)).isFalse();
// THEN the dump contains the flags and the default values
String dump = dumpToString();
assertThat(dump).contains(" sysui_flag_1: false\n");
assertThat(dump).contains(" sysui_flag_" + flagIdA + ": true\n");
assertThat(dump).contains(" sysui_flag_" + flagIdB + ": true\n");
assertThat(dump).contains(" sysui_flag_" + flagIdC + ": false\n");
}
private String dumpToString() {

214
packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt Normal file
View File

@@ -0,0 +1,214 @@
/*
* Copyright (C) 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.systemui.flags
import android.content.Context
import android.database.ContentObserver
import android.net.Uri
import android.os.Handler
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.util.mockito.any
import com.android.systemui.util.mockito.eq
import com.android.systemui.util.mockito.mock
import com.android.systemui.util.mockito.withArgCaptor
import com.google.common.truth.Truth.assertThat
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyNoMoreInteractions
import org.mockito.MockitoAnnotations
import java.util.function.Consumer
/**
* NOTE: This test is for the version of FeatureFlagManager in src-release, which should not allow
* overriding, and should never return any value other than the one provided as the default.
*/
@SmallTest
class FlagManagerTest : SysuiTestCase() {
private lateinit var mFlagManager: FlagManager
@Mock private lateinit var mMockContext: Context
@Mock private lateinit var mFlagSettingsHelper: FlagSettingsHelper
@Mock private lateinit var mHandler: Handler
@Before
fun setup() {
MockitoAnnotations.initMocks(this)
mFlagManager = FlagManager(mMockContext, mFlagSettingsHelper, mHandler)
}
@Test
fun testContentObserverAddedAndRemoved() {
val listener1 = mock<FlagListenable.Listener>()
val listener2 = mock<FlagListenable.Listener>()
// no interactions before adding listener
verifyNoMoreInteractions(mFlagSettingsHelper)
// adding the first listener registers the observer
mFlagManager.addListener(BooleanFlag(1, true), listener1)
val observer = withArgCaptor<ContentObserver> {
verify(mFlagSettingsHelper).registerContentObserver(any(), any(), capture())
}
verifyNoMoreInteractions(mFlagSettingsHelper)
// adding another listener does nothing
mFlagManager.addListener(BooleanFlag(2, true), listener2)
verifyNoMoreInteractions(mFlagSettingsHelper)
// removing the original listener does nothing with second one still present
mFlagManager.removeListener(listener1)
verifyNoMoreInteractions(mFlagSettingsHelper)
// removing the final listener unregisters the observer
mFlagManager.removeListener(listener2)
verify(mFlagSettingsHelper).unregisterContentObserver(eq(observer))
verifyNoMoreInteractions(mFlagSettingsHelper)
}
@Test
fun testObserverClearsCache() {
val listener = mock<FlagListenable.Listener>()
val clearCacheAction = mock<Consumer<Int>>()
mFlagManager.clearCacheAction = clearCacheAction
mFlagManager.addListener(BooleanFlag(1, true), listener)
val observer = withArgCaptor<ContentObserver> {
verify(mFlagSettingsHelper).registerContentObserver(any(), any(), capture())
}
observer.onChange(false, flagUri(1))
verify(clearCacheAction).accept(eq(1))
}
@Test
fun testObserverInvokesListeners() {
val listener1 = mock<FlagListenable.Listener>()
val listener10 = mock<FlagListenable.Listener>()
mFlagManager.addListener(BooleanFlag(1, true), listener1)
mFlagManager.addListener(BooleanFlag(10, true), listener10)
val observer = withArgCaptor<ContentObserver> {
verify(mFlagSettingsHelper).registerContentObserver(any(), any(), capture())
}
observer.onChange(false, flagUri(1))
val flagEvent1 = withArgCaptor<FlagListenable.FlagEvent> {
verify(listener1).onFlagChanged(capture())
}
assertThat(flagEvent1.flagId).isEqualTo(1)
verifyNoMoreInteractions(listener1, listener10)
observer.onChange(false, flagUri(10))
val flagEvent10 = withArgCaptor<FlagListenable.FlagEvent> {
verify(listener10).onFlagChanged(capture())
}
assertThat(flagEvent10.flagId).isEqualTo(10)
verifyNoMoreInteractions(listener1, listener10)
}
fun flagUri(id: Int): Uri = Uri.parse("content://settings/system/systemui/flags/$id")
@Test
fun testOnlySpecificFlagListenerIsInvoked() {
val listener1 = mock<FlagListenable.Listener>()
val listener10 = mock<FlagListenable.Listener>()
mFlagManager.addListener(BooleanFlag(1, true), listener1)
mFlagManager.addListener(BooleanFlag(10, true), listener10)
mFlagManager.dispatchListenersAndMaybeRestart(1)
val flagEvent1 = withArgCaptor<FlagListenable.FlagEvent> {
verify(listener1).onFlagChanged(capture())
}
assertThat(flagEvent1.flagId).isEqualTo(1)
verifyNoMoreInteractions(listener1, listener10)
mFlagManager.dispatchListenersAndMaybeRestart(10)
val flagEvent10 = withArgCaptor<FlagListenable.FlagEvent> {
verify(listener10).onFlagChanged(capture())
}
assertThat(flagEvent10.flagId).isEqualTo(10)
verifyNoMoreInteractions(listener1, listener10)
}
@Test
fun testSameListenerCanBeUsedForMultipleFlags() {
val listener = mock<FlagListenable.Listener>()
mFlagManager.addListener(BooleanFlag(1, true), listener)
mFlagManager.addListener(BooleanFlag(10, true), listener)
mFlagManager.dispatchListenersAndMaybeRestart(1)
val flagEvent1 = withArgCaptor<FlagListenable.FlagEvent> {
verify(listener).onFlagChanged(capture())
}
assertThat(flagEvent1.flagId).isEqualTo(1)
verifyNoMoreInteractions(listener)
mFlagManager.dispatchListenersAndMaybeRestart(10)
val flagEvent10 = withArgCaptor<FlagListenable.FlagEvent> {
verify(listener, times(2)).onFlagChanged(capture())
}
assertThat(flagEvent10.flagId).isEqualTo(10)
verifyNoMoreInteractions(listener)
}
@Test
fun testRestartWithNoListeners() {
val restartAction = mock<Consumer<Boolean>>()
mFlagManager.restartAction = restartAction
mFlagManager.dispatchListenersAndMaybeRestart(1)
verify(restartAction).accept(eq(false))
verifyNoMoreInteractions(restartAction)
}
@Test
fun testListenerCanSuppressRestart() {
val restartAction = mock<Consumer<Boolean>>()
mFlagManager.restartAction = restartAction
mFlagManager.addListener(BooleanFlag(1, true)) { event ->
event.requestNoRestart()
}
mFlagManager.dispatchListenersAndMaybeRestart(1)
verify(restartAction).accept(eq(true))
verifyNoMoreInteractions(restartAction)
}
@Test
fun testListenerOnlySuppressesRestartForOwnFlag() {
val restartAction = mock<Consumer<Boolean>>()
mFlagManager.restartAction = restartAction
mFlagManager.addListener(BooleanFlag(10, true)) { event ->
event.requestNoRestart()
}
mFlagManager.dispatchListenersAndMaybeRestart(1)
verify(restartAction).accept(eq(false))
verifyNoMoreInteractions(restartAction)
}
@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)
verify(restartAction).accept(eq(false))
verifyNoMoreInteractions(restartAction)
}
}