From a0d7d457b352b09c264f2190fabab41e62b2e694 Mon Sep 17 00:00:00 2001 From: Aaron Liu Date: Tue, 28 Mar 2023 10:22:04 -0700 Subject: [PATCH] Ensure that flow is run on main dispatcher When adding and removing the callback for KeyguardUpdateMonitor, there is an assertion that the operation must be on the main thread. The callback was being removed on a worker thread. We can ensure that this is not the case with flowOn. Fixes: 274558013 Test: restart device and unlock the device several times. Change-Id: I1f40e8b7f2cf7245f0cb17e2d0e10b9ece619234 --- .../data/repository/KeyguardRepository.kt | 5 + .../repository/KeyguardRepositoryImplTest.kt | 100 ++++++++++++++---- 2 files changed, 84 insertions(+), 21 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardRepository.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardRepository.kt index 76f20d25b0ec1..6bc9837e87516 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardRepository.kt @@ -25,6 +25,7 @@ import com.android.systemui.common.coroutine.ChannelExt.trySendWithFailureLoggin import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow import com.android.systemui.common.shared.model.Position import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.doze.DozeHost import com.android.systemui.doze.DozeMachine import com.android.systemui.doze.DozeTransitionCallback @@ -43,12 +44,14 @@ import com.android.systemui.statusbar.phone.BiometricUnlockController.WakeAndUnl import com.android.systemui.statusbar.phone.DozeParameters import com.android.systemui.statusbar.policy.KeyguardStateController import javax.inject.Inject +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.flowOn /** Defines interface for classes that encapsulate application state for the keyguard. */ interface KeyguardRepository { @@ -195,6 +198,7 @@ constructor( private val dozeParameters: DozeParameters, private val authController: AuthController, private val dreamOverlayCallbackController: DreamOverlayCallbackController, + @Main private val mainDispatcher: CoroutineDispatcher ) : KeyguardRepository { private val _animateBottomAreaDozingTransitions = MutableStateFlow(false) override val animateBottomAreaDozingTransitions = @@ -387,6 +391,7 @@ constructor( awaitClose { keyguardUpdateMonitor.removeCallback(callback) } } + .flowOn(mainDispatcher) .distinctUntilChanged() override val linearDozeAmount: Flow = conflatedCallbackFlow { diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/KeyguardRepositoryImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/KeyguardRepositoryImplTest.kt index 0e6f8d4e0720e..ce17167f587d1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/KeyguardRepositoryImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/repository/KeyguardRepositoryImplTest.kt @@ -46,10 +46,12 @@ import com.android.systemui.util.mockito.argumentCaptor import com.android.systemui.util.mockito.whenever import com.android.systemui.util.mockito.withArgCaptor import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Before @@ -59,6 +61,7 @@ import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations +@OptIn(ExperimentalCoroutinesApi::class) @SmallTest @RunWith(AndroidJUnit4::class) class KeyguardRepositoryImplTest : SysuiTestCase() { @@ -73,6 +76,9 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Mock private lateinit var keyguardUpdateMonitor: KeyguardUpdateMonitor @Mock private lateinit var dreamOverlayCallbackController: DreamOverlayCallbackController @Mock private lateinit var dozeParameters: DozeParameters + private val mainDispatcher = StandardTestDispatcher() + private val testDispatcher = StandardTestDispatcher() + private val testScope = TestScope(testDispatcher) private lateinit var underTest: KeyguardRepositoryImpl @@ -92,12 +98,13 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { dozeParameters, authController, dreamOverlayCallbackController, + mainDispatcher ) } @Test fun animateBottomAreaDozingTransitions() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { assertThat(underTest.animateBottomAreaDozingTransitions.value).isEqualTo(false) underTest.setAnimateDozingTransitions(true) @@ -112,7 +119,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun bottomAreaAlpha() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { assertThat(underTest.bottomAreaAlpha.value).isEqualTo(1f) underTest.setBottomAreaAlpha(0.1f) @@ -133,7 +140,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun clockPosition() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { assertThat(underTest.clockPosition.value).isEqualTo(Position(0, 0)) underTest.setClockPosition(0, 1) @@ -151,11 +158,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun isKeyguardShowing() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { whenever(keyguardStateController.isShowing).thenReturn(false) var latest: Boolean? = null val job = underTest.isKeyguardShowing.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isFalse() assertThat(underTest.isKeyguardShowing()).isFalse() @@ -164,11 +172,13 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { whenever(keyguardStateController.isShowing).thenReturn(true) captor.value.onKeyguardShowingChanged() + runCurrent() assertThat(latest).isTrue() assertThat(underTest.isKeyguardShowing()).isTrue() whenever(keyguardStateController.isShowing).thenReturn(false) captor.value.onKeyguardShowingChanged() + runCurrent() assertThat(latest).isFalse() assertThat(underTest.isKeyguardShowing()).isFalse() @@ -197,11 +207,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun isKeyguardOccluded() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { whenever(keyguardStateController.isOccluded).thenReturn(false) var latest: Boolean? = null val job = underTest.isKeyguardOccluded.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isFalse() val captor = argumentCaptor() @@ -209,10 +220,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { whenever(keyguardStateController.isOccluded).thenReturn(true) captor.value.onKeyguardShowingChanged() + runCurrent() assertThat(latest).isTrue() whenever(keyguardStateController.isOccluded).thenReturn(false) captor.value.onKeyguardShowingChanged() + runCurrent() assertThat(latest).isFalse() job.cancel() @@ -220,11 +233,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun isKeyguardUnlocked() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { whenever(keyguardStateController.isUnlocked).thenReturn(false) var latest: Boolean? = null val job = underTest.isKeyguardUnlocked.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isFalse() val captor = argumentCaptor() @@ -232,10 +246,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { whenever(keyguardStateController.isUnlocked).thenReturn(true) captor.value.onUnlockedChanged() + runCurrent() assertThat(latest).isTrue() whenever(keyguardStateController.isUnlocked).thenReturn(false) captor.value.onUnlockedChanged() + runCurrent() assertThat(latest).isFalse() job.cancel() @@ -243,82 +259,98 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun isDozing() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { var latest: Boolean? = null val job = underTest.isDozing.onEach { latest = it }.launchIn(this) + runCurrent() val captor = argumentCaptor() verify(dozeHost).addCallback(captor.capture()) captor.value.onDozingChanged(true) + runCurrent() assertThat(latest).isTrue() captor.value.onDozingChanged(false) + runCurrent() assertThat(latest).isFalse() job.cancel() + runCurrent() verify(dozeHost).removeCallback(captor.value) } @Test fun `isDozing - starts with correct initial value for isDozing`() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { var latest: Boolean? = null whenever(statusBarStateController.isDozing).thenReturn(true) var job = underTest.isDozing.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isTrue() job.cancel() whenever(statusBarStateController.isDozing).thenReturn(false) job = underTest.isDozing.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isFalse() job.cancel() } @Test fun dozeAmount() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { val values = mutableListOf() val job = underTest.linearDozeAmount.onEach(values::add).launchIn(this) val captor = argumentCaptor() + runCurrent() verify(statusBarStateController).addCallback(captor.capture()) captor.value.onDozeAmountChanged(0.433f, 0.4f) + runCurrent() captor.value.onDozeAmountChanged(0.498f, 0.5f) + runCurrent() captor.value.onDozeAmountChanged(0.661f, 0.65f) + runCurrent() assertThat(values).isEqualTo(listOf(0f, 0.433f, 0.498f, 0.661f)) job.cancel() + runCurrent() verify(statusBarStateController).removeCallback(captor.value) } @Test fun wakefulness() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { val values = mutableListOf() val job = underTest.wakefulness.onEach(values::add).launchIn(this) + runCurrent() val captor = argumentCaptor() verify(wakefulnessLifecycle).addObserver(captor.capture()) whenever(wakefulnessLifecycle.wakefulness) .thenReturn(WakefulnessLifecycle.WAKEFULNESS_WAKING) captor.value.onStartedWakingUp() + runCurrent() whenever(wakefulnessLifecycle.wakefulness) .thenReturn(WakefulnessLifecycle.WAKEFULNESS_AWAKE) captor.value.onFinishedWakingUp() + runCurrent() whenever(wakefulnessLifecycle.wakefulness) .thenReturn(WakefulnessLifecycle.WAKEFULNESS_GOING_TO_SLEEP) captor.value.onStartedGoingToSleep() + runCurrent() whenever(wakefulnessLifecycle.wakefulness) .thenReturn(WakefulnessLifecycle.WAKEFULNESS_ASLEEP) captor.value.onFinishedGoingToSleep() + runCurrent() assertThat(values.map { it.state }) .isEqualTo( @@ -333,12 +365,13 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { ) job.cancel() + runCurrent() verify(wakefulnessLifecycle).removeObserver(captor.value) } @Test fun isUdfpsSupported() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { whenever(keyguardUpdateMonitor.isUdfpsSupported).thenReturn(true) assertThat(underTest.isUdfpsSupported()).isTrue() @@ -348,11 +381,11 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun isKeyguardGoingAway() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { whenever(keyguardStateController.isKeyguardGoingAway).thenReturn(false) var latest: Boolean? = null val job = underTest.isKeyguardGoingAway.onEach { latest = it }.launchIn(this) - + runCurrent() assertThat(latest).isFalse() val captor = argumentCaptor() @@ -360,10 +393,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { whenever(keyguardStateController.isKeyguardGoingAway).thenReturn(true) captor.value.onKeyguardGoingAwayChanged() + runCurrent() assertThat(latest).isTrue() whenever(keyguardStateController.isKeyguardGoingAway).thenReturn(false) captor.value.onKeyguardGoingAwayChanged() + runCurrent() assertThat(latest).isFalse() job.cancel() @@ -371,20 +406,23 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun isDreamingFromKeyguardUpdateMonitor() = - runTest(UnconfinedTestDispatcher()) { + TestScope(mainDispatcher).runTest { whenever(keyguardUpdateMonitor.isDreaming()).thenReturn(false) var latest: Boolean? = null val job = underTest.isDreaming.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isFalse() val captor = argumentCaptor() verify(keyguardUpdateMonitor).registerCallback(captor.capture()) captor.value.onDreamingStateChanged(true) + runCurrent() assertThat(latest).isTrue() captor.value.onDreamingStateChanged(false) + runCurrent() assertThat(latest).isFalse() job.cancel() @@ -392,11 +430,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun isDreamingFromDreamOverlayCallbackController() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { whenever(dreamOverlayCallbackController.isDreaming).thenReturn(false) var latest: Boolean? = null val job = underTest.isDreamingWithOverlay.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isFalse() val listener = @@ -405,9 +444,11 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { } listener.onStartDream() + runCurrent() assertThat(latest).isTrue() listener.onWakeUp() + runCurrent() assertThat(latest).isFalse() job.cancel() @@ -415,10 +456,11 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun biometricUnlockState() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { val values = mutableListOf() val job = underTest.biometricUnlockState.onEach(values::add).launchIn(this) + runCurrent() val captor = argumentCaptor() verify(biometricUnlockController).addBiometricModeListener(captor.capture()) @@ -435,6 +477,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { .forEach { whenever(biometricUnlockController.mode).thenReturn(it) captor.value.onModeChanged(it) + runCurrent() } assertThat(values) @@ -454,12 +497,13 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { ) job.cancel() + runCurrent() verify(biometricUnlockController).removeBiometricModeListener(captor.value) } @Test fun dozeTransitionModel() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { // For the initial state whenever(dozeTransitionListener.oldState).thenReturn(DozeMachine.State.UNINITIALIZED) whenever(dozeTransitionListener.newState).thenReturn(DozeMachine.State.UNINITIALIZED) @@ -467,6 +511,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { val values = mutableListOf() val job = underTest.dozeTransitionModel.onEach(values::add).launchIn(this) + runCurrent() val listener = withArgCaptor { verify(dozeTransitionListener).addCallback(capture()) @@ -475,20 +520,26 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { // These don't have to reflect real transitions from the DozeMachine. Only that the // transitions are properly emitted listener.onDozeTransition(DozeMachine.State.INITIALIZED, DozeMachine.State.DOZE) + runCurrent() listener.onDozeTransition(DozeMachine.State.DOZE, DozeMachine.State.DOZE_AOD) + runCurrent() listener.onDozeTransition(DozeMachine.State.DOZE_AOD_DOCKED, DozeMachine.State.FINISH) + runCurrent() listener.onDozeTransition( DozeMachine.State.DOZE_REQUEST_PULSE, DozeMachine.State.DOZE_PULSING ) + runCurrent() listener.onDozeTransition( DozeMachine.State.DOZE_SUSPEND_TRIGGERS, DozeMachine.State.DOZE_PULSE_DONE ) + runCurrent() listener.onDozeTransition( DozeMachine.State.DOZE_AOD_PAUSING, DozeMachine.State.DOZE_AOD_PAUSED ) + runCurrent() assertThat(values) .isEqualTo( @@ -517,15 +568,17 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { ) job.cancel() + runCurrent() verify(dozeTransitionListener).removeCallback(listener) } @Test fun fingerprintSensorLocation() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { val values = mutableListOf() val job = underTest.fingerprintSensorLocation.onEach(values::add).launchIn(this) + runCurrent() val captor = argumentCaptor() verify(authController).addCallback(captor.capture()) @@ -539,6 +592,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { .onEach { whenever(authController.fingerprintSensorLocation).thenReturn(it) captor.value.onFingerprintLocationChanged() + runCurrent() } .also { dispatchedSensorLocations -> assertThat(values).isEqualTo(listOf(null) + dispatchedSensorLocations) @@ -549,11 +603,12 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun faceSensorLocation() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { val values = mutableListOf() val job = underTest.faceSensorLocation.onEach(values::add).launchIn(this) val captor = argumentCaptor() + runCurrent() verify(authController).addCallback(captor.capture()) // An initial, null value should be initially emitted so that flows combined with this @@ -571,6 +626,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { .onEach { whenever(authController.faceSensorLocation).thenReturn(it) captor.value.onFaceSensorLocationChanged() + runCurrent() } .also { dispatchedSensorLocations -> assertThat(values).isEqualTo(listOf(null) + dispatchedSensorLocations) @@ -581,10 +637,11 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { @Test fun biometricUnlockSource() = - runTest(UnconfinedTestDispatcher()) { + testScope.runTest { val values = mutableListOf() val job = underTest.biometricUnlockSource.onEach(values::add).launchIn(this) + runCurrent() val captor = argumentCaptor() verify(keyguardUpdateMonitor).registerCallback(captor.capture()) @@ -603,6 +660,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() { ) .onEach { biometricSourceType -> captor.value.onBiometricAuthenticated(0, biometricSourceType, false) + runCurrent() } assertThat(values)