diff --git a/packages/SystemUI/res/values/flags.xml b/packages/SystemUI/res/values/flags.xml new file mode 100644 index 0000000000000..db8bfec2bac5a --- /dev/null +++ b/packages/SystemUI/res/values/flags.xml @@ -0,0 +1,26 @@ + + + + + true + + false + false + + + false + diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java new file mode 100644 index 0000000000000..b77fcc822b880 --- /dev/null +++ b/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 + * false + * } + * + * 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 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_"; +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java b/packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java index bbc4b780bc524..964c499e4b42c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/FeatureFlags.java +++ b/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 -* } - * - * 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 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); } } diff --git a/packages/SystemUI/src/com/android/systemui/util/wrapper/BuildInfo.java b/packages/SystemUI/src/com/android/systemui/util/wrapper/BuildInfo.java new file mode 100644 index 0000000000000..5e68a15f8ec77 --- /dev/null +++ b/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; + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java new file mode 100644 index 0000000000000..c79037b761aa0 --- /dev/null +++ b/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 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 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; +}