Merge "Better support for Resource based flags."

This commit is contained in:
Dave Mankoff
2021-12-03 15:26:13 +00:00
committed by Android (Google) Code Review
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() {