diff options
12 files changed, 71 insertions, 90 deletions
diff --git a/packages/SystemUI/aconfig/systemui.aconfig b/packages/SystemUI/aconfig/systemui.aconfig index 04c96ab5927f..aebdaaba3ce1 100644 --- a/packages/SystemUI/aconfig/systemui.aconfig +++ b/packages/SystemUI/aconfig/systemui.aconfig @@ -105,3 +105,10 @@ flag { "keyguard_root_view." bug: "278054201" } + +flag { + name: "qs_new_pipeline" + namespace: "systemui" + description: "Use the new pipeline for Quick Settings. Should have no behavior changes." + bug: "241772429" +} diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt index dd971b966e7f..b237b87987a4 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt @@ -345,13 +345,6 @@ object Flags { "qs_user_detail_shortcut" ) - @JvmField - val QS_PIPELINE_NEW_HOST = unreleasedFlag("qs_pipeline_new_host", teamfood = true) - - // TODO(b/278068252): Tracking Bug - @JvmField - val QS_PIPELINE_AUTO_ADD = unreleasedFlag("qs_pipeline_auto_add", teamfood = true) - // TODO(b/296357483): Tracking Bug @JvmField val QS_PIPELINE_NEW_TILES = unreleasedFlag("qs_pipeline_new_tiles") diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSHostAdapter.kt b/packages/SystemUI/src/com/android/systemui/qs/QSHostAdapter.kt index 65a2c8ca692b..c77233eb1737 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSHostAdapter.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/QSHostAdapter.kt @@ -35,10 +35,10 @@ import kotlinx.coroutines.launch /** * Adapter to determine what real class to use for classes that depend on [QSHost]. - * * When [QSPipelineFlagsRepository.pipelineHostEnabled] is false, all calls will be routed to + * * When [QSPipelineFlagsRepository.pipelineEnabled] is false, all calls will be routed to * [QSTileHost]. - * * When [QSPipelineFlagsRepository.pipelineHostEnabled] is true, calls regarding the current set - * of tiles will be routed to [CurrentTilesInteractor]. Other calls (like [createTileView]) will + * * When [QSPipelineFlagsRepository.pipelineEnabled] is true, calls regarding the current set of + * tiles will be routed to [CurrentTilesInteractor]. Other calls (like [createTileView]) will * still be routed to [QSTileHost]. * * This routing also includes dumps. @@ -60,7 +60,7 @@ constructor( private const val TAG = "QSTileHost" } - private val useNewHost = flags.pipelineHostEnabled + private val useNewHost = flags.pipelineEnabled @GuardedBy("callbacksMap") private val callbacksMap = mutableMapOf<QSHost.Callback, Job>() diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java index 10f95e0b7201..1fab58e18ad2 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java @@ -31,7 +31,6 @@ import androidx.annotation.Nullable; import com.android.internal.annotations.VisibleForTesting; import com.android.systemui.Dumpable; import com.android.systemui.ProtoDumpable; -import com.android.systemui.res.R; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dump.nano.SystemUIProtoDump; @@ -49,6 +48,7 @@ import com.android.systemui.qs.pipeline.data.repository.CustomTileAddedRepositor import com.android.systemui.qs.pipeline.domain.interactor.PanelInteractor; import com.android.systemui.qs.pipeline.shared.QSPipelineFlagsRepository; import com.android.systemui.qs.tiles.di.NewQSTileFactory; +import com.android.systemui.res.R; import com.android.systemui.settings.UserFileManager; import com.android.systemui.settings.UserTracker; import com.android.systemui.shade.ShadeController; @@ -57,8 +57,6 @@ import com.android.systemui.tuner.TunerService; import com.android.systemui.tuner.TunerService.Tunable; import com.android.systemui.util.settings.SecureSettings; -import dagger.Lazy; - import org.jetbrains.annotations.NotNull; import java.io.PrintWriter; @@ -75,6 +73,8 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Provider; +import dagger.Lazy; + /** Platform implementation of the quick settings tile host * * This class keeps track of the set of current tiles and is the in memory source of truth @@ -167,7 +167,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, P // finishes before creating any tiles. tunerService.addTunable(this, TILES_SETTING); // AutoTileManager can modify mTiles so make sure mTiles has already been initialized. - if (!mFeatureFlags.getPipelineAutoAddEnabled()) { + if (!mFeatureFlags.getPipelineEnabled()) { mAutoTiles = autoTiles.get(); } }); @@ -288,9 +288,10 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, P } } // Do not process tiles if the flag is enabled. - if (mFeatureFlags.getPipelineHostEnabled()) { + if (mFeatureFlags.getPipelineEnabled()) { return; } + QSPipelineFlagsRepository.Utils.assertInLegacyMode(); if (newValue == null && UserManager.isDeviceInDemoMode(mContext)) { newValue = mContext.getResources().getString(R.string.quick_settings_tiles_retail_mode); } diff --git a/packages/SystemUI/src/com/android/systemui/qs/dagger/QSHostModule.kt b/packages/SystemUI/src/com/android/systemui/qs/dagger/QSHostModule.kt index b2111d765a9d..496a6f830e8c 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/dagger/QSHostModule.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/dagger/QSHostModule.kt @@ -48,7 +48,7 @@ interface QSHostModule { qsHost: QSTileHost, panelInteractorImpl: PanelInteractorImpl ): PanelInteractor { - return if (featureFlags.pipelineHostEnabled) { + return if (featureFlags.pipelineEnabled) { panelInteractorImpl } else { qsHost @@ -62,7 +62,7 @@ interface QSHostModule { qsHost: QSTileHost, customTileAddedRepository: CustomTileAddedSharedPrefsRepository ): CustomTileAddedRepository { - return if (featureFlags.pipelineHostEnabled) { + return if (featureFlags.pipelineEnabled) { customTileAddedRepository } else { qsHost diff --git a/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractor.kt b/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractor.kt index 4bb8c6e4bb2d..4bda7307c667 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractor.kt @@ -20,7 +20,6 @@ import android.content.ComponentName import android.content.Context import android.content.Intent import android.os.UserHandle -import android.util.Log import com.android.systemui.Dumpable import com.android.systemui.ProtoDumpable import com.android.systemui.dagger.SysUISingleton @@ -174,7 +173,7 @@ constructor( } init { - if (featureFlags.pipelineHostEnabled) { + if (featureFlags.pipelineEnabled) { startTileCollection() } } diff --git a/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/startable/QSPipelineCoreStartable.kt b/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/startable/QSPipelineCoreStartable.kt index 1539f05508d0..2930acdeaaa3 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/startable/QSPipelineCoreStartable.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/startable/QSPipelineCoreStartable.kt @@ -35,7 +35,7 @@ constructor( ) : CoreStartable { override fun start() { - if (featureFlags.pipelineAutoAddEnabled) { + if (featureFlags.pipelineEnabled) { autoAddInteractor.init(currentTilesInteractor) restoreReconciliationInteractor.start() } diff --git a/packages/SystemUI/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepository.kt b/packages/SystemUI/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepository.kt index 1a71b715fe3a..5c7420cb3c1b 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepository.kt @@ -1,8 +1,10 @@ package com.android.systemui.qs.pipeline.shared +import com.android.systemui.Flags as AconfigFlags import com.android.systemui.dagger.SysUISingleton import com.android.systemui.flags.FeatureFlagsClassic import com.android.systemui.flags.Flags +import com.android.systemui.flags.RefactorFlagUtils import javax.inject.Inject /** Encapsulate the different QS pipeline flags and their dependencies */ @@ -12,16 +14,18 @@ class QSPipelineFlagsRepository constructor( private val featureFlags: FeatureFlagsClassic, ) { - - /** @see Flags.QS_PIPELINE_NEW_HOST */ - val pipelineHostEnabled: Boolean - get() = featureFlags.isEnabled(Flags.QS_PIPELINE_NEW_HOST) - - /** @see Flags.QS_PIPELINE_AUTO_ADD */ - val pipelineAutoAddEnabled: Boolean - get() = pipelineHostEnabled && featureFlags.isEnabled(Flags.QS_PIPELINE_AUTO_ADD) + val pipelineEnabled: Boolean + get() = AconfigFlags.qsNewPipeline() /** @see Flags.QS_PIPELINE_NEW_TILES */ val pipelineTilesEnabled: Boolean get() = featureFlags.isEnabled(Flags.QS_PIPELINE_NEW_TILES) + + companion object Utils { + fun assertInLegacyMode() = + RefactorFlagUtils.assertInLegacyMode( + AconfigFlags.qsNewPipeline(), + AconfigFlags.FLAG_QS_NEW_PIPELINE + ) + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java index 3877bb6660ba..201133288de5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java @@ -36,6 +36,7 @@ import com.android.systemui.qs.QSHost; import com.android.systemui.qs.ReduceBrightColorsController; import com.android.systemui.qs.UserSettingObserver; import com.android.systemui.qs.external.CustomTile; +import com.android.systemui.qs.pipeline.shared.QSPipelineFlagsRepository; import com.android.systemui.res.R; import com.android.systemui.statusbar.policy.CastController; import com.android.systemui.statusbar.policy.CastController.CastDevice; @@ -142,6 +143,7 @@ public class AutoTileManager implements UserAwareController { * Init method must be called after construction to start listening */ public void init() { + QSPipelineFlagsRepository.Utils.assertInLegacyMode(); if (mInitialized) { Log.w(TAG, "Trying to re-initialize"); return; diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java index 79411f427f1f..3b07913de7c5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java @@ -17,6 +17,8 @@ package com.android.systemui.qs; +import static com.android.systemui.Flags.FLAG_QS_NEW_PIPELINE; + import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; @@ -39,6 +41,7 @@ import android.database.ContentObserver; import android.os.Handler; import android.os.Looper; import android.os.UserHandle; +import android.platform.test.flag.junit.SetFlagsRule; import android.testing.AndroidTestingRunner; import android.util.SparseArray; import android.view.View; @@ -48,7 +51,6 @@ import androidx.test.filters.SmallTest; import com.android.internal.logging.MetricsLogger; import com.android.internal.util.CollectionUtils; -import com.android.systemui.res.R; import com.android.systemui.SysuiTestCase; import com.android.systemui.classifier.FalsingManagerFake; import com.android.systemui.dump.nano.SystemUIProtoDump; @@ -67,6 +69,7 @@ import com.android.systemui.qs.logging.QSLogger; import com.android.systemui.qs.pipeline.shared.QSPipelineFlagsRepository; import com.android.systemui.qs.tileimpl.QSTileImpl; import com.android.systemui.qs.tiles.di.NewQSTileFactory; +import com.android.systemui.res.R; import com.android.systemui.settings.UserFileManager; import com.android.systemui.settings.UserTracker; import com.android.systemui.shade.ShadeController; @@ -78,9 +81,8 @@ import com.android.systemui.util.settings.FakeSettings; import com.android.systemui.util.settings.SecureSettings; import com.android.systemui.util.time.FakeSystemClock; -import dagger.Lazy; - import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -94,6 +96,8 @@ import java.util.concurrent.Executor; import javax.inject.Provider; +import dagger.Lazy; + @RunWith(AndroidTestingRunner.class) @SmallTest public class QSTileHostTest extends SysuiTestCase { @@ -104,6 +108,9 @@ public class QSTileHostTest extends SysuiTestCase { private static final String CUSTOM_TILE_SPEC = CustomTile.toSpec(CUSTOM_TILE); private static final String SETTING = QSHost.TILES_SETTING; + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + @Mock private PluginManager mPluginManager; @Mock @@ -146,8 +153,7 @@ public class QSTileHostTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); mFeatureFlags = new FakeFeatureFlags(); - mFeatureFlags.set(Flags.QS_PIPELINE_NEW_HOST, false); - mFeatureFlags.set(Flags.QS_PIPELINE_AUTO_ADD, false); + mSetFlagsRule.disableFlags(FLAG_QS_NEW_PIPELINE); // TODO(b/299909337): Add test checking the new factory is used when the flag is on mFeatureFlags.set(Flags.QS_PIPELINE_NEW_TILES, false); mQSPipelineFlagsRepository = new QSPipelineFlagsRepository(mFeatureFlags); @@ -690,17 +696,6 @@ public class QSTileHostTest extends SysuiTestCase { assertEquals(CUSTOM_TILE.getClassName(), proto.tiles[1].getComponentName().className); } - @Test - public void testUserChange_flagOn_autoTileManagerNotified() { - mFeatureFlags.set(Flags.QS_PIPELINE_NEW_HOST, true); - int currentUser = mUserTracker.getUserId(); - clearInvocations(mAutoTiles); - when(mUserTracker.getUserId()).thenReturn(currentUser + 1); - - mQSTileHost.onTuningChanged(SETTING, "a,b"); - verify(mAutoTiles).changeUser(UserHandle.of(currentUser + 1)); - } - private SharedPreferences getSharedPreferencesForUser(int user) { return mUserFileManager.getSharedPreferences(QSTileHost.TILES, 0, user); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractorImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractorImplTest.kt index a89338a38e89..355ca816e789 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractorImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractorImplTest.kt @@ -21,9 +21,11 @@ import android.content.Context import android.content.Intent import android.content.pm.UserInfo import android.os.UserHandle +import android.platform.test.flag.junit.SetFlagsRule import android.service.quicksettings.Tile import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.SmallTest +import com.android.systemui.Flags.FLAG_QS_NEW_PIPELINE import com.android.systemui.SysuiTestCase import com.android.systemui.coroutines.collectLastValue import com.android.systemui.dump.nano.SystemUIProtoDump @@ -60,6 +62,7 @@ import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyString @@ -74,6 +77,8 @@ import org.mockito.MockitoAnnotations @OptIn(ExperimentalCoroutinesApi::class) class CurrentTilesInteractorImplTest : SysuiTestCase() { + @Rule @JvmField val setFlagsRule = SetFlagsRule() + private val tileSpecRepository: TileSpecRepository = FakeTileSpecRepository() private val userRepository = FakeUserRepository() private val installedTilesPackageRepository = FakeInstalledTilesComponentRepository() @@ -104,8 +109,7 @@ class CurrentTilesInteractorImplTest : SysuiTestCase() { fun setup() { MockitoAnnotations.initMocks(this) - featureFlags.set(Flags.QS_PIPELINE_NEW_HOST, true) - featureFlags.set(Flags.QS_PIPELINE_AUTO_ADD, true) + setFlagsRule.enableFlags(FLAG_QS_NEW_PIPELINE) // TODO(b/299909337): Add test checking the new factory is used when the flag is on featureFlags.set(Flags.QS_PIPELINE_NEW_TILES, true) diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepositoryTest.kt index 489221e86d0b..62ca965a81dd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/shared/QSPipelineFlagsRepositoryTest.kt @@ -1,61 +1,37 @@ package com.android.systemui.qs.pipeline.shared +import android.platform.test.flag.junit.SetFlagsRule +import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.SmallTest +import com.android.systemui.Flags import com.android.systemui.SysuiTestCase -import com.android.systemui.flags.FakeFeatureFlags -import com.android.systemui.flags.Flags +import com.android.systemui.flags.FakeFeatureFlagsClassic import com.google.common.truth.Truth.assertThat -import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.junit.runners.Parameterized -import org.junit.runners.Parameterized.Parameter -import org.junit.runners.Parameterized.Parameters @SmallTest -@RunWith(Parameterized::class) +@RunWith(AndroidJUnit4::class) class QSPipelineFlagsRepositoryTest : SysuiTestCase() { - companion object { - @Parameters( - name = - """ -WHEN: qs_pipeline_new_host = {0}, qs_pipeline_auto_add = {1} -THEN: pipelineNewHost = {2}, pipelineAutoAdd = {3} - """ - ) - @JvmStatic - fun data(): List<Array<Boolean>> = - (0 until 4).map { combination -> - val qs_pipeline_new_host = combination and 0b10 != 0 - val qs_pipeline_auto_add = combination and 0b01 != 0 - arrayOf( - qs_pipeline_new_host, - qs_pipeline_auto_add, - /* pipelineNewHost = */ qs_pipeline_new_host, - /* pipelineAutoAdd = */ qs_pipeline_new_host && qs_pipeline_auto_add, - ) - } - } - @JvmField @Parameter(0) var qsPipelineNewHostFlag: Boolean = false - @JvmField @Parameter(1) var qsPipelineAutoAddFlag: Boolean = false - @JvmField @Parameter(2) var pipelineNewHostExpected: Boolean = false - @JvmField @Parameter(3) var pipelineAutoAddExpected: Boolean = false + @Rule @JvmField val setFlagsRule = SetFlagsRule() + + private val fakeFeatureFlagsClassic = FakeFeatureFlagsClassic() - private val featureFlags = FakeFeatureFlags() - private val pipelineFlags = QSPipelineFlagsRepository(featureFlags) + private val underTest = QSPipelineFlagsRepository(fakeFeatureFlagsClassic) - @Before - fun setUp() { - featureFlags.apply { - set(Flags.QS_PIPELINE_NEW_HOST, qsPipelineNewHostFlag) - set(Flags.QS_PIPELINE_AUTO_ADD, qsPipelineAutoAddFlag) - } + @Test + fun pipelineFlagDisabled() { + setFlagsRule.disableFlags(Flags.FLAG_QS_NEW_PIPELINE) + + assertThat(underTest.pipelineEnabled).isFalse() } @Test - fun flagLogic() { - assertThat(pipelineFlags.pipelineHostEnabled).isEqualTo(pipelineNewHostExpected) - assertThat(pipelineFlags.pipelineAutoAddEnabled).isEqualTo(pipelineAutoAddExpected) + fun pipelineFlagEnabled() { + setFlagsRule.enableFlags(Flags.FLAG_QS_NEW_PIPELINE) + + assertThat(underTest.pipelineEnabled).isTrue() } } |