Converts FeatureFlags to use overlayable resource flags

Updates the FeatureFlags wrapper to be backed by resource booleans.
These flags can be modified via RRO to create different build
configurations.

These flags can be locally overridden on all configurations where
are_flags_overrideable is true. It's intended that this value is
true for development builds but not production ones.

Test: atest SystemUITests:FeatureFlagReaderTest
Bug: 177954419
Change-Id: I847fa2659c0ca000431681f24f460269efbd49af

packages/SystemUI/res/values/flags.xml

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  ~ 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.
+  -->
+
+<resources>
+    <bool name="are_flags_overrideable">true</bool>
+
+    <bool name="flag_notification_pipeline2">false</bool>
+    <bool name="flag_notification_pipeline2_rendering">false</bool>
+
+    <!-- b/171917882 -->
+    <bool name="flag_notification_twocolumn">false</bool>
+</resources>

packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java

@@ -0,0 +1,165 @@
+/*
+ * 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.annotation.NonNull;
+import android.content.res.Resources;
+import android.provider.DeviceConfig;
+import android.util.SparseArray;
+
+import androidx.annotation.BoolRes;
+import androidx.annotation.Nullable;
+
+import com.android.systemui.R;
+import com.android.systemui.assist.DeviceConfigHelper;
+import com.android.systemui.dagger.SysUISingleton;
+import com.android.systemui.dagger.qualifiers.Background;
+import com.android.systemui.dagger.qualifiers.Main;
+import com.android.systemui.statusbar.FeatureFlags;
+import com.android.systemui.util.wrapper.BuildInfo;
+
+import java.util.concurrent.Executor;
+
+import javax.inject.Inject;
+/**
+ * Reads and caches feature flags for quick access
+ *
+ * Feature flags must be defined as boolean resources. For example:
+ *
+ * {@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 device_config put systemui flag_foo_bar_baz true
+ * }
+ *
+ * Note that all storage keys begin with "flag_", even if their associated resId does not.
+ *
+ * Calls to this class should probably be wrapped by {@link FeatureFlags}.
+ */
+@SysUISingleton
+public class FeatureFlagReader {
+    private final Resources mResources;
+    private final DeviceConfigHelper mDeviceConfig;
+    private final boolean mAreFlagsOverrideable;
+
+    private final SparseArray<CachedFlag> mCachedFlags = new SparseArray<>();
+
+    @Inject
+    public FeatureFlagReader(
+            @Main Resources resources,
+            BuildInfo build,
+            DeviceConfigHelper deviceConfig,
+            @Background Executor executor) {
+        mResources = resources;
+        mDeviceConfig = deviceConfig;
+        mAreFlagsOverrideable =
+                build.isDebuggable() && mResources.getBoolean(R.bool.are_flags_overrideable);
+
+        mDeviceConfig.addOnPropertiesChangedListener(executor, this::onPropertiesChanged);
+    }
+
+    /**
+     * 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 = mDeviceConfig.getBoolean(flagNameToStorageKey(name), value);
+                }
+
+                cachedFlag = new CachedFlag(name, value);
+                mCachedFlags.put(resId, cachedFlag);
+            }
+
+            return cachedFlag.value;
+        }
+    }
+
+    private void onPropertiesChanged(@NonNull DeviceConfig.Properties properties) {
+        synchronized (mCachedFlags) {
+            for (String key : properties.getKeyset()) {
+                String flagName = storageKeyToFlagName(key);
+                if (flagName != null) {
+                    clearCachedFlag(flagName);
+                }
+            }
+        }
+    }
+
+    private void clearCachedFlag(String flagName) {
+        for (int i = 0; i < mCachedFlags.size(); i++) {
+            CachedFlag flag = mCachedFlags.valueAt(i);
+            if (flag.name.equals(flagName)) {
+                mCachedFlags.removeAt(i);
+                break;
+            }
+        }
+    }
+
+    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;
+        }
+    }
+
+    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 = "flag_";
+    private static final String RESNAME_PREFIX = "flag_";
+}

packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java

@@ -16,78 +16,36 @@
 
 package com.android.systemui.statusbar;
 
-import android.annotation.NonNull;
-import android.provider.DeviceConfig;
-import android.util.ArrayMap;
-
+import com.android.systemui.R;
 import com.android.systemui.dagger.SysUISingleton;
-import com.android.systemui.dagger.qualifiers.Background;
-
-import java.util.Map;
-import java.util.concurrent.Executor;
+import com.android.systemui.flags.FeatureFlagReader;
 
 import javax.inject.Inject;
 
 /**
  * Class to manage simple DeviceConfig-based feature flags.
  *
- * To enable or disable a flag, run:
- *
- * {@code
- *  $ adb shell device_config put systemui <key> <true|false>
-*  }
- *
- * You will probably need to restart systemui for the changes to be picked up:
- *
- * {@code
- *  $ adb shell am restart com.android.systemui
- * }
+ * See {@link FeatureFlagReader} for instructions on defining and flipping flags.
  */
 @SysUISingleton
 public class FeatureFlags {
-    private final Map<String, Boolean> mCachedDeviceConfigFlags = new ArrayMap<>();
+    private final FeatureFlagReader mFlagReader;
 
     @Inject
-    public FeatureFlags(@Background Executor executor) {
-        DeviceConfig.addOnPropertiesChangedListener(
-                /* namespace= */ "systemui",
-                executor,
-                this::onPropertiesChanged);
+    public FeatureFlags(FeatureFlagReader flagReader) {
+        mFlagReader = flagReader;
     }
 
     public boolean isNewNotifPipelineEnabled() {
-        return getDeviceConfigFlag("notification.newpipeline.enabled", /* defaultValue= */ true);
+        return mFlagReader.isEnabled(R.bool.flag_notification_pipeline2);
     }
 
     public boolean isNewNotifPipelineRenderingEnabled() {
-        return isNewNotifPipelineEnabled()
-                && getDeviceConfigFlag("notification.newpipeline.rendering", /* defaultValue= */
-                false);
+        return mFlagReader.isEnabled(R.bool.flag_notification_pipeline2_rendering);
     }
 
-    /**
-     * Flag used for guarding development of b/171917882.
-     */
+    /** b/171917882 */
     public boolean isTwoColumnNotificationShadeEnabled() {
-        return getDeviceConfigFlag("notification.twocolumn", /* defaultValue= */ false);
-    }
-
-    private void onPropertiesChanged(@NonNull DeviceConfig.Properties properties) {
-        synchronized (mCachedDeviceConfigFlags) {
-            for (String key : properties.getKeyset()) {
-                mCachedDeviceConfigFlags.remove(key);
-            }
-        }
-    }
-
-    private boolean getDeviceConfigFlag(String key, boolean defaultValue) {
-        synchronized (mCachedDeviceConfigFlags) {
-            Boolean flag = mCachedDeviceConfigFlags.get(key);
-            if (flag == null) {
-                flag = DeviceConfig.getBoolean(/* namespace= */ "systemui", key, defaultValue);
-                mCachedDeviceConfigFlags.put(key, flag);
-            }
-            return flag;
-        }
+        return mFlagReader.isEnabled(R.bool.flag_notification_twocolumn);
     }
 }

packages/SystemUI/src/com/android/systemui/util/wrapper/BuildInfo.java

@@ -0,0 +1,37 @@
+/*
+ * 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.util.wrapper;
+
+import android.os.Build;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+/**
+ * Testable wrapper around {@link Build}.
+ */
+@Singleton
+public class BuildInfo {
+    @Inject
+    public BuildInfo() {
+    }
+
+    /** @see Build#IS_DEBUGGABLE */
+    public boolean isDebuggable() {
+        return Build.IS_DEBUGGABLE;
+    }
+}

packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java

@@ -0,0 +1,175 @@
+/*
+ * 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.any;
+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 android.provider.DeviceConfig;
+
+import androidx.annotation.BoolRes;
+import androidx.test.filters.SmallTest;
+
+import com.android.systemui.R;
+import com.android.systemui.SysuiTestCase;
+import com.android.systemui.assist.DeviceConfigHelper;
+import com.android.systemui.util.wrapper.BuildInfo;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.Executor;
+
+@SmallTest
+public class FeatureFlagReaderTest extends SysuiTestCase {
+    @Mock private Resources mResources;
+    @Mock private BuildInfo mBuildInfo;
+    @Mock private DeviceConfigHelper mDeviceConfig;
+    @Mock private Executor mBackgroundExecutor;
+
+    private FeatureFlagReader mReader;
+
+    @Captor private ArgumentCaptor<DeviceConfig.OnPropertiesChangedListener> mListenerCaptor;
+    private DeviceConfig.OnPropertiesChangedListener mPropChangeListener;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        when(mDeviceConfig.getBoolean(anyString(), anyBoolean()))
+                .thenAnswer(invocation -> invocation.getArgument(1));
+
+        defineFlag(FLAG_RESID_0, false);
+        defineFlag(FLAG_RESID_1, true);
+
+        initialize(true, true);
+
+        verify(mDeviceConfig).addOnPropertiesChangedListener(any(), mListenerCaptor.capture());
+        mPropChangeListener = mListenerCaptor.getValue();
+    }
+
+    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, mDeviceConfig, mBackgroundExecutor);
+    }
+
+    @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(mDeviceConfig, times(1)).getBoolean(fakeStorageKey(FLAG_RESID_0), false);
+    }
+
+    @Test
+    public void testCachesAreClearedAfterPropsChange() {
+        // GIVEN a flag whose value has already been queried
+        assertFalse(mReader.isEnabled(FLAG_RESID_0));
+
+        // WHEN the value of the flag changes
+        overrideFlag(FLAG_RESID_0, true);
+        Map<String, String> changedMap = new HashMap<>();
+        changedMap.put(fakeStorageKey(FLAG_RESID_0), "true");
+        DeviceConfig.Properties properties =
+                new DeviceConfig.Properties("systemui", changedMap);
+        mPropChangeListener.onPropertiesChanged(properties);
+
+        // THEN the new value is provided
+        assertTrue(mReader.isEnabled(FLAG_RESID_0));
+    }
+
+    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(mDeviceConfig.getBoolean(eq(fakeStorageKey(resId)), anyBoolean()))
+                .thenReturn(value);
+    }
+
+    private String fakeStorageKey(@BoolRes int resId) {
+        return "flag_testname_" + resId;
+    }
+
+    private static final int FLAG_RESID_0 = 47;
+    private static final int FLAG_RESID_1 = 48;
+}