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

Commit c24a427c authored by Alejandro Nijamkin's avatar Alejandro Nijamkin
Browse files

[flexiglass] Don't emit from sharedFlowWithState when CANCELED

The attached bug occurs when the state originating from the CANCELED of
LockscreenToPrimaryBouncerTransitionViewModel is received downstream in
DeviceEntryIconViewModel _after_ the correct value from
PrimaryBouncerToLockscreenTransitionViewModel is received. This causes
the alpha of the lock indication view to remain at 0 rather than at 1.

See the long comment in the changed code for more details about why we
should filter out values that come from CANCELED, when Flexiglass is on.

Fix: 379845752
Test: manually verified that the original bug is fixed when Flexiglass
is on
Test: manually verified that swiping completely to Bouncer and returning
to the lockscreen shows the icon, Flexiglass on
Test: unlocked the the device, relocked, and repeated the steps above -
Flexiglass on
Test: manually verified all of the above also when Flexiglass is off
Flag: com.android.systemui.scene_container

Change-Id: I12be2d0a79ecbf89e6282753c8b8c965ac3839fd
parent b5664e8e
Loading
Loading
Loading
Loading
+7 −4
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import com.android.systemui.keyguard.shared.model.KeyguardState
import com.android.systemui.keyguard.shared.model.TransitionState
import com.android.systemui.keyguard.shared.model.TransitionStep
import com.android.systemui.kosmos.testScope
import com.android.systemui.scene.shared.flag.SceneContainerFlag
import com.android.systemui.shade.shadeTestUtil
import com.android.systemui.testKosmos
import com.google.common.truth.Truth.assertThat
@@ -171,8 +172,10 @@ class AodToLockscreenTransitionViewModelTest(flags: FlagsParameterization) : Sys
            // WHEN transition is cancelled
            repository.sendTransitionStep(step(.1f, TransitionState.CANCELED))

            // THEN alpha is immediately set to 1f (expected lockscreen alpha state)
            assertThat(deviceEntryBackgroundViewAlpha).isEqualTo(1f)
            // THEN alpha updates according to whether the scene framework is enabled (CANCELED is
            // ignored when the scene framework is enabled).
            assertThat(deviceEntryBackgroundViewAlpha)
                .isEqualTo(if (SceneContainerFlag.isEnabled) 0f else 1f)
        }

    @Test
@@ -195,14 +198,14 @@ class AodToLockscreenTransitionViewModelTest(flags: FlagsParameterization) : Sys

    private fun step(
        value: Float,
        state: TransitionState = TransitionState.RUNNING
        state: TransitionState = TransitionState.RUNNING,
    ): TransitionStep {
        return TransitionStep(
            from = KeyguardState.AOD,
            to = KeyguardState.LOCKSCREEN,
            value = value,
            transitionState = state,
            ownerName = "AodToLockscreenTransitionViewModelTest"
            ownerName = "AodToLockscreenTransitionViewModelTest",
        )
    }
}
+9 −5
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import com.android.systemui.keyguard.shared.model.TransitionState
import com.android.systemui.keyguard.shared.model.TransitionStep
import com.android.systemui.kosmos.testScope
import com.android.systemui.res.R
import com.android.systemui.scene.shared.flag.SceneContainerFlag
import com.android.systemui.shade.ShadeTestUtil
import com.android.systemui.shade.shadeTestUtil
import com.android.systemui.testKosmos
@@ -178,11 +179,13 @@ class LockscreenToOccludedTransitionViewModelTest(flags: FlagsParameterization)
                    ),
                testScope = testScope,
            )
            assertThat(values.size).isEqualTo(3)
            assertThat(values.size).isEqualTo(if (SceneContainerFlag.isEnabled) 2 else 3)
            values.forEach { assertThat(it).isIn(Range.closed(0f, 100f)) }

            // Cancel will reset the translation
            assertThat(values[2]).isEqualTo(0)
            // When the scene framework is not enabled, cancel will reset the translation
            if (!SceneContainerFlag.isEnabled) {
                assertThat(values.last()).isEqualTo(0f)
            }
        }

    @Test
@@ -242,8 +245,9 @@ class LockscreenToOccludedTransitionViewModelTest(flags: FlagsParameterization)
            // WHEN transition is canceled
            repository.sendTransitionStep(step(1f, TransitionState.CANCELED))

            // THEN alpha is immediately set to 0f
            assertThat(actual).isEqualTo(0f)
            // THEN alpha updates according to whether the scene framework is enabled (CANCELED is
            // ignored when the scene framework is enabled).
            assertThat(actual).isEqualTo(if (SceneContainerFlag.isEnabled) 1f else 0f)
        }

    private fun step(
+41 −26
Original line number Diff line number Diff line
@@ -36,7 +36,6 @@ import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull

/**
@@ -51,17 +50,11 @@ constructor(
    private val logger: KeyguardTransitionAnimationLogger,
) {
    /** Invoke once per transition between FROM->TO states to get access to a shared flow. */
    fun setup(
        duration: Duration,
        edge: Edge,
    ): FlowBuilder {
    fun setup(duration: Duration, edge: Edge): FlowBuilder {
        return FlowBuilder(duration, edge)
    }

    inner class FlowBuilder(
        private val transitionDuration: Duration,
        private val edge: Edge,
    ) {
    inner class FlowBuilder(private val transitionDuration: Duration, private val edge: Edge) {
        fun setupWithoutSceneContainer(edge: Edge.StateToState): FlowBuilder {
            if (SceneContainerFlag.isEnabled) return this
            return setup(this.transitionDuration, edge)
@@ -72,6 +65,8 @@ constructor(
         * in the range of [0, 1]. View animations should begin and end within a subset of this
         * range. This function maps the [startTime] and [duration] into [0, 1], when this subset is
         * valid.
         *
         * Note that [onCancel] isn't used when the scene framework is enabled.
         */
        fun sharedFlow(
            duration: Duration,
@@ -81,7 +76,7 @@ constructor(
            onCancel: (() -> Float)? = null,
            onFinish: (() -> Float)? = null,
            interpolator: Interpolator = LINEAR,
            name: String? = null
            name: String? = null,
        ): Flow<Float> {
            return sharedFlowWithState(
                    duration = duration,
@@ -113,7 +108,7 @@ constructor(
            onCancel: (() -> Float)? = null,
            onFinish: (() -> Float)? = null,
            interpolator: Interpolator = LINEAR,
            name: String? = null
            name: String? = null,
        ): Flow<StateToValue> {
            if (!duration.isPositive()) {
                throw IllegalArgumentException("duration must be a positive number: $duration")
@@ -155,7 +150,26 @@ constructor(

            return transitionInteractor
                .transition(edge)
                .map { step ->
                .mapNotNull { step ->
                    if (SceneContainerFlag.isEnabled && step.transitionState == CANCELED) {
                        // When the scene framework is enabled, there's no need to emit an alpha
                        // value when the keyguard transition animation is canceled because there's
                        // always going to be a new, reversed keyguard transition animation back to
                        // the original KeyguardState that starts right when this one was canceled.
                        //
                        // For example, if swiping up slightly on the Lockscreen scene and then
                        // releasing before the transition to the Bouncer scene is committed, the
                        // KTF transition of LOCKSCREEN -> PRIMARY_BOUNCER received a CANCELED and
                        // the scene framework immediately starts a reversed transition of
                        // PRIMARY_BOUNCER -> LOCKSCREEN, which picks up where the previous one left
                        // off.
                        //
                        // If it were allowed for the CANCELED from the original KTF transition to
                        // emit a value, a race condition could form where the value from CANCELED
                        // arrives downstream _after_ the reversed transition is finished, causing
                        // the transition to end up in an incorrect state at rest.
                        null
                    } else {
                        StateToValue(
                                from = step.from,
                                to = step.to,
@@ -166,10 +180,11 @@ constructor(
                                        RUNNING -> stepToValue(step)
                                        CANCELED -> onCancel?.invoke()
                                        FINISHED -> onFinish?.invoke()
                                }
                                    },
                            )
                            .also { logger.logTransitionStep(name, step, it.value) }
                    }
                }
                .distinctUntilChanged()
        }

@@ -181,7 +196,7 @@ constructor(
                duration = 1.milliseconds,
                onStep = { value },
                onCancel = { value },
                onFinish = { value }
                onFinish = { value },
            )
        }
    }