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

Commit 5699d7c2 authored by Matt Pietal's avatar Matt Pietal
Browse files

Fix bouncer dismiss race condition 

The bouncer startDisappear flow contains a runnable which must
be consumed in order to tell KeyguardViewMediator to begin hiding
the keyguard. This is supposed to happen after the bouncer disappear
animation plays. However, in rare cases, the runnable gets conflated
with a null value that is intended to clear it.

Use MutableSharedFlow instead of conflating with MutableStateFlow

Test: atest KeyguardBouncerRepositoryTest
Fixes: 383318506
Flag: EXEMPT bugfix
Change-Id: I7c697fdc5c0bbc5b418065f8b65d48aca68b1953
parent 127461b4

packages/SystemUI/tests/src/com/android/systemui/bouncer/data/repository/KeyguardBouncerRepositoryTest.kt→packages/SystemUI/multivalentTests/src/com/android/systemui/bouncer/data/repository/KeyguardBouncerRepositoryTest.kt

+94 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2025 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.bouncer.data.repository



import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.android.systemui.coroutines.collectValues

import com.android.systemui.kosmos.testScope

import com.android.systemui.log.table.TableLogBuffer

import com.android.systemui.testKosmos

import com.android.systemui.util.mockito.any

import com.android.systemui.util.mockito.eq

import com.android.systemui.util.time.SystemClock

import com.google.common.truth.Truth.assertThat

import kotlinx.coroutines.test.runCurrent

import kotlinx.coroutines.test.runTest

import org.junit.Before

import org.junit.Test
@@ -33,18 +51,44 @@ class KeyguardBouncerRepositoryTest : SysuiTestCase() { 
    fun setup() {

        MockitoAnnotations.initMocks(this)

        underTest =

            object :

                KeyguardBouncerRepositoryImpl(

                    systemClock,

                    testScope.backgroundScope,

                    bouncerLogger,

            )

                ) {

                override fun isDebuggable(): Boolean = true

            }

    }



    @Test

    fun changingFlowValueTriggersLogging() =

        testScope.runTest {

            underTest.setPrimaryShow(true)

            runCurrent()

            Mockito.verify(bouncerLogger)

                .logChange(eq(""), eq("PrimaryBouncerShow"), value = eq(false), any())

        }



    @Test

    fun primaryStartDisappearAnimation() =

        testScope.runTest {

            assertThat(underTest.isPrimaryBouncerStartingDisappearAnimation()).isFalse()



            underTest.setPrimaryStartDisappearAnimation(Runnable {})

            assertThat(underTest.isPrimaryBouncerStartingDisappearAnimation()).isTrue()



            underTest.setPrimaryStartDisappearAnimation(null)

            assertThat(underTest.isPrimaryBouncerStartingDisappearAnimation()).isFalse()



            val disappearFlow by collectValues(underTest.primaryBouncerStartingDisappearAnimation)

            underTest.setPrimaryStartDisappearAnimation(null)

            assertThat(disappearFlow[0]).isNull()



            // Now issue two in a row to make sure one is not dropped

            underTest.setPrimaryStartDisappearAnimation(Runnable {})

            underTest.setPrimaryStartDisappearAnimation(null)

            assertThat(disappearFlow[1]).isNotNull()

            assertThat(disappearFlow[2]).isNull()

        }

}

packages/SystemUI/multivalentTests/src/com/android/systemui/bouncer/domain/interactor/PrimaryBouncerInteractorTest.kt

+3 −6
Original line number Diff line number Diff line
@@ -105,7 +105,7 @@ class PrimaryBouncerInteractorTest : SysuiTestCase() { 
                mSelectedUserInteractor,

                faceAuthInteractor,

            )

        whenever(repository.primaryBouncerStartingDisappearAnimation.value).thenReturn(null)

        whenever(repository.isPrimaryBouncerStartingDisappearAnimation()).thenReturn(false)

        whenever(repository.primaryBouncerShow.value).thenReturn(false)

        whenever(bouncerView.delegate).thenReturn(bouncerViewDelegate)

        resources = context.orCreateTestableResources
@@ -199,7 +199,6 @@ class PrimaryBouncerInteractorTest : SysuiTestCase() { 
    @Test

    fun testExpansion_fullyShown() {

        whenever(repository.panelExpansionAmount.value).thenReturn(0.5f)

        whenever(repository.primaryBouncerStartingDisappearAnimation.value).thenReturn(null)

        underTest.setPanelExpansion(EXPANSION_VISIBLE)

        verify(falsingCollector).onBouncerShown()

        verify(mPrimaryBouncerCallbackInteractor).dispatchFullyShown()
@@ -208,7 +207,6 @@ class PrimaryBouncerInteractorTest : SysuiTestCase() { 
    @Test

    fun testExpansion_fullyHidden() {

        whenever(repository.panelExpansionAmount.value).thenReturn(0.5f)

        whenever(repository.primaryBouncerStartingDisappearAnimation.value).thenReturn(null)

        underTest.setPanelExpansion(EXPANSION_HIDDEN)

        verify(repository).setPrimaryShow(false)

        verify(falsingCollector).onBouncerHidden()
@@ -307,7 +305,6 @@ class PrimaryBouncerInteractorTest : SysuiTestCase() { 
    fun testIsFullShowing() {

        whenever(repository.primaryBouncerShow.value).thenReturn(true)

        whenever(repository.panelExpansionAmount.value).thenReturn(EXPANSION_VISIBLE)

        whenever(repository.primaryBouncerStartingDisappearAnimation.value).thenReturn(null)

        assertThat(underTest.isFullyShowing()).isTrue()

        whenever(repository.primaryBouncerShow.value).thenReturn(false)

        assertThat(underTest.isFullyShowing()).isFalse()
@@ -333,9 +330,9 @@ class PrimaryBouncerInteractorTest : SysuiTestCase() { 


    @Test

    fun testIsAnimatingAway() {

        whenever(repository.primaryBouncerStartingDisappearAnimation.value).thenReturn(Runnable {})

        whenever(repository.isPrimaryBouncerStartingDisappearAnimation()).thenReturn(true)

        assertThat(underTest.isAnimatingAway()).isTrue()

        whenever(repository.primaryBouncerStartingDisappearAnimation.value).thenReturn(null)

        whenever(repository.isPrimaryBouncerStartingDisappearAnimation()).thenReturn(false)

        assertThat(underTest.isAnimatingAway()).isFalse()

    }

packages/SystemUI/src/com/android/systemui/bouncer/data/repository/KeyguardBouncerRepository.kt

+24 −6
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.systemui.bouncer.data.repository



import android.annotation.SuppressLint

import android.os.Build

import android.util.Log

import com.android.keyguard.KeyguardSecurityModel
@@ -51,7 +52,11 @@ interface KeyguardBouncerRepository { 
    val primaryBouncerShow: StateFlow<Boolean>

    val primaryBouncerShowingSoon: StateFlow<Boolean>

    val primaryBouncerStartingToHide: StateFlow<Boolean>

    val primaryBouncerStartingDisappearAnimation: StateFlow<Runnable?>

    val primaryBouncerStartingDisappearAnimation: MutableSharedFlow<Runnable?>



    fun isPrimaryBouncerStartingDisappearAnimation(): Boolean



    fun isDebuggable(): Boolean



    /** Determines if we want to instantaneously show the primary bouncer instead of translating. */

    val primaryBouncerScrimmed: StateFlow<Boolean>
@@ -128,7 +133,7 @@ interface KeyguardBouncerRepository { 
}



@SysUISingleton

class KeyguardBouncerRepositoryImpl

open class KeyguardBouncerRepositoryImpl

@Inject

constructor(

    private val clock: SystemClock,
@@ -144,9 +149,19 @@ constructor( 
    override val primaryBouncerShowingSoon = _primaryBouncerShowingSoon.asStateFlow()

    private val _primaryBouncerStartingToHide = MutableStateFlow(false)

    override val primaryBouncerStartingToHide = _primaryBouncerStartingToHide.asStateFlow()

    private val _primaryBouncerDisappearAnimation = MutableStateFlow<Runnable?>(null)



    @SuppressLint("SharedFlowCreation")

    override val primaryBouncerStartingDisappearAnimation =

        _primaryBouncerDisappearAnimation.asStateFlow()

        MutableSharedFlow<Runnable?>(extraBufferCapacity = 2, replay = 1)



    override fun isPrimaryBouncerStartingDisappearAnimation(): Boolean {

        val replayCache = primaryBouncerStartingDisappearAnimation.replayCache

        return if (!replayCache.isEmpty()) {

            replayCache.last() != null

        } else {

            false

        }

    }



    /** Determines if we want to instantaneously show the primary bouncer instead of translating. */

    private val _primaryBouncerScrimmed = MutableStateFlow(false)
@@ -177,6 +192,7 @@ constructor( 
        _keyguardAuthenticatedPrimaryAuth.asSharedFlow()



    /** Whether the user requested to show the bouncer when device is already authenticated */

    @SuppressLint("SharedFlowCreation")

    private val _userRequestedBouncerWhenAlreadyAuthenticated = MutableSharedFlow<Int>()

    override val userRequestedBouncerWhenAlreadyAuthenticated: Flow<Int> =

        _userRequestedBouncerWhenAlreadyAuthenticated.asSharedFlow()
@@ -226,7 +242,7 @@ constructor( 
    }



    override fun setPrimaryStartDisappearAnimation(runnable: Runnable?) {

        _primaryBouncerDisappearAnimation.value = runnable

        primaryBouncerStartingDisappearAnimation.tryEmit(runnable)

    }



    override fun setPanelExpansion(panelExpansion: Float) {
@@ -265,9 +281,11 @@ constructor( 
        _lastShownSecurityMode.value = securityMode

    }



    override fun isDebuggable() = Build.IS_DEBUGGABLE



    /** Sets up logs for state flows. */

    private fun setUpLogging() {

        if (!Build.IS_DEBUGGABLE) {

        if (!isDebuggable()) {

            return

        }

packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/PrimaryBouncerInteractor.kt

+7 −7
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import android.os.Handler 
import android.os.Trace

import android.util.Log

import android.view.View

import com.android.app.tracing.coroutines.launchTraced as launch

import com.android.keyguard.KeyguardSecurityModel

import com.android.keyguard.KeyguardUpdateMonitor

import com.android.systemui.DejankUtils
@@ -54,7 +55,6 @@ import kotlinx.coroutines.flow.combine 
import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.flow.filterNotNull

import kotlinx.coroutines.flow.map

import com.android.app.tracing.coroutines.launchTraced as launch



/**

 * Encapsulates business logic for interacting with the lock-screen primary (pin/pattern/password)
@@ -145,7 +145,7 @@ constructor( 
                TAG,

                "PrimaryBouncerInteractor#show is being called before the " +

                    "primaryBouncerDelegate is set. Let's exit early so we don't " +

                    "set the wrong primaryBouncer state."

                    "set the wrong primaryBouncer state.",

            )

            return false

        }
@@ -197,7 +197,7 @@ constructor( 
        if (isFullyShowing()) {

            SysUiStatsLog.write(

                SysUiStatsLog.KEYGUARD_BOUNCER_STATE_CHANGED,

                SysUiStatsLog.KEYGUARD_BOUNCER_STATE_CHANGED__STATE__HIDDEN

                SysUiStatsLog.KEYGUARD_BOUNCER_STATE_CHANGED__STATE__HIDDEN,

            )

            dismissCallbackRegistry.notifyDismissCancelled()

        }
@@ -223,7 +223,7 @@ constructor( 
    fun setPanelExpansion(expansion: Float) {

        val oldExpansion = repository.panelExpansionAmount.value

        val expansionChanged = oldExpansion != expansion

        if (repository.primaryBouncerStartingDisappearAnimation.value == null) {

        if (!repository.isPrimaryBouncerStartingDisappearAnimation()) {

            repository.setPanelExpansion(expansion)

        }
@@ -272,7 +272,7 @@ constructor( 
     */

    fun setDismissAction(

        onDismissAction: ActivityStarter.OnDismissAction?,

        cancelAction: Runnable?

        cancelAction: Runnable?,

    ) {

        repository.bouncerDismissActionModel =

            if (onDismissAction != null && cancelAction != null) {
@@ -344,7 +344,7 @@ constructor( 
    fun isFullyShowing(): Boolean {

        return (repository.primaryBouncerShowingSoon.value || isBouncerShowing()) &&

            repository.panelExpansionAmount.value == KeyguardBouncerConstants.EXPANSION_VISIBLE &&

            repository.primaryBouncerStartingDisappearAnimation.value == null

            !repository.isPrimaryBouncerStartingDisappearAnimation()

    }



    /** Returns whether bouncer is scrimmed. */
@@ -361,7 +361,7 @@ constructor( 


    /** Return whether bouncer is animating away. */

    fun isAnimatingAway(): Boolean {

        return repository.primaryBouncerStartingDisappearAnimation.value != null

        return repository.isPrimaryBouncerStartingDisappearAnimation()

    }



    /** Return whether bouncer will dismiss with actions */

packages/SystemUI/tests/utils/src/com/android/systemui/bouncer/data/repository/FakeKeyguardBouncerRepository.kt

+14 −3
Original line number Diff line number Diff line
@@ -23,9 +23,18 @@ class FakeKeyguardBouncerRepository @Inject constructor() : KeyguardBouncerRepos 
    override val primaryBouncerShowingSoon = _primaryBouncerShowingSoon.asStateFlow()

    private val _primaryBouncerStartingToHide = MutableStateFlow(false)

    override val primaryBouncerStartingToHide = _primaryBouncerStartingToHide.asStateFlow()

    private val _primaryBouncerDisappearAnimation = MutableStateFlow<Runnable?>(null)

    override val primaryBouncerStartingDisappearAnimation =

        _primaryBouncerDisappearAnimation.asStateFlow()

        MutableSharedFlow<Runnable?>(extraBufferCapacity = 2, replay = 1)



    override fun isPrimaryBouncerStartingDisappearAnimation(): Boolean {

        val replayCache = primaryBouncerStartingDisappearAnimation.replayCache

        return if (!replayCache.isEmpty()) {

            replayCache.last() != null

        } else {

            false

        }

    }



    private val _primaryBouncerScrimmed = MutableStateFlow(false)

    override val primaryBouncerScrimmed = _primaryBouncerScrimmed.asStateFlow()

    private val _panelExpansionAmount = MutableStateFlow(KeyguardBouncerConstants.EXPANSION_HIDDEN)
@@ -53,6 +62,8 @@ class FakeKeyguardBouncerRepository @Inject constructor() : KeyguardBouncerRepos 
        MutableStateFlow(KeyguardSecurityModel.SecurityMode.Invalid)

    override var bouncerDismissActionModel: BouncerDismissActionModel? = null



    override fun isDebuggable() = true



    override fun setPrimaryScrimmed(isScrimmed: Boolean) {

        _primaryBouncerScrimmed.value = isScrimmed

    }
@@ -74,7 +85,7 @@ class FakeKeyguardBouncerRepository @Inject constructor() : KeyguardBouncerRepos 
    }



    override fun setPrimaryStartDisappearAnimation(runnable: Runnable?) {

        _primaryBouncerDisappearAnimation.value = runnable

        primaryBouncerStartingDisappearAnimation.tryEmit(runnable)

    }



    override fun setPanelExpansion(panelExpansion: Float) {