From 0407508cfa73a8b5c48e55a105d264109433a592 Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Mon, 10 Jul 2023 13:05:51 -0700 Subject: [PATCH] [flexiglass] Drive window view visibility. When the internal state of our scene system is updated, the visibility of the window view that contains our scene graph is driven by the new startable. Bug: 290656769 Test: added unit tests Test: manually verified that, when the feature flag is enabled, the visibility calls coming from legacy code into WindowRootView#setVisibility are properly ignored and the visibility of the root view is driven by the new system instead. Test: manually verified no harm done. When the feature flag is disabled, the visibility of the window root view is still updated by the legacy code. Change-Id: I20dccfdc7089dda0732b56c384d139e79a4c2f96 --- .../scene/SceneContainerFrameworkModule.kt | 2 + .../domain/interactor/SceneInteractor.kt | 12 ++- .../SceneContainerStartableModule.kt | 32 ++++++ .../SystemUiDefaultSceneContainerStartable.kt | 69 +++++++++++++ .../scene/ui/view/SceneWindowRootView.kt | 61 +++--------- .../ui/view/SceneWindowRootViewBinder.kt | 92 +++++++++++++++++ ...temUiDefaultSceneContainerStartableTest.kt | 98 +++++++++++++++++++ 7 files changed, 317 insertions(+), 49 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartableModule.kt create mode 100644 packages/SystemUI/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartable.kt create mode 100644 packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootViewBinder.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartableTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/scene/SceneContainerFrameworkModule.kt b/packages/SystemUI/src/com/android/systemui/scene/SceneContainerFrameworkModule.kt index 0a9839e2f18b6..26c52199f4934 100644 --- a/packages/SystemUI/src/com/android/systemui/scene/SceneContainerFrameworkModule.kt +++ b/packages/SystemUI/src/com/android/systemui/scene/SceneContainerFrameworkModule.kt @@ -16,6 +16,7 @@ package com.android.systemui.scene +import com.android.systemui.scene.domain.startable.SceneContainerStartableModule import com.android.systemui.scene.shared.model.SceneContainerConfigModule import com.android.systemui.scene.ui.composable.SceneModule import com.android.systemui.scene.ui.viewmodel.SceneContainerViewModelModule @@ -25,6 +26,7 @@ import dagger.Module includes = [ SceneContainerConfigModule::class, + SceneContainerStartableModule::class, SceneContainerViewModelModule::class, SceneModule::class, ], diff --git a/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt b/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt index c704ea838130e..4582370679aba 100644 --- a/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt @@ -24,7 +24,17 @@ import com.android.systemui.scene.shared.model.SceneTransitionModel import javax.inject.Inject import kotlinx.coroutines.flow.StateFlow -/** Business logic and app state accessors for the scene framework. */ +/** + * Generic business logic and app state accessors for the scene framework. + * + * Note that scene container specific business logic does not belong in this class. Instead, it + * should be hoisted to a class that is specific to that scene container, for an example, please see + * [SystemUiDefaultSceneContainerStartable]. + * + * Also note that this class should not depend on state or logic of other modules or features. + * Instead, other feature modules should depend on and call into this class when their parts of the + * application state change. + */ @SysUISingleton class SceneInteractor @Inject diff --git a/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartableModule.kt b/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartableModule.kt new file mode 100644 index 0000000000000..b3de2d158a538 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartableModule.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2023 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.scene.domain.startable + +import com.android.systemui.CoreStartable +import dagger.Binds +import dagger.Module +import dagger.multibindings.ClassKey +import dagger.multibindings.IntoMap + +@Module +interface SceneContainerStartableModule { + + @Binds + @IntoMap + @ClassKey(SystemUiDefaultSceneContainerStartable::class) + fun bind(impl: SystemUiDefaultSceneContainerStartable): CoreStartable +} diff --git a/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartable.kt b/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartable.kt new file mode 100644 index 0000000000000..91400196bbb16 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartable.kt @@ -0,0 +1,69 @@ +/* + * Copyright 2023 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.scene.domain.startable + +import com.android.systemui.CoreStartable +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Application +import com.android.systemui.flags.FeatureFlags +import com.android.systemui.flags.Flags +import com.android.systemui.scene.domain.interactor.SceneInteractor +import com.android.systemui.scene.shared.model.SceneContainerNames +import com.android.systemui.scene.shared.model.SceneKey +import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch + +/** + * Hooks up business logic that manipulates the state of the [SceneInteractor] for the default + * system UI scene container (the one named [SceneContainerNames.SYSTEM_UI_DEFAULT]) based on state + * from other systems. + */ +@SysUISingleton +class SystemUiDefaultSceneContainerStartable +@Inject +constructor( + @Application private val applicationScope: CoroutineScope, + private val sceneInteractor: SceneInteractor, + private val featureFlags: FeatureFlags, +) : CoreStartable { + + override fun start() { + if (featureFlags.isEnabled(Flags.SCENE_CONTAINER)) { + keepVisibilityUpdated() + } + } + + /** Drives visibility of the scene container. */ + private fun keepVisibilityUpdated() { + applicationScope.launch { + sceneInteractor + .currentScene(CONTAINER_NAME) + .map { it.key } + .distinctUntilChanged() + .collect { sceneKey -> + sceneInteractor.setVisible(CONTAINER_NAME, sceneKey != SceneKey.Gone) + } + } + } + + companion object { + private const val CONTAINER_NAME = SceneContainerNames.SYSTEM_UI_DEFAULT + } +} diff --git a/packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootView.kt b/packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootView.kt index 2ad5429668d0e..c456be6e5ab2a 100644 --- a/packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootView.kt +++ b/packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootView.kt @@ -2,19 +2,10 @@ package com.android.systemui.scene.ui.view import android.content.Context import android.util.AttributeSet -import androidx.activity.OnBackPressedDispatcher -import androidx.activity.OnBackPressedDispatcherOwner -import androidx.activity.setViewTreeOnBackPressedDispatcherOwner -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle -import com.android.systemui.compose.ComposeFacade -import com.android.systemui.lifecycle.repeatWhenAttached +import android.view.View import com.android.systemui.scene.shared.model.Scene import com.android.systemui.scene.shared.model.SceneContainerConfig -import com.android.systemui.scene.shared.model.SceneKey import com.android.systemui.scene.ui.viewmodel.SceneContainerViewModel -import kotlinx.coroutines.launch /** A root view of the main SysUI window that supports scenes. */ class SceneWindowRootView( @@ -30,45 +21,19 @@ class SceneWindowRootView( containerConfig: SceneContainerConfig, scenes: Set, ) { - val unsortedSceneByKey: Map = scenes.associateBy { scene -> scene.key } - val sortedSceneByKey: Map = buildMap { - containerConfig.sceneKeys.forEach { sceneKey -> - val scene = - checkNotNull(unsortedSceneByKey[sceneKey]) { - "Scene not found for key \"$sceneKey\"!" - } - - put(sceneKey, scene) + SceneWindowRootViewBinder.bind( + view = this@SceneWindowRootView, + viewModel = viewModel, + containerConfig = containerConfig, + scenes = scenes, + onVisibilityChangedInternal = { isVisible -> + super.setVisibility(if (isVisible) View.VISIBLE else View.INVISIBLE) } - } + ) + } - repeatWhenAttached { - lifecycleScope.launch { - repeatOnLifecycle(Lifecycle.State.CREATED) { - setViewTreeOnBackPressedDispatcherOwner( - object : OnBackPressedDispatcherOwner { - override val onBackPressedDispatcher = - OnBackPressedDispatcher().apply { - setOnBackInvokedDispatcher(viewRootImpl.onBackInvokedDispatcher) - } - - override val lifecycle: Lifecycle = - this@repeatWhenAttached.lifecycle - } - ) - - addView( - ComposeFacade.createSceneContainerView( - context = context, - viewModel = viewModel, - sceneByKey = sortedSceneByKey, - ) - ) - } - - // Here when destroyed. - removeAllViews() - } - } + override fun setVisibility(visibility: Int) { + // Do nothing. We don't want external callers to invoke this. Instead, we drive our own + // visibility from our view-binder. } } diff --git a/packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootViewBinder.kt b/packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootViewBinder.kt new file mode 100644 index 0000000000000..5aa5feeedcf12 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/scene/ui/view/SceneWindowRootViewBinder.kt @@ -0,0 +1,92 @@ +/* + * Copyright 2023 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.scene.ui.view + +import android.view.ViewGroup +import androidx.activity.OnBackPressedDispatcher +import androidx.activity.OnBackPressedDispatcherOwner +import androidx.activity.setViewTreeOnBackPressedDispatcherOwner +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle +import com.android.systemui.compose.ComposeFacade +import com.android.systemui.lifecycle.repeatWhenAttached +import com.android.systemui.scene.shared.model.Scene +import com.android.systemui.scene.shared.model.SceneContainerConfig +import com.android.systemui.scene.shared.model.SceneKey +import com.android.systemui.scene.ui.viewmodel.SceneContainerViewModel +import kotlinx.coroutines.launch + +object SceneWindowRootViewBinder { + + /** Binds between the view and view-model pertaining to a specific scene container. */ + fun bind( + view: ViewGroup, + viewModel: SceneContainerViewModel, + containerConfig: SceneContainerConfig, + scenes: Set, + onVisibilityChangedInternal: (isVisible: Boolean) -> Unit, + ) { + val unsortedSceneByKey: Map = scenes.associateBy { scene -> scene.key } + val sortedSceneByKey: Map = buildMap { + containerConfig.sceneKeys.forEach { sceneKey -> + val scene = + checkNotNull(unsortedSceneByKey[sceneKey]) { + "Scene not found for key \"$sceneKey\"!" + } + + put(sceneKey, scene) + } + } + + view.repeatWhenAttached { + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.CREATED) { + view.setViewTreeOnBackPressedDispatcherOwner( + object : OnBackPressedDispatcherOwner { + override val onBackPressedDispatcher = + OnBackPressedDispatcher().apply { + setOnBackInvokedDispatcher( + view.viewRootImpl.onBackInvokedDispatcher + ) + } + + override val lifecycle: Lifecycle = this@repeatWhenAttached.lifecycle + } + ) + + view.addView( + ComposeFacade.createSceneContainerView( + context = view.context, + viewModel = viewModel, + sceneByKey = sortedSceneByKey, + ) + ) + + launch { + viewModel.isVisible.collect { isVisible -> + onVisibilityChangedInternal(isVisible) + } + } + } + + // Here when destroyed. + view.removeAllViews() + } + } + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartableTest.kt b/packages/SystemUI/tests/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartableTest.kt new file mode 100644 index 0000000000000..df3701edc6ded --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/scene/domain/startable/SystemUiDefaultSceneContainerStartableTest.kt @@ -0,0 +1,98 @@ +/* + * Copyright 2023 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. + */ + +@file:OptIn(ExperimentalCoroutinesApi::class, ExperimentalCoroutinesApi::class) + +package com.android.systemui.scene.domain.startable + +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.coroutines.collectLastValue +import com.android.systemui.flags.FakeFeatureFlags +import com.android.systemui.flags.Flags +import com.android.systemui.scene.SceneTestUtils +import com.android.systemui.scene.shared.model.SceneContainerNames +import com.android.systemui.scene.shared.model.SceneKey +import com.android.systemui.scene.shared.model.SceneModel +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@SmallTest +@RunWith(JUnit4::class) +class SystemUiDefaultSceneContainerStartableTest : SysuiTestCase() { + + private val utils = SceneTestUtils(this) + private val testScope = utils.testScope + private val sceneInteractor = utils.sceneInteractor() + private val featureFlags = FakeFeatureFlags() + + private val underTest = + SystemUiDefaultSceneContainerStartable( + applicationScope = testScope.backgroundScope, + sceneInteractor = sceneInteractor, + featureFlags = featureFlags, + ) + + @Test + fun start_featureEnabled_keepsVisibilityUpdated() = + testScope.runTest { + featureFlags.set(Flags.SCENE_CONTAINER, true) + val isVisible by + collectLastValue(sceneInteractor.isVisible(SceneContainerNames.SYSTEM_UI_DEFAULT)) + assertThat(isVisible).isTrue() + + underTest.start() + + sceneInteractor.setCurrentScene( + SceneContainerNames.SYSTEM_UI_DEFAULT, + SceneModel(SceneKey.Gone) + ) + assertThat(isVisible).isFalse() + + sceneInteractor.setCurrentScene( + SceneContainerNames.SYSTEM_UI_DEFAULT, + SceneModel(SceneKey.Shade) + ) + assertThat(isVisible).isTrue() + } + + @Test + fun start_featureDisabled_doesNotUpdateVisibility() = + testScope.runTest { + featureFlags.set(Flags.SCENE_CONTAINER, false) + val isVisible by + collectLastValue(sceneInteractor.isVisible(SceneContainerNames.SYSTEM_UI_DEFAULT)) + assertThat(isVisible).isTrue() + + underTest.start() + + sceneInteractor.setCurrentScene( + SceneContainerNames.SYSTEM_UI_DEFAULT, + SceneModel(SceneKey.Gone) + ) + assertThat(isVisible).isTrue() + + sceneInteractor.setCurrentScene( + SceneContainerNames.SYSTEM_UI_DEFAULT, + SceneModel(SceneKey.Shade) + ) + assertThat(isVisible).isTrue() + } +}