Merge "Remove FeatureFlagReader. Allow resource overlays." into sc-v2-dev

This commit is contained in:
Dave Mankoff
2021-11-09 19:02:53 +00:00
committed by Android (Google) Code Review
8 changed files with 99 additions and 361 deletions

View File

@@ -50,17 +50,6 @@ java_library {
srcs: ["src/com/android/systemui/EventLogTags.logtags"],
}
java_library {
name: "SystemUI-flags",
srcs: [
"src/com/android/systemui/flags/Flags.java",
],
libs: [
"SystemUI-flag-types",
],
static_kotlin_stdlib: false,
}
filegroup {
name: "ReleaseJavaFiles",
srcs: [
@@ -126,7 +115,6 @@ android_library {
"iconloader_base",
"SystemUI-tags",
"SystemUI-proto",
"SystemUI-flags",
"monet",
"dagger2",
"jsr330",

View File

@@ -45,9 +45,6 @@ android_library {
":wm_shell-aidls",
":wm_shell_util-sources",
],
libs: [
"SystemUI-flags",
],
static_libs: [
"PluginCoreLib",
"androidx.dynamicanimation_dynamicanimation",
@@ -75,7 +72,6 @@ java_library {
],
static_kotlin_stdlib: false,
libs: [
"SystemUI-flags",
"androidx.concurrent_concurrent-futures",
],
static_libs: [

View File

@@ -22,15 +22,21 @@ import android.os.Parcelable
interface Flag<T> : Parcelable {
val id: Int
val default: T
val resourceOverride: Int
override fun describeContents() = 0
fun hasResourceOverride(): Boolean {
return resourceOverride != -1
}
}
// Consider using the "parcelize" kotlin library.
data class BooleanFlag @JvmOverloads constructor(
override val id: Int,
override val default: Boolean = false
override val default: Boolean = false,
override val resourceOverride: Int = -1
) : Flag<Boolean> {
companion object {
@@ -54,7 +60,8 @@ data class BooleanFlag @JvmOverloads constructor(
data class StringFlag @JvmOverloads constructor(
override val id: Int,
override val default: String = ""
override val default: String = "",
override val resourceOverride: Int = -1
) : Flag<String> {
companion object {
@JvmField
@@ -77,7 +84,8 @@ data class StringFlag @JvmOverloads constructor(
data class IntFlag @JvmOverloads constructor(
override val id: Int,
override val default: Int = 0
override val default: Int = 0,
override val resourceOverride: Int = -1
) : Flag<Int> {
companion object {
@@ -101,7 +109,8 @@ data class IntFlag @JvmOverloads constructor(
data class LongFlag @JvmOverloads constructor(
override val id: Int,
override val default: Long = 0
override val default: Long = 0,
override val resourceOverride: Int = -1
) : Flag<Long> {
companion object {
@@ -125,7 +134,8 @@ data class LongFlag @JvmOverloads constructor(
data class FloatFlag @JvmOverloads constructor(
override val id: Int,
override val default: Float = 0f
override val default: Float = 0f,
override val resourceOverride: Int = -1
) : Flag<Float> {
companion object {
@@ -149,7 +159,8 @@ data class FloatFlag @JvmOverloads constructor(
data class DoubleFlag @JvmOverloads constructor(
override val id: Int,
override val default: Double = 0.0
override val default: Double = 0.0,
override val resourceOverride: Int = -1
) : Flag<Double> {
companion object {

View File

@@ -1,175 +0,0 @@
/*
* 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.res.Resources;
import android.util.SparseArray;
import androidx.annotation.BoolRes;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.android.systemui.Dumpable;
import com.android.systemui.R;
import com.android.systemui.dagger.SysUISingleton;
import com.android.systemui.dagger.qualifiers.Main;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.util.wrapper.BuildInfo;
import java.io.FileDescriptor;
import java.io.PrintWriter;
import java.util.ArrayList;
import javax.inject.Inject;
/**
* Reads and caches feature flags for quick access
*
* Feature flags must be defined as boolean resources. For example:t
*
* {@code
* <bool name="flag_foo_bar_baz">false</bool>
* }
*
* It is strongly recommended that the name of the resource begin with "flag_".
*
* Flags can be overridden via adb on development builds. For example, to override the flag from the
* previous example, do the following:
*
* {@code
* $ adb shell setprop persist.systemui.flag_foo_bar_baz 1
* }
*
* Note that all storage keys begin with "flag_", even if their associated resId does not.
*
* Calls to this class should probably be wrapped by a method in {@link FeatureFlags}.
*/
@SysUISingleton
public class FeatureFlagReader implements Dumpable {
private final Resources mResources;
private final boolean mAreFlagsOverrideable;
private final SystemPropertiesHelper mSystemPropertiesHelper;
private final SparseArray<CachedFlag> mCachedFlags = new SparseArray<>();
private final FlagReader mFlagReader;
@Inject
public FeatureFlagReader(
@Main Resources resources,
BuildInfo build,
DumpManager dumpManager,
SystemPropertiesHelper systemPropertiesHelper,
FlagReader reader) {
mResources = resources;
mFlagReader = reader;
mSystemPropertiesHelper = systemPropertiesHelper;
mAreFlagsOverrideable =
build.isDebuggable() && mResources.getBoolean(R.bool.are_flags_overrideable);
dumpManager.registerDumpable("FeatureFlags", this);
}
boolean isEnabled(BooleanFlag flag) {
return mFlagReader.isEnabled(flag.getId(), flag.getDefault());
}
void addListener(FlagReader.Listener listener) {
mFlagReader.addListener(listener);
}
void removeListener(FlagReader.Listener listener) {
mFlagReader.removeListener(listener);
}
/**
* Returns true if the specified feature flag has been enabled.
*
* @param resId The backing boolean resource that determines the value of the flag. This value
* can be overridden via DeviceConfig on development builds.
*/
public boolean isEnabled(@BoolRes int resId) {
synchronized (mCachedFlags) {
CachedFlag cachedFlag = mCachedFlags.get(resId);
if (cachedFlag == null) {
String name = resourceIdToFlagName(resId);
boolean value = mResources.getBoolean(resId);
if (mAreFlagsOverrideable) {
value = mSystemPropertiesHelper.getBoolean(flagNameToStorageKey(name), value);
}
cachedFlag = new CachedFlag(name, value);
mCachedFlags.put(resId, cachedFlag);
}
return cachedFlag.value;
}
}
private String resourceIdToFlagName(@BoolRes int resId) {
String resName = mResources.getResourceEntryName(resId);
if (resName.startsWith(RESNAME_PREFIX)) {
resName = resName.substring(RESNAME_PREFIX.length());
}
return resName;
}
private String flagNameToStorageKey(String flagName) {
if (flagName.startsWith(STORAGE_KEY_PREFIX)) {
return flagName;
} else {
return STORAGE_KEY_PREFIX + flagName;
}
}
@Nullable
private String storageKeyToFlagName(String configName) {
if (configName.startsWith(STORAGE_KEY_PREFIX)) {
return configName.substring(STORAGE_KEY_PREFIX.length());
} else {
return null;
}
}
@Override
public void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter pw, @NonNull String[] args) {
ArrayList<String> flagStrings = new ArrayList<>(mCachedFlags.size());
for (int i = 0; i < mCachedFlags.size(); i++) {
int key = mCachedFlags.keyAt(i);
// get the object by the key.
CachedFlag flag = mCachedFlags.get(key);
flagStrings.add(" " + RESNAME_PREFIX + flag.name + ": " + flag.value + "\n");
}
flagStrings.sort(String.CASE_INSENSITIVE_ORDER);
pw.println("AreFlagsOverrideable: " + mAreFlagsOverrideable);
pw.println("Cached FeatureFlags:");
for (String flagString : flagStrings) {
pw.print(flagString);
}
}
private static class CachedFlag {
public final String name;
public final boolean value;
private CachedFlag(String name, boolean value) {
this.name = name;
this.value = value;
}
}
private static final String STORAGE_KEY_PREFIX = "persist.systemui.flag_";
private static final String RESNAME_PREFIX = "flag_";
}

View File

@@ -17,13 +17,17 @@
package com.android.systemui.flags;
import android.content.Context;
import android.content.res.Resources;
import android.util.FeatureFlagUtils;
import android.util.Log;
import android.util.SparseArray;
import android.widget.Toast;
import androidx.annotation.BoolRes;
import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.R;
import com.android.systemui.dagger.SysUISingleton;
import com.android.systemui.dagger.qualifiers.Main;
import java.util.ArrayList;
import java.util.HashMap;
@@ -39,13 +43,16 @@ import javax.inject.Inject;
*/
@SysUISingleton
public class FeatureFlags {
private final FeatureFlagReader mFlagReader;
private final Resources mResources;
private final FlagReader mFlagReader;
private final Context mContext;
private final Map<Integer, Flag<?>> mFlagMap = new HashMap<>();
private final Map<Integer, List<Listener>> mListeners = new HashMap<>();
private final SparseArray<Boolean> mCachedFlags = new SparseArray<>();
@Inject
public FeatureFlags(FeatureFlagReader flagReader, Context context) {
public FeatureFlags(@Main Resources resources, FlagReader flagReader, Context context) {
mResources = resources;
mFlagReader = flagReader;
mContext = context;
@@ -59,7 +66,7 @@ public class FeatureFlags {
};
@VisibleForTesting
void addFlag(Flag flag) {
void addFlag(Flag<?> flag) {
mFlagMap.put(flag.getId(), flag);
}
@@ -68,7 +75,15 @@ public class FeatureFlags {
* @return The value of the flag.
*/
public boolean isEnabled(BooleanFlag flag) {
return mFlagReader.isEnabled(flag);
boolean def = flag.getDefault();
if (flag.hasResourceOverride()) {
try {
def = isEnabledInOverlay(flag.getResourceOverride());
} catch (Resources.NotFoundException e) {
// no-op
}
}
return mFlagReader.isEnabled(flag.getId(), def);
}
/**
@@ -118,13 +133,11 @@ public class FeatureFlags {
}
public boolean isPeopleTileEnabled() {
// TODO(b/202860494): different resource overlays have different values.
return mFlagReader.isEnabled(R.bool.flag_conversations);
return isEnabled(Flags.PEOPLE_TILE);
}
public boolean isMonetEnabled() {
// TODO(b/202860494): used in wallpaper picker. Always true, maybe delete.
return mFlagReader.isEnabled(R.bool.flag_monet);
return isEnabled(Flags.MONET);
}
public boolean isPMLiteEnabled() {
@@ -132,8 +145,7 @@ public class FeatureFlags {
}
public boolean isChargingRippleEnabled() {
// TODO(b/202860494): different resource overlays have different values.
return mFlagReader.isEnabled(R.bool.flag_charging_ripple);
return isEnabled(Flags.CHARGING_RIPPLE);
}
public boolean isOngoingCallStatusBarChipEnabled() {
@@ -150,8 +162,7 @@ public class FeatureFlags {
}
public boolean isSmartspaceEnabled() {
// TODO(b/202860494): different resource overlays have different values.
return mFlagReader.isEnabled(R.bool.flag_smartspace);
return isEnabled(Flags.SMARTSPACE);
}
public boolean isSmartspaceDedupingEnabled() {
@@ -195,6 +206,16 @@ public class FeatureFlags {
return FeatureFlagUtils.isEnabled(context, FeatureFlagUtils.SETTINGS_PROVIDER_MODEL);
}
private boolean isEnabledInOverlay(@BoolRes int resId) {
synchronized (mCachedFlags) {
if (!mCachedFlags.contains(resId)) {
mCachedFlags.put(resId, mResources.getBoolean(resId));
}
return mCachedFlags.get(resId);
}
}
/** Simple interface for beinga alerted when a specific flag changes value. */
public interface Listener {
/** */

View File

@@ -16,6 +16,8 @@
package com.android.systemui.flags;
import com.android.systemui.R;
import java.lang.reflect.Field;
import java.util.HashMap;
import java.util.Map;
@@ -47,7 +49,6 @@ public class Flags {
public static final BooleanFlag NOTIFICATION_UPDATES =
new BooleanFlag(102, true);
/***************************************/
// 200 - keyguard/lockscreen
public static final BooleanFlag KEYGUARD_LAYOUT =
@@ -59,6 +60,9 @@ 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);
/***************************************/
// 300 - power menu
public static final BooleanFlag POWER_MENU_LITE =
@@ -72,6 +76,9 @@ 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);
/***************************************/
// 500 - quick settings
public static final BooleanFlag NEW_USER_SWITCHER =
@@ -80,6 +87,9 @@ 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);
/***************************************/
// 600- status bar
public static final BooleanFlag COMBINED_STATUS_BAR_SIGNAL_ICONS =
@@ -96,6 +106,11 @@ public class Flags {
public static final BooleanFlag ONGOING_CALL_IN_IMMERSIVE_CHIP_TAP =
new BooleanFlag(702, true);
/***************************************/
// 800 - general visual/theme
public static final BooleanFlag MONET =
new BooleanFlag(800, true, R.bool.flag_monet);
// Pay no attention to the reflection behind the curtain.
// ========================== Curtain ==========================
// | |

View File

@@ -1,148 +0,0 @@
/*
* 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 static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.content.res.Resources;
import androidx.annotation.BoolRes;
import androidx.test.filters.SmallTest;
import com.android.systemui.R;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.shared.plugins.PluginManager;
import com.android.systemui.util.wrapper.BuildInfo;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@SmallTest
public class FeatureFlagReaderTest extends SysuiTestCase {
@Mock private Resources mResources;
@Mock private BuildInfo mBuildInfo;
@Mock private DumpManager mDumpManager;
@Mock private SystemPropertiesHelper mSystemPropertiesHelper;
@Mock private FlagReader mFlagReader;
private FeatureFlagReader mReader;
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
when(mSystemPropertiesHelper.getBoolean(anyString(), anyBoolean()))
.thenAnswer(invocation -> invocation.getArgument(1));
defineFlag(FLAG_RESID_0, false);
defineFlag(FLAG_RESID_1, true);
initialize(true, true);
}
private void initialize(boolean isDebuggable, boolean isOverrideable) {
when(mBuildInfo.isDebuggable()).thenReturn(isDebuggable);
when(mResources.getBoolean(R.bool.are_flags_overrideable)).thenReturn(isOverrideable);
mReader = new FeatureFlagReader(
mResources, mBuildInfo, mDumpManager, mSystemPropertiesHelper, mFlagReader);
}
@Test
public void testCantOverrideIfNotDebuggable() {
// GIVEN that the build is not debuggable
initialize(false, true);
// GIVEN that a flag has been overridden to true
overrideFlag(FLAG_RESID_0, true);
// THEN the flag is still false
assertFalse(mReader.isEnabled(FLAG_RESID_0));
}
@Test
public void testCantOverrideIfNotOverrideable() {
// GIVEN that flags are not overrideable
initialize(true, false);
// GIVEN that a flag has been overridden to true
overrideFlag(FLAG_RESID_0, true);
// THEN the flag is still false
assertFalse(mReader.isEnabled(FLAG_RESID_0));
}
@Test
public void testReadFlags() {
assertFalse(mReader.isEnabled(FLAG_RESID_0));
assertTrue(mReader.isEnabled(FLAG_RESID_1));
}
@Test
public void testOverrideFlags() {
// GIVEN that flags are overridden
overrideFlag(FLAG_RESID_0, true);
overrideFlag(FLAG_RESID_1, false);
// THEN the reader returns the overridden values
assertTrue(mReader.isEnabled(FLAG_RESID_0));
assertFalse(mReader.isEnabled(FLAG_RESID_1));
}
@Test
public void testThatFlagReadsAreCached() {
// GIVEN that a flag is overridden
overrideFlag(FLAG_RESID_0, true);
// WHEN the flag is queried many times
mReader.isEnabled(FLAG_RESID_0);
mReader.isEnabled(FLAG_RESID_0);
mReader.isEnabled(FLAG_RESID_0);
mReader.isEnabled(FLAG_RESID_0);
// THEN the underlying resource and override are only queried once
verify(mResources, times(1)).getBoolean(FLAG_RESID_0);
verify(mSystemPropertiesHelper, times(1))
.getBoolean(fakeStorageKey(FLAG_RESID_0), false);
}
private void defineFlag(int resId, boolean value) {
when(mResources.getBoolean(resId)).thenReturn(value);
when(mResources.getResourceEntryName(resId)).thenReturn(fakeStorageKey(resId));
}
private void overrideFlag(int resId, boolean value) {
when(mSystemPropertiesHelper.getBoolean(eq(fakeStorageKey(resId)), anyBoolean()))
.thenReturn(value);
}
private String fakeStorageKey(@BoolRes int resId) {
return "persist.systemui.flag_testname_" + resId;
}
private static final int FLAG_RESID_0 = 47;
private static final int FLAG_RESID_1 = 48;
}

View File

@@ -18,7 +18,12 @@ package com.android.systemui.flags;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.content.res.Resources;
import androidx.test.filters.SmallTest;
@@ -29,11 +34,13 @@ import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.stubbing.Answer;
@SmallTest
public class FeatureFlagsTest extends SysuiTestCase {
@Mock FeatureFlagReader mFeatureFlagReader;
@Mock Resources mResources;
@Mock FlagReader mFeatureFlagReader;
private FeatureFlags mFeatureFlags;
@@ -41,7 +48,10 @@ public class FeatureFlagsTest extends SysuiTestCase {
public void setup() {
MockitoAnnotations.initMocks(this);
mFeatureFlags = new FeatureFlags(mFeatureFlagReader, getContext());
when(mFeatureFlagReader.isEnabled(anyInt(), anyBoolean())).thenAnswer(
(Answer<Boolean>) invocation -> invocation.getArgument(1));
mFeatureFlags = new FeatureFlags(mResources, mFeatureFlagReader, getContext());
}
@Test
@@ -103,6 +113,26 @@ public class FeatureFlagsTest extends SysuiTestCase {
pluginListener.onFlagChanged(flag.getId());
// Assert that the change was not triggered
assertThat(changedFlag[0]).isNull();
}
@Test
public void testBooleanDefault() {
BooleanFlag flag = new BooleanFlag(1, true);
mFeatureFlags.addFlag(flag);
assertThat(mFeatureFlags.isEnabled(flag)).isTrue();
}
@Test
public void testBooleanResourceOverlay() {
int resourceId = 12;
BooleanFlag flag = new BooleanFlag(1, false, resourceId);
when(mResources.getBoolean(resourceId)).thenReturn(true);
when(mResources.getResourceEntryName(resourceId)).thenReturn("flag");
mFeatureFlags.addFlag(flag);
assertThat(mFeatureFlags.isEnabled(flag)).isTrue();
}
}