diff options
11 files changed, 264 insertions, 64 deletions
diff --git a/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt index bb3df8f0358a..7b216017df7d 100644 --- a/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt +++ b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt @@ -18,17 +18,15 @@ package com.android.systemui.flags import android.content.Context import android.os.Handler -import com.android.internal.statusbar.IStatusBarService import com.android.systemui.dagger.qualifiers.Main -import com.android.systemui.flags.FeatureFlagsDebug.ALL_FLAGS import com.android.systemui.util.settings.SettingsUtilModule import dagger.Binds import dagger.Module import dagger.Provides -import javax.inject.Named @Module(includes = [ FeatureFlagsDebugStartableModule::class, + FlagsCommonModule::class, ServerFlagReaderModule::class, SettingsUtilModule::class, ]) @@ -43,20 +41,5 @@ abstract class FlagsModule { fun provideFlagManager(context: Context, @Main handler: Handler): FlagManager { return FlagManager(context, handler) } - - @JvmStatic - @Provides - @Named(ALL_FLAGS) - fun providesAllFlags(): Map<Int, Flag<*>> = Flags.collectFlags() - - @JvmStatic - @Provides - fun providesRestarter(barService: IStatusBarService): Restarter { - return object: Restarter { - override fun restart() { - barService.restart() - } - } - } } } diff --git a/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt b/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt index 0f7e732fceb1..aef887667527 100644 --- a/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt +++ b/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt @@ -16,29 +16,15 @@ package com.android.systemui.flags -import com.android.internal.statusbar.IStatusBarService import dagger.Binds import dagger.Module -import dagger.Provides @Module(includes = [ FeatureFlagsReleaseStartableModule::class, + FlagsCommonModule::class, ServerFlagReaderModule::class ]) abstract class FlagsModule { @Binds abstract fun bindsFeatureFlagRelease(impl: FeatureFlagsRelease): FeatureFlags - - @Module - companion object { - @JvmStatic - @Provides - fun providesRestarter(barService: IStatusBarService): Restarter { - return object: Restarter { - override fun restart() { - barService.restart() - } - } - } - } } diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java index 3adeeac2e4d4..1f061e9ff1c6 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java @@ -21,6 +21,7 @@ import static com.android.systemui.flags.FlagManager.ACTION_SET_FLAG; import static com.android.systemui.flags.FlagManager.EXTRA_FLAGS; import static com.android.systemui.flags.FlagManager.EXTRA_ID; import static com.android.systemui.flags.FlagManager.EXTRA_VALUE; +import static com.android.systemui.flags.FlagsCommonModule.ALL_FLAGS; import static java.util.Objects.requireNonNull; @@ -59,20 +60,20 @@ import javax.inject.Named; * * Flags can be set (or unset) via the following adb command: * - * adb shell cmd statusbar flag <id> <on|off|toggle|erase> + * adb shell cmd statusbar flag <id> <on|off|toggle|erase> * - * Alternatively, you can change flags via a broadcast intent: + * Alternatively, you can change flags via a broadcast intent: * - * adb shell am broadcast -a com.android.systemui.action.SET_FLAG --ei id <id> [--ez value <0|1>] + * adb shell am broadcast -a com.android.systemui.action.SET_FLAG --ei id <id> [--ez value <0|1>] * * To restore a flag back to its default, leave the `--ez value <0|1>` off of the command. */ @SysUISingleton public class FeatureFlagsDebug implements FeatureFlags { static final String TAG = "SysUIFlags"; - static final String ALL_FLAGS = "all_flags"; private final FlagManager mFlagManager; + private final Context mContext; private final SecureSettings mSecureSettings; private final Resources mResources; private final SystemPropertiesHelper mSystemProperties; @@ -83,6 +84,14 @@ public class FeatureFlagsDebug implements FeatureFlags { private final Map<Integer, String> mStringFlagCache = new TreeMap<>(); private final Restarter mRestarter; + private final ServerFlagReader.ChangeListener mOnPropertiesChanged = + new ServerFlagReader.ChangeListener() { + @Override + public void onChange() { + mRestarter.restart(); + } + }; + @Inject public FeatureFlagsDebug( FlagManager flagManager, @@ -93,23 +102,28 @@ public class FeatureFlagsDebug implements FeatureFlags { DeviceConfigProxy deviceConfigProxy, ServerFlagReader serverFlagReader, @Named(ALL_FLAGS) Map<Integer, Flag<?>> allFlags, - Restarter barService) { + Restarter restarter) { mFlagManager = flagManager; + mContext = context; mSecureSettings = secureSettings; mResources = resources; mSystemProperties = systemProperties; mDeviceConfigProxy = deviceConfigProxy; mServerFlagReader = serverFlagReader; mAllFlags = allFlags; - mRestarter = barService; + mRestarter = restarter; + } + /** Call after construction to setup listeners. */ + void init() { IntentFilter filter = new IntentFilter(); filter.addAction(ACTION_SET_FLAG); filter.addAction(ACTION_GET_FLAGS); - flagManager.setOnSettingsChangedAction(this::restartSystemUI); - flagManager.setClearCacheAction(this::removeFromCache); - context.registerReceiver(mReceiver, filter, null, null, + mFlagManager.setOnSettingsChangedAction(this::restartSystemUI); + mFlagManager.setClearCacheAction(this::removeFromCache); + mContext.registerReceiver(mReceiver, filter, null, null, Context.RECEIVER_EXPORTED_UNAUDITED); + mServerFlagReader.listenForChanges(mAllFlags.values(), mOnPropertiesChanged); } @Override @@ -196,7 +210,7 @@ public class FeatureFlagsDebug implements FeatureFlags { return mStringFlagCache.get(id); } - /** Specific override for Boolean flags that checks against the teamfood list.*/ + /** Specific override for Boolean flags that checks against the teamfood list. */ private boolean readFlagValue(int id, boolean defaultValue) { Boolean result = readBooleanFlagOverride(id); boolean hasServerOverride = mServerFlagReader.hasOverride(id); @@ -273,6 +287,7 @@ public class FeatureFlagsDebug implements FeatureFlags { private void dispatchListenersAndMaybeRestart(int id, Consumer<Boolean> restartAction) { mFlagManager.dispatchListenersAndMaybeRestart(id, restartAction); } + /** Works just like {@link #eraseFlag(int)} except that it doesn't restart SystemUI. */ private void eraseInternal(int id) { // We can't actually "erase" things from sysprops, but we can set them to empty! @@ -358,7 +373,7 @@ public class FeatureFlagsDebug implements FeatureFlags { } } - Bundle extras = getResultExtras(true); + Bundle extras = getResultExtras(true); if (extras != null) { extras.putParcelableArrayList(EXTRA_FLAGS, pFlags); } diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt index 560dcbd78c42..62713348c789 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt @@ -31,7 +31,7 @@ constructor( dumpManager: DumpManager, private val commandRegistry: CommandRegistry, private val flagCommand: FlagCommand, - featureFlags: FeatureFlags + private val featureFlags: FeatureFlagsDebug ) : CoreStartable { init { @@ -41,6 +41,7 @@ constructor( } override fun start() { + featureFlags.init() commandRegistry.registerCommand(FlagCommand.FLAG_COMMAND) { flagCommand } } } diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java index 40a8a1a9ef01..30cad5faa2de 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java @@ -16,6 +16,8 @@ package com.android.systemui.flags; +import static com.android.systemui.flags.FlagsCommonModule.ALL_FLAGS; + import static java.util.Objects.requireNonNull; import android.content.res.Resources; @@ -34,6 +36,7 @@ import java.io.PrintWriter; import java.util.Map; import javax.inject.Inject; +import javax.inject.Named; /** * Default implementation of the a Flag manager that returns default values for release builds @@ -49,26 +52,47 @@ public class FeatureFlagsRelease implements FeatureFlags { private final SystemPropertiesHelper mSystemProperties; private final DeviceConfigProxy mDeviceConfigProxy; private final ServerFlagReader mServerFlagReader; + private final Restarter mRestarter; + private final Map<Integer, Flag<?>> mAllFlags; SparseBooleanArray mBooleanCache = new SparseBooleanArray(); SparseArray<String> mStringCache = new SparseArray<>(); + private final ServerFlagReader.ChangeListener mOnPropertiesChanged = + new ServerFlagReader.ChangeListener() { + @Override + public void onChange() { + mRestarter.restart(); + } + }; + @Inject public FeatureFlagsRelease( @Main Resources resources, SystemPropertiesHelper systemProperties, DeviceConfigProxy deviceConfigProxy, - ServerFlagReader serverFlagReader) { + ServerFlagReader serverFlagReader, + @Named(ALL_FLAGS) Map<Integer, Flag<?>> allFlags, + Restarter restarter) { mResources = resources; mSystemProperties = systemProperties; mDeviceConfigProxy = deviceConfigProxy; mServerFlagReader = serverFlagReader; + mAllFlags = allFlags; + mRestarter = restarter; + } + + /** Call after construction to setup listeners. */ + void init() { + mServerFlagReader.listenForChanges(mAllFlags.values(), mOnPropertiesChanged); } @Override - public void addListener(@NonNull Flag<?> flag, @NonNull Listener listener) {} + public void addListener(@NonNull Flag<?> flag, @NonNull Listener listener) { + } @Override - public void removeListener(@NonNull Listener listener) {} + public void removeListener(@NonNull Listener listener) { + } @Override public boolean isEnabled(@NotNull UnreleasedFlag flag) { diff --git a/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java b/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java index 4d254313a57b..1e93c0b7002d 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java @@ -16,6 +16,8 @@ package com.android.systemui.flags; +import static com.android.systemui.flags.FlagsCommonModule.ALL_FLAGS; + import androidx.annotation.NonNull; import com.android.systemui.statusbar.commandline.Command; @@ -42,7 +44,7 @@ public class FlagCommand implements Command { @Inject FlagCommand( FeatureFlagsDebug featureFlags, - @Named(FeatureFlagsDebug.ALL_FLAGS) Map<Integer, Flag<?>> allFlags + @Named(ALL_FLAGS) Map<Integer, Flag<?>> allFlags ) { mFeatureFlags = featureFlags; mAllFlags = allFlags; diff --git a/packages/SystemUI/src/com/android/systemui/flags/FlagsCommonModule.kt b/packages/SystemUI/src/com/android/systemui/flags/FlagsCommonModule.kt new file mode 100644 index 000000000000..e1f4944741a0 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/flags/FlagsCommonModule.kt @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2022 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 com.android.internal.statusbar.IStatusBarService +import dagger.Module +import dagger.Provides +import javax.inject.Named + +/** Module containing shared code for all FeatureFlag implementations. */ +@Module +interface FlagsCommonModule { + companion object { + const val ALL_FLAGS = "all_flags" + + @JvmStatic + @Provides + @Named(ALL_FLAGS) + fun providesAllFlags(): Map<Int, Flag<*>> { + return Flags.collectFlags() + } + + @JvmStatic + @Provides + fun providesRestarter(barService: IStatusBarService): Restarter { + return object : Restarter { + override fun restart() { + barService.restart() + } + } + } + } +} diff --git a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt index fc5b9f4eea05..694fa01bb4a5 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt @@ -16,9 +16,13 @@ package com.android.systemui.flags +import android.provider.DeviceConfig +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Background import com.android.systemui.util.DeviceConfigProxy -import dagger.Binds import dagger.Module +import dagger.Provides +import java.util.concurrent.Executor import javax.inject.Inject interface ServerFlagReader { @@ -27,40 +31,99 @@ interface ServerFlagReader { /** Returns any stored server-side setting or the default if not set. */ fun readServerOverride(flagId: Int, default: Boolean): Boolean + + /** Register a listener for changes to any of the passed in flags. */ + fun listenForChanges(values: Collection<Flag<*>>, listener: ChangeListener) + + interface ChangeListener { + fun onChange() + } } class ServerFlagReaderImpl @Inject constructor( - private val deviceConfig: DeviceConfigProxy + private val namespace: String, + private val deviceConfig: DeviceConfigProxy, + @Background private val executor: Executor ) : ServerFlagReader { + + private val listeners = + mutableListOf<Pair<ServerFlagReader.ChangeListener, Collection<Flag<*>>>>() + + private val onPropertiesChangedListener = object : DeviceConfig.OnPropertiesChangedListener { + override fun onPropertiesChanged(properties: DeviceConfig.Properties) { + if (properties.namespace != namespace) { + return + } + + for ((listener, flags) in listeners) { + propLoop@ for (propName in properties.keyset) { + for (flag in flags) { + if (propName == getServerOverrideName(flag.id)) { + listener.onChange() + break@propLoop + } + } + } + } + } + } + override fun hasOverride(flagId: Int): Boolean = deviceConfig.getProperty( - SYSUI_NAMESPACE, + namespace, getServerOverrideName(flagId) ) != null override fun readServerOverride(flagId: Int, default: Boolean): Boolean { return deviceConfig.getBoolean( - SYSUI_NAMESPACE, + namespace, getServerOverrideName(flagId), default ) } + override fun listenForChanges( + flags: Collection<Flag<*>>, + listener: ServerFlagReader.ChangeListener + ) { + if (listeners.isEmpty()) { + deviceConfig.addOnPropertiesChangedListener( + namespace, + executor, + onPropertiesChangedListener + ) + } + listeners.add(Pair(listener, flags)) + } + private fun getServerOverrideName(flagId: Int): String { return "flag_override_$flagId" } } -private val SYSUI_NAMESPACE = "systemui" - @Module interface ServerFlagReaderModule { - @Binds - fun bindsReader(impl: ServerFlagReaderImpl): ServerFlagReader + companion object { + private val SYSUI_NAMESPACE = "systemui" + + @JvmStatic + @Provides + @SysUISingleton + fun bindsReader( + deviceConfig: DeviceConfigProxy, + @Background executor: Executor + ): ServerFlagReader { + return ServerFlagReaderImpl( + SYSUI_NAMESPACE, deviceConfig, executor + ) + } + } } class ServerFlagReaderFake : ServerFlagReader { private val flagMap: MutableMap<Int, Boolean> = mutableMapOf() + private val listeners = + mutableListOf<Pair<ServerFlagReader.ChangeListener, Collection<Flag<*>>>>() override fun hasOverride(flagId: Int): Boolean { return flagMap.containsKey(flagId) @@ -72,9 +135,24 @@ class ServerFlagReaderFake : ServerFlagReader { fun setFlagValue(flagId: Int, value: Boolean) { flagMap.put(flagId, value) + + for ((listener, flags) in listeners) { + flagLoop@ for (flag in flags) { + if (flagId == flag.id) { + listener.onChange() + break@flagLoop + } + } + } } fun eraseFlag(flagId: Int) { flagMap.remove(flagId) } + + override fun listenForChanges( + flags: Collection<Flag<*>>, + listener: ServerFlagReader.ChangeListener + ) { + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt index 20a82c63cfdd..4b3b70e3ae77 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt @@ -88,6 +88,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { flagMap, restarter ) + mFeatureFlagsDebug.init() verify(flagManager).onSettingsChangedAction = any() broadcastReceiver = withArgCaptor { verify(mockContext).registerReceiver(capture(), any(), nullable(), nullable(), @@ -255,11 +256,11 @@ class FeatureFlagsDebugTest : SysuiTestCase() { broadcastReceiver.onReceive(mockContext, Intent()) broadcastReceiver.onReceive(mockContext, Intent("invalid action")) broadcastReceiver.onReceive(mockContext, Intent(FlagManager.ACTION_SET_FLAG)) - setByBroadcast(0, false) // unknown id does nothing - setByBroadcast(1, "string") // wrong type does nothing - setByBroadcast(2, 123) // wrong type does nothing - setByBroadcast(3, false) // wrong type does nothing - setByBroadcast(4, 123) // wrong type does nothing + setByBroadcast(0, false) // unknown id does nothing + setByBroadcast(1, "string") // wrong type does nothing + setByBroadcast(2, 123) // wrong type does nothing + setByBroadcast(3, false) // wrong type does nothing + setByBroadcast(4, 123) // wrong type does nothing verifyNoMoreInteractions(flagManager, secureSettings) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt index 575c14262b74..b2dd60c9566d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt @@ -38,8 +38,9 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { @Mock private lateinit var mResources: Resources @Mock private lateinit var mSystemProperties: SystemPropertiesHelper + @Mock private lateinit var restarter: Restarter + private val flagMap = mutableMapOf<Int, Flag<*>>() private val serverFlagReader = ServerFlagReaderFake() - private val deviceConfig = DeviceConfigProxyFake() @Before @@ -49,7 +50,9 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { mResources, mSystemProperties, deviceConfig, - serverFlagReader) + serverFlagReader, + flagMap, + restarter) } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt new file mode 100644 index 000000000000..6f5f460d41c4 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2022 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.test.suitebuilder.annotation.SmallTest +import android.testing.AndroidTestingRunner +import com.android.systemui.SysuiTestCase +import com.android.systemui.util.DeviceConfigProxyFake +import com.android.systemui.util.concurrency.FakeExecutor +import com.android.systemui.util.time.FakeSystemClock +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.verify +import org.mockito.MockitoAnnotations + +@SmallTest +@RunWith(AndroidTestingRunner::class) +class ServerFlagReaderImplTest : SysuiTestCase() { + + private val NAMESPACE = "test" + + @Mock private lateinit var changeListener: ServerFlagReader.ChangeListener + + private lateinit var serverFlagReader: ServerFlagReaderImpl + private val deviceConfig = DeviceConfigProxyFake() + private val executor = FakeExecutor(FakeSystemClock()) + + @Before + fun setup() { + MockitoAnnotations.initMocks(this) + + serverFlagReader = ServerFlagReaderImpl(NAMESPACE, deviceConfig, executor) + } + + @Test + fun testChange_alertsListener() { + val flag = ReleasedFlag(1) + serverFlagReader.listenForChanges(listOf(flag), changeListener) + + deviceConfig.setProperty(NAMESPACE, "flag_override_1", "1", false) + executor.runAllReady() + + verify(changeListener).onChange() + } +} |