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 9574101b466c8..b611c9659db89 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt @@ -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 : Parcelable { +interface Flag { val id: Int +} + +interface ParcelableFlag : Flag, Parcelable { val default: T - val resourceOverride: Int - override fun describeContents() = 0 +} - fun hasResourceOverride(): Boolean { - return resourceOverride != -1 - } +interface ResourceFlag : Flag { + 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 { + override val default: Boolean = false +) : ParcelableFlag { 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 + data class StringFlag @JvmOverloads constructor( override val id: Int, - override val default: String = "", - override val resourceOverride: Int = -1 -) : Flag { + override val default: String = "" +) : ParcelableFlag { companion object { @JvmField val CREATOR = object : Parcelable.Creator { @@ -82,11 +89,15 @@ data class StringFlag @JvmOverloads constructor( } } +data class ResourceStringFlag constructor( + override val id: Int, + @StringRes override val resourceId: Int +) : ResourceFlag + data class IntFlag @JvmOverloads constructor( override val id: Int, - override val default: Int = 0, - override val resourceOverride: Int = -1 -) : Flag { + override val default: Int = 0 +) : ParcelableFlag { 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 + data class LongFlag @JvmOverloads constructor( override val id: Int, - override val default: Long = 0, - override val resourceOverride: Int = -1 -) : Flag { + override val default: Long = 0 +) : ParcelableFlag { 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 { + override val default: Float = 0f +) : ParcelableFlag { 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 + data class DoubleFlag @JvmOverloads constructor( override val id: Int, - override val default: Double = 0.0, - override val resourceOverride: Int = -1 -) : Flag { + override val default: Double = 0.0 +) : ParcelableFlag { companion object { @JvmField 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 e61cb5c9a53eb..b2ca2d7765d9f 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt @@ -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>? = + val listOfFlags: java.util.ArrayList>? = 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() diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt index 91a391272be7e..26c6a4b096fc8 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt @@ -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 diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java index 0ee47a7eea187..3f00b87188d3f 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java @@ -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> knownFlagMap = Flags.collectFlags(); ArrayList> flags = new ArrayList<>(knownFlagMap.values()); + + // Convert all flags to parcelable flags. + ArrayList> 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 diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java index bd6cb66570516..5b6404fa6a1f5 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java @@ -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)); } } } diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.java b/packages/SystemUI/src/com/android/systemui/flags/Flags.java index 458cdc1f78147..c2934931662bf 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.java +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.java @@ -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 diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.java b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.java index 475dde204185e..dea42f969866b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.java @@ -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() {