Better support for Resource based flags.

Introduce ResourceFlag class and ParcelableFlag classes, from which
other flag types are based.

ResourceFlags always pull there default values out of
context.getResources. They can not, however, be directly
serialized and sent to the flag app.

For that, we now have ParcelableFlag. Prior to this change, we
were not properly alerting the Flag app about the current value of
flags when it asked. This change now also fixes that.

All flags are coerced into ParcelableFlag before being sent to the
app.

Note that ResourceFlags perform runtime logic, even in release builds,
meaing that the opportunity for optimizing them at compile time is
much smaller. Tools such as proguard will not have the opportunity
to strip them out entirely.

Bug: 203548827
Test: atest SystemUITests
Change-Id: I3c37ccae16c00237f9cffffc9c22580454ecb8cc
This commit is contained in:
Dave Mankoff
2021-11-17 09:54:44 -05:00
parent 2389bc5b74
commit 62ac77294a
7 changed files with 148 additions and 63 deletions

View File

@@ -16,28 +16,31 @@
package com.android.systemui.flags
import android.annotation.BoolRes
import android.annotation.IntegerRes
import android.annotation.StringRes
import android.os.Parcel
import android.os.Parcelable
interface Flag<T> : Parcelable {
interface Flag<T> {
val id: Int
}
interface ParcelableFlag<T> : Flag<T>, Parcelable {
val default: T
val resourceOverride: Int
override fun describeContents() = 0
}
fun hasResourceOverride(): Boolean {
return resourceOverride != -1
}
interface ResourceFlag<T> : Flag<T> {
val resourceId: Int
}
// Consider using the "parcelize" kotlin library.
data class BooleanFlag @JvmOverloads constructor(
override val id: Int,
override val default: Boolean = false,
override val resourceOverride: Int = -1
) : Flag<Boolean> {
override val default: Boolean = false
) : ParcelableFlag<Boolean> {
companion object {
@JvmField
@@ -58,11 +61,15 @@ data class BooleanFlag @JvmOverloads constructor(
}
}
data class ResourceBooleanFlag constructor(
override val id: Int,
@BoolRes override val resourceId: Int
) : ResourceFlag<Boolean>
data class StringFlag @JvmOverloads constructor(
override val id: Int,
override val default: String = "",
override val resourceOverride: Int = -1
) : Flag<String> {
override val default: String = ""
) : ParcelableFlag<String> {
companion object {
@JvmField
val CREATOR = object : Parcelable.Creator<StringFlag> {
@@ -82,11 +89,15 @@ data class StringFlag @JvmOverloads constructor(
}
}
data class ResourceStringFlag constructor(
override val id: Int,
@StringRes override val resourceId: Int
) : ResourceFlag<String>
data class IntFlag @JvmOverloads constructor(
override val id: Int,
override val default: Int = 0,
override val resourceOverride: Int = -1
) : Flag<Int> {
override val default: Int = 0
) : ParcelableFlag<Int> {
companion object {
@JvmField
@@ -107,11 +118,15 @@ data class IntFlag @JvmOverloads constructor(
}
}
data class ResourceIntFlag constructor(
override val id: Int,
@IntegerRes override val resourceId: Int
) : ResourceFlag<Int>
data class LongFlag @JvmOverloads constructor(
override val id: Int,
override val default: Long = 0,
override val resourceOverride: Int = -1
) : Flag<Long> {
override val default: Long = 0
) : ParcelableFlag<Long> {
companion object {
@JvmField
@@ -134,9 +149,8 @@ data class LongFlag @JvmOverloads constructor(
data class FloatFlag @JvmOverloads constructor(
override val id: Int,
override val default: Float = 0f,
override val resourceOverride: Int = -1
) : Flag<Float> {
override val default: Float = 0f
) : ParcelableFlag<Float> {
companion object {
@JvmField
@@ -157,11 +171,15 @@ data class FloatFlag @JvmOverloads constructor(
}
}
data class ResourceFloatFlag constructor(
override val id: Int,
override val resourceId: Int
) : ResourceFlag<Int>
data class DoubleFlag @JvmOverloads constructor(
override val id: Int,
override val default: Double = 0.0,
override val resourceOverride: Int = -1
) : Flag<Double> {
override val default: Double = 0.0
) : ParcelableFlag<Double> {
companion object {
@JvmField

View File

@@ -60,7 +60,7 @@ class FlagManager constructor(
object : BroadcastReceiver() {
override fun onReceive(context: Context, intent: Intent) {
val extras: Bundle? = getResultExtras(false)
val listOfFlags: java.util.ArrayList<Flag<*>>? =
val listOfFlags: java.util.ArrayList<ParcelableFlag<*>>? =
extras?.getParcelableArrayList(FIELD_FLAGS)
if (listOfFlags != null) {
completer.set(listOfFlags)
@@ -108,6 +108,10 @@ class FlagManager constructor(
}
}
override fun isEnabled(flag: ResourceBooleanFlag): Boolean {
throw RuntimeException("Not implemented in FlagManager")
}
override fun addListener(listener: FlagReader.Listener) {
synchronized(listeners) {
val registerNeeded = listeners.isEmpty()

View File

@@ -24,6 +24,8 @@ interface FlagReader {
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

View File

@@ -22,6 +22,7 @@ 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;
@@ -30,7 +31,6 @@ import android.content.res.Resources;
import android.os.Bundle;
import android.util.Log;
import androidx.annotation.BoolRes;
import androidx.annotation.NonNull;
import com.android.systemui.Dumpable;
@@ -89,16 +89,18 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
public boolean isEnabled(BooleanFlag flag) {
int id = flag.getId();
if (!mBooleanFlagCache.containsKey(id)) {
boolean def = flag.getDefault();
if (flag.hasResourceOverride()) {
try {
def = isEnabledInOverlay(flag.getResourceOverride());
} catch (Resources.NotFoundException e) {
// no-op
}
}
mBooleanFlagCache.put(id, isEnabled(id, flag.getDefault()));
}
mBooleanFlagCache.put(id, isEnabled(id, def));
return mBooleanFlagCache.get(id);
}
@Override
public boolean isEnabled(ResourceBooleanFlag flag) {
int id = flag.getId();
if (!mBooleanFlagCache.containsKey(id)) {
mBooleanFlagCache.put(
id, isEnabled(id, mResources.getBoolean(flag.getResourceId())));
}
return mBooleanFlagCache.get(id);
@@ -111,6 +113,7 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
return result == null ? defaultValue : result;
}
/** Returns the stored value or null if not set. */
private Boolean isEnabledInternal(int id) {
try {
@@ -121,10 +124,6 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
return null;
}
private boolean isEnabledInOverlay(@BoolRes int resId) {
return mResources.getBoolean(resId);
}
/** Set whether a given {@link BooleanFlag} is enabled or not. */
public void setEnabled(int id, boolean value) {
Boolean currentValue = isEnabledInternal(id);
@@ -185,9 +184,19 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
} else if (ACTION_GET_FLAGS.equals(action)) {
Map<Integer, Flag<?>> knownFlagMap = Flags.collectFlags();
ArrayList<Flag<?>> flags = new ArrayList<>(knownFlagMap.values());
// Convert all flags to parcelable flags.
ArrayList<ParcelableFlag<?>> pFlags = new ArrayList<>();
for (Flag<?> f : flags) {
ParcelableFlag<?> pf = toParcelableFlag(f);
if (pf != null) {
pFlags.add(pf);
}
}
Bundle extras = getResultExtras(true);
if (extras != null) {
extras.putParcelableArrayList(FIELD_FLAGS, flags);
extras.putParcelableArrayList(FIELD_FLAGS, pFlags);
}
}
}
@@ -215,6 +224,25 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable {
setEnabled(id, extras.getBoolean(FIELD_VALUE));
}
}
/**
* Ensures that the data we send to the app reflects the current state of the flags.
*
* Also converts an non-parcelable versions of the flags to their parcelable versions.
*/
@Nullable
private ParcelableFlag<?> toParcelableFlag(Flag<?> f) {
if (f instanceof BooleanFlag) {
return new BooleanFlag(f.getId(), isEnabled((BooleanFlag) f));
}
if (f instanceof ResourceBooleanFlag) {
return new BooleanFlag(f.getId(), isEnabled((ResourceBooleanFlag) f));
}
// TODO: add support for other flag types.
Log.w(TAG, "Unsupported Flag Type. Please file a bug.");
return null;
}
};
@Override

View File

@@ -16,12 +16,14 @@
package com.android.systemui.flags;
import android.content.res.Resources;
import android.util.SparseBooleanArray;
import androidx.annotation.NonNull;
import com.android.systemui.Dumpable;
import com.android.systemui.dagger.SysUISingleton;
import com.android.systemui.dagger.qualifiers.Main;
import com.android.systemui.dump.DumpManager;
import java.io.FileDescriptor;
@@ -37,9 +39,11 @@ import javax.inject.Inject;
*/
@SysUISingleton
public class FeatureFlagsRelease implements FeatureFlags, Dumpable {
SparseBooleanArray mAccessedFlags = new SparseBooleanArray();
private final Resources mResources;
SparseBooleanArray mFlagCache = new SparseBooleanArray();
@Inject
public FeatureFlagsRelease(DumpManager dumpManager) {
public FeatureFlagsRelease(@Main Resources resources, DumpManager dumpManager) {
mResources = resources;
dumpManager.registerDumpable("SysUIFlags", this);
}
@@ -54,19 +58,29 @@ public class FeatureFlagsRelease implements FeatureFlags, Dumpable {
return isEnabled(flag.getId(), flag.getDefault());
}
@Override
public boolean isEnabled(ResourceBooleanFlag flag) {
int cacheIndex = mFlagCache.indexOfKey(flag.getId());
if (cacheIndex < 0) {
return isEnabled(flag.getId(), mResources.getBoolean(flag.getResourceId()));
}
return mFlagCache.valueAt(cacheIndex);
}
@Override
public boolean isEnabled(int key, boolean defaultValue) {
mAccessedFlags.append(key, defaultValue);
mFlagCache.append(key, defaultValue);
return defaultValue;
}
@Override
public void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter pw, @NonNull String[] args) {
pw.println("can override: false");
int size = mAccessedFlags.size();
int size = mFlagCache.size();
for (int i = 0; i < size; i++) {
pw.println(" sysui_flag_" + mAccessedFlags.keyAt(i)
+ ": " + mAccessedFlags.valueAt(i));
pw.println(" sysui_flag_" + mFlagCache.keyAt(i)
+ ": " + mFlagCache.valueAt(i));
}
}
}

View File

@@ -61,8 +61,8 @@ public class Flags {
public static final BooleanFlag NEW_UNLOCK_SWIPE_ANIMATION =
new BooleanFlag(202, true);
public static final BooleanFlag CHARGING_RIPPLE =
new BooleanFlag(203, false, R.bool.flag_charging_ripple);
public static final ResourceBooleanFlag CHARGING_RIPPLE =
new ResourceBooleanFlag(203, R.bool.flag_charging_ripple);
/***************************************/
// 300 - power menu
@@ -77,8 +77,8 @@ public class Flags {
public static final BooleanFlag SMARTSPACE_SHARED_ELEMENT_TRANSITION_ENABLED =
new BooleanFlag(401, false);
public static final BooleanFlag SMARTSPACE =
new BooleanFlag(402, false, R.bool.flag_smartspace);
public static final ResourceBooleanFlag SMARTSPACE =
new ResourceBooleanFlag(402, R.bool.flag_smartspace);
/***************************************/
// 500 - quick settings
@@ -88,11 +88,11 @@ public class Flags {
public static final BooleanFlag COMBINED_QS_HEADERS =
new BooleanFlag(501, false);
public static final BooleanFlag PEOPLE_TILE =
new BooleanFlag(502, false, R.bool.flag_conversations);
public static final ResourceBooleanFlag PEOPLE_TILE =
new ResourceBooleanFlag(502, R.bool.flag_conversations);
public static final BooleanFlag QS_USER_DETAIL_SHORTCUT =
new BooleanFlag(503, false, R.bool.flag_lockscreen_qs_user_detail_shortcut);
public static final ResourceBooleanFlag QS_USER_DETAIL_SHORTCUT =
new ResourceBooleanFlag(503, R.bool.flag_lockscreen_qs_user_detail_shortcut);
/***************************************/
// 600- status bar
@@ -115,8 +115,8 @@ public class Flags {
/***************************************/
// 800 - general visual/theme
public static final BooleanFlag MONET =
new BooleanFlag(800, true, R.bool.flag_monet);
public static final ResourceBooleanFlag MONET =
new ResourceBooleanFlag(800, R.bool.flag_monet);
/***************************************/
// 900 - media

View File

@@ -18,15 +18,14 @@ package com.android.systemui.flags;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import android.content.Context;
import android.content.res.Resources;
import androidx.test.filters.SmallTest;
@@ -51,13 +50,14 @@ import java.io.StringWriter;
public class FeatureFlagsReleaseTest extends SysuiTestCase {
FeatureFlagsRelease mFeatureFlagsRelease;
@Mock private Resources mResources;
@Mock private DumpManager mDumpManager;
@Before
public void setup() {
MockitoAnnotations.initMocks(this);
mFeatureFlagsRelease = new FeatureFlagsRelease(mDumpManager);
mFeatureFlagsRelease = new FeatureFlagsRelease(mResources, mDumpManager);
}
@After
@@ -67,16 +67,35 @@ public class FeatureFlagsReleaseTest extends SysuiTestCase {
verifyNoMoreInteractions(mDumpManager);
}
@Test
public void testBooleanResourceFlag() {
int flagId = 213;
int flagResourceId = 3;
ResourceBooleanFlag flag = new ResourceBooleanFlag(flagId, flagResourceId);
when(mResources.getBoolean(flagResourceId)).thenReturn(true);
assertThat(mFeatureFlagsRelease.isEnabled(flag)).isTrue();
}
@Test
public void testDump() {
int flagIdA = 213;
int flagIdB = 18;
int flagResourceId = 3;
BooleanFlag flagA = new BooleanFlag(flagIdA, true);
ResourceBooleanFlag flagB = new ResourceBooleanFlag(flagIdB, flagResourceId);
when(mResources.getBoolean(flagResourceId)).thenReturn(true);
// WHEN the flags have been accessed
assertFalse(mFeatureFlagsRelease.isEnabled(1, false));
assertTrue(mFeatureFlagsRelease.isEnabled(2, true));
assertThat(mFeatureFlagsRelease.isEnabled(1, false)).isFalse();
assertThat(mFeatureFlagsRelease.isEnabled(flagA)).isTrue();
assertThat(mFeatureFlagsRelease.isEnabled(flagB)).isTrue();
// 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_2: true\n");
assertThat(dump).contains(" sysui_flag_" + flagIdA + ": true\n");
assertThat(dump).contains(" sysui_flag_" + flagIdB + ": true\n");
}
private String dumpToString() {