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

Commit 88d438f8 authored by Ioana Alexandru's avatar Ioana Alexandru
Browse files

[flexiglass] Remove usages of transitionValue with deprecated states

When transitionValue is called with a state that is modeled as a scene
or overlay in flexiglass, it ends up bundling them all up together as
UNDEFINED. This means that states like OCCLUDED and GONE were actually
"any state that's not defined in KTF", including things like BOUNCER,
leading to incorrect transition values.

There are currently a few known transition bugs related to this, all
coming from the OCCLUDED check in SharedNotificationContainerViewModel,
but this will be an issue for every other usage of transitionValue with
a deprecated state, so I updated all the usages I could find to
hopefully prevent other similar issues.

Fix: 440600291
Fix: 440669658
Flag: com.android.systemui.scene_container
Test: tested manually + existing unit tests pass
Change-Id: Iaafc40730e4cb0594278d9ccc0b4e7c66d7acc93
parent 66ec497d
Loading
Loading
Loading
Loading
+29 −4
Original line number Diff line number Diff line
@@ -15,12 +15,14 @@
 */
package com.android.systemui.flags

import androidx.test.ext.junit.runners.AndroidJUnit4
import android.platform.test.flag.junit.FlagsParameterization
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.coroutines.collectLastValue
import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor
import com.android.systemui.keyguard.shared.model.KeyguardState
import com.android.systemui.scene.domain.interactor.SceneInteractor
import com.android.systemui.scene.shared.model.Scenes
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.StandardTestDispatcher
@@ -34,27 +36,44 @@ import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.`when` as whenever
import org.mockito.MockitoAnnotations
import platform.test.runner.parameterized.ParameterizedAndroidJunit4
import platform.test.runner.parameterized.Parameters

/**
 * Be careful with the {FeatureFlagsReleaseRestarter} in this test. It has a call to System.exit()!
 */
@SmallTest
@RunWith(AndroidJUnit4::class)
class NotOccludedConditionTest : SysuiTestCase() {
@RunWith(ParameterizedAndroidJunit4::class)
class NotOccludedConditionTest(flags: FlagsParameterization) : SysuiTestCase() {
    private lateinit var condition: NotOccludedCondition

    @Mock private lateinit var keyguardTransitionInteractor: KeyguardTransitionInteractor
    @Mock private lateinit var sceneInteractor: SceneInteractor
    private val transitionValue = MutableStateFlow(0f)
    private val currentScene = MutableStateFlow(Scenes.Gone)

    private val testDispatcher: TestDispatcher = StandardTestDispatcher()
    private val testScope: TestScope = TestScope(testDispatcher)

    companion object {
        @JvmStatic
        @Parameters(name = "{0}")
        fun getParams(): List<FlagsParameterization> {
            return FlagsParameterization.allCombinationsOf().andSceneContainer()
        }
    }

    init {
        mSetFlagsRule.setFlagsParameterization(flags)
    }

    @Before
    fun setup() {
        MockitoAnnotations.initMocks(this)
        whenever(keyguardTransitionInteractor.transitionValue(KeyguardState.OCCLUDED))
            .thenReturn(transitionValue)
        condition = NotOccludedCondition({ keyguardTransitionInteractor })
        whenever(sceneInteractor.currentScene).thenReturn(currentScene)
        condition = NotOccludedCondition({ keyguardTransitionInteractor }, { sceneInteractor })
        testScope.runCurrent()
    }

@@ -63,7 +82,9 @@ class NotOccludedConditionTest : SysuiTestCase() {
        testScope.runTest {
            val canRestart by collectLastValue(condition.canRestartNow)

            currentScene.emit(Scenes.Occluded)
            transitionValue.emit(1f)

            assertThat(canRestart).isFalse()
        }

@@ -72,7 +93,9 @@ class NotOccludedConditionTest : SysuiTestCase() {
        testScope.runTest {
            val canRestart by collectLastValue(condition.canRestartNow)

            currentScene.emit(Scenes.Lockscreen)
            transitionValue.emit(0f)

            assertThat(canRestart).isTrue()
        }

@@ -81,10 +104,12 @@ class NotOccludedConditionTest : SysuiTestCase() {
        testScope.runTest {
            val canRestart by collectLastValue(condition.canRestartNow)

            currentScene.emit(Scenes.Occluded)
            transitionValue.emit(1f)

            assertThat(canRestart).isFalse()

            currentScene.emit(Scenes.Gone)
            transitionValue.emit(0f)

            assertThat(canRestart).isTrue()
+12 −5
Original line number Diff line number Diff line
@@ -2,23 +2,30 @@ package com.android.systemui.flags

import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor
import com.android.systemui.keyguard.shared.model.KeyguardState
import com.android.systemui.scene.domain.interactor.SceneInteractor
import com.android.systemui.scene.shared.flag.SceneContainerFlag
import com.android.systemui.scene.shared.model.Scenes
import dagger.Lazy
import javax.inject.Inject
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map

/** Returns true when the device is "asleep" as defined by the [WakefullnessLifecycle]. */
class NotOccludedCondition
@Inject
constructor(
    private val keyguardTransitionInteractorLazy: Lazy<KeyguardTransitionInteractor>,
    private val sceneInteractorProvider: Lazy<SceneInteractor>,
) : ConditionalRestarter.Condition {

    /** Returns true when the lockscreen isn't occluded by an activity (like a call or camera). */
    override val canRestartNow: Flow<Boolean>
        get() {
            return keyguardTransitionInteractorLazy
                .get()
                .transitionValue(KeyguardState.OCCLUDED)
                .map { it == 0f }
            return if (SceneContainerFlag.isEnabled) {
                sceneInteractorProvider.get().currentScene.map { it != Scenes.Occluded }
            } else {
                keyguardTransitionInteractorLazy.get().transitionValue(KeyguardState.OCCLUDED).map {
                    it == 0f
                }
            }
        }
}
+2 −0
Original line number Diff line number Diff line
@@ -412,6 +412,8 @@ constructor(
        configurationInteractor
            .dimensionPixelSize(R.dimen.keyguard_translate_distance_on_swipe_up)
            .flatMapLatest { translationDistance ->
                // TODO: b/441274212 - Update this to use the correct signals when SceneContainer is
                //  turned on.
                combineTransform(
                    shadeRepository.legacyShadeExpansion.onStart { emit(0f) },
                    keyguardTransitionInteractor.transitionValue(GONE).onStart { emit(0f) },
+4 −5
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ import com.android.systemui.keyguard.ui.viewmodel.ViewStateAccessor
import com.android.systemui.media.controls.domain.pipeline.MediaDataManager
import com.android.systemui.media.controls.shared.model.MediaData
import com.android.systemui.res.R
import com.android.systemui.scene.domain.interactor.SceneInteractor
import com.android.systemui.scene.shared.flag.SceneContainerFlag
import com.android.systemui.scene.shared.model.Overlays
import com.android.systemui.scene.shared.model.Scenes
@@ -143,6 +144,7 @@ constructor(
    private val keyguardInteractor: KeyguardInteractor,
    private val keyguardTransitionInteractor: KeyguardTransitionInteractor,
    private val shadeInteractor: ShadeInteractor,
    private val sceneInteractor: SceneInteractor,
    private val bouncerInteractor: BouncerInteractor,
    shadeModeInteractor: ShadeModeInteractor,
    notificationStackAppearanceInteractor: NotificationStackAppearanceInteractor,
@@ -704,15 +706,12 @@ constructor(
    }

    fun keyguardAlpha(viewState: ViewStateAccessor, scope: CoroutineScope): Flow<Float> {
        val isKeyguardOccluded =
            keyguardTransitionInteractor.transitionValue(OCCLUDED).map { it == 1f }

        val isKeyguardNotVisibleInState =
            if (SceneContainerFlag.isEnabled) {
                isKeyguardOccluded
                sceneInteractor.currentScene.map { it == Scenes.Occluded }
            } else {
                anyOf(
                    isKeyguardOccluded,
                    keyguardTransitionInteractor.transitionValue(OCCLUDED).map { it == 1f },
                    keyguardTransitionInteractor
                        .transitionValue(content = Scenes.Gone, stateWithoutSceneContainer = GONE)
                        .map { it == 1f },
+2 −1
Original line number Diff line number Diff line
@@ -57,7 +57,7 @@ import com.android.systemui.kosmos.Kosmos
import com.android.systemui.kosmos.Kosmos.Fixture
import com.android.systemui.kosmos.applicationCoroutineScope
import com.android.systemui.media.controls.domain.pipeline.legacyMediaDataManagerImpl
import com.android.systemui.media.controls.domain.pipeline.mediaDataManager
import com.android.systemui.scene.domain.interactor.sceneInteractor
import com.android.systemui.shade.domain.interactor.shadeInteractor
import com.android.systemui.shade.domain.interactor.shadeModeInteractor
import com.android.systemui.shade.largeScreenHeaderHelper
@@ -78,6 +78,7 @@ val Kosmos.sharedNotificationContainerViewModel by Fixture {
        keyguardInteractor = keyguardInteractor,
        keyguardTransitionInteractor = keyguardTransitionInteractor,
        shadeInteractor = shadeInteractor,
        sceneInteractor = sceneInteractor,
        bouncerInteractor = bouncerInteractor,
        shadeModeInteractor = shadeModeInteractor,
        notificationStackAppearanceInteractor = notificationStackAppearanceInteractor,