Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6416c279 authored by Alejandro Nijamkin's avatar Alejandro Nijamkin
Browse files

[flexiglass] Make sure scene and userActions are in-sync 

Makes sure that the userActions map used for each scene matches that
scene exactly. Before this CL, there was a chance that the current scene
inside STL and the current scene in the scene stack were not the same.
This would cause occasional brief times when the userActions being sent
for the current scene was actually the one for the previous scene; which
caused a crash, by-design, inside of STL when that scene's builder was
called if one of the user actions went to the real current scene.

To make them match, the changes actually go to the real scene object for
each scene key instead of relying on a flow in the scene stack of
classes.

Note that, as a side effect, there's a lot more flow collection now as,
before this change, SceneContainer tried to only collect from the
userActions/sceneDestinations flow of the current scene. This CL changes
that to always collect from the flows of all scenes - which is
unfortunate but not clear that it creates performance issues.

Perhaps if we change the userActions parameter of the scene builder in
STL to allow for lazy / provider instead of a direct read it will reduce
performance impact.

Fix: 349430360
Test: manually verified that the original crash from the bug doesn't
happen anymore
Test: manually smoked-tested many scene transitions on locked and
unlocked device (shade and QS over lockscreen, bouncer, unlock, shade
and QS over gone, re-lock, shade and QS) also display on and off
Flag: com.android.systemui.scene_container

Change-Id: I1348b3aa22090c435bb246fd5e82517a0c32cb7c
parent 72cd9a53

packages/SystemUI/compose/features/src/com/android/systemui/scene/ui/composable/SceneContainer.kt

+10 −12
Original line number Diff line number Diff line
@@ -34,6 +34,8 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle 
import com.android.compose.animation.scene.MutableSceneTransitionLayoutState

import com.android.compose.animation.scene.SceneKey

import com.android.compose.animation.scene.SceneTransitionLayout

import com.android.compose.animation.scene.UserAction

import com.android.compose.animation.scene.UserActionResult

import com.android.compose.animation.scene.observableTransitionState

import com.android.systemui.ribbon.ui.composable.BottomRightCornerRibbon

import com.android.systemui.scene.shared.model.SceneDataSourceDelegator
@@ -56,7 +58,6 @@ import com.android.systemui.scene.ui.viewmodel.SceneContainerViewModel 
 *   must have entries in this map.

 * @param modifier A modifier.

 */

@OptIn(ExperimentalComposeUiApi::class)

@Composable

fun SceneContainer(

    viewModel: SceneContainerViewModel,
@@ -66,8 +67,6 @@ fun SceneContainer( 
) {

    val coroutineScope = rememberCoroutineScope()

    val currentSceneKey: SceneKey by viewModel.currentScene.collectAsStateWithLifecycle()

    val currentDestinations by

        viewModel.currentDestinationScenes(coroutineScope).collectAsStateWithLifecycle()

    val state: MutableSceneTransitionLayoutState = remember {

        MutableSceneTransitionLayoutState(

            initialScene = currentSceneKey,
@@ -88,20 +87,19 @@ fun SceneContainer( 
        onDispose { viewModel.setTransitionState(null) }

    }



    val userActionsBySceneKey: Map<SceneKey, Map<UserAction, UserActionResult>> =

        sceneByKey.values.associate { scene ->

            val userActions by scene.destinationScenes.collectAsStateWithLifecycle(emptyMap())

            val resolvedUserActions = viewModel.resolveSceneFamilies(userActions)

            scene.key to resolvedUserActions

        }



    Box(

        modifier = Modifier.fillMaxSize(),

    ) {

        SceneTransitionLayout(state = state, modifier = modifier.fillMaxSize()) {

            sceneByKey.forEach { (sceneKey, composableScene) ->

                scene(

                    key = sceneKey,

                    userActions =

                        if (sceneKey == currentSceneKey) {

                            currentDestinations

                        } else {

                            viewModel.resolveSceneFamilies(composableScene.destinationScenes.value)

                        },

                ) {

                scene(key = sceneKey, userActions = checkNotNull(userActionsBySceneKey[sceneKey])) {

                    with(composableScene) {

                        this@scene.Content(

                            modifier = Modifier.element(sceneKey.rootElementKey).fillMaxSize(),

packages/SystemUI/multivalentTests/src/com/android/systemui/scene/SceneFrameworkIntegrationTest.kt

+0 −1
Original line number Diff line number Diff line
@@ -133,7 +133,6 @@ class SceneFrameworkIntegrationTest : SysuiTestCase() { 
                sceneInteractor = sceneInteractor,

                falsingInteractor = kosmos.falsingInteractor,

                powerInteractor = kosmos.powerInteractor,

                scenes = kosmos.scenes,

            )

            .apply { setTransitionState(transitionState) }

    }

packages/SystemUI/multivalentTests/src/com/android/systemui/scene/ui/viewmodel/SceneContainerViewModelTest.kt

+0 −22
Original line number Diff line number Diff line
@@ -28,10 +28,8 @@ import com.android.systemui.kosmos.testScope 
import com.android.systemui.power.data.repository.fakePowerRepository

import com.android.systemui.power.domain.interactor.powerInteractor

import com.android.systemui.scene.domain.interactor.sceneInteractor

import com.android.systemui.scene.fakeScenes

import com.android.systemui.scene.sceneContainerConfig

import com.android.systemui.scene.sceneKeys

import com.android.systemui.scene.scenes

import com.android.systemui.scene.shared.model.Scenes

import com.android.systemui.scene.shared.model.fakeSceneDataSource

import com.android.systemui.testKosmos
@@ -66,7 +64,6 @@ class SceneContainerViewModelTest : SysuiTestCase() { 
                sceneInteractor = sceneInteractor,

                falsingInteractor = kosmos.falsingInteractor,

                powerInteractor = kosmos.powerInteractor,

                scenes = kosmos.scenes,

            )

    }
@@ -217,23 +214,4 @@ class SceneContainerViewModelTest : SysuiTestCase() { 


            assertThat(isVisible).isFalse()

        }



    @Test

    fun currentDestinationScenes_onlyTheCurrentSceneIsCollected() =

        testScope.runTest {

            val unused by collectLastValue(underTest.currentDestinationScenes(backgroundScope))

            val currentScene by collectLastValue(sceneInteractor.currentScene)

            kosmos.fakeScenes.forEach { scene ->

                fakeSceneDataSource.changeScene(toScene = scene.key)

                runCurrent()

                assertThat(currentScene).isEqualTo(scene.key)



                assertThat(scene.isDestinationScenesBeingCollected).isTrue()

                kosmos.fakeScenes

                    .filter { it.key != scene.key }

                    .forEach { otherScene ->

                        assertThat(otherScene.isDestinationScenesBeingCollected).isFalse()

                    }

            }

        }

}

packages/SystemUI/src/com/android/systemui/scene/ui/viewmodel/SceneContainerViewModel.kt

+0 −25
Original line number Diff line number Diff line
@@ -26,17 +26,12 @@ import com.android.systemui.classifier.domain.interactor.FalsingInteractor 
import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.power.domain.interactor.PowerInteractor

import com.android.systemui.scene.domain.interactor.SceneInteractor

import com.android.systemui.scene.shared.model.Scene

import com.android.systemui.scene.shared.model.Scenes

import com.android.systemui.utils.coroutines.flow.flatMapLatestConflated

import javax.inject.Inject

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.SharingStarted

import kotlinx.coroutines.flow.StateFlow

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.map

import kotlinx.coroutines.flow.stateIn



/** Models UI state for the scene container. */

@SysUISingleton
@@ -46,7 +41,6 @@ constructor( 
    private val sceneInteractor: SceneInteractor,

    private val falsingInteractor: FalsingInteractor,

    private val powerInteractor: PowerInteractor,

    scenes: Set<@JvmSuppressWildcards Scene>,

) {

    /**

     * Keys of all scenes in the container.
@@ -62,25 +56,6 @@ constructor( 
    /** Whether the container is visible. */

    val isVisible: StateFlow<Boolean> = sceneInteractor.isVisible



    private val destinationScenesBySceneKey =

        scenes.associate { scene ->

            scene.key to scene.destinationScenes.flatMapLatestConflated { replaceSceneFamilies(it) }

        }



    fun currentDestinationScenes(

        scope: CoroutineScope,

    ): StateFlow<Map<UserAction, UserActionResult>> {

        return currentScene

            .flatMapLatestConflated { currentSceneKey ->

                checkNotNull(destinationScenesBySceneKey[currentSceneKey])

            }

            .stateIn(

                scope = scope,

                started = SharingStarted.WhileSubscribed(),

                initialValue = emptyMap(),

            )

    }



    /**

     * Binds the given flow so the system remembers it.

     *