From 3e304dffc3b2f415854df9010efd2d2c5fd977fb Mon Sep 17 00:00:00 2001 From: Aaron Liu Date: Wed, 19 Oct 2022 11:03:51 -0700 Subject: [PATCH] [Bouncer] Fix flicker tests. Fixes an issue where dismissaction is set to null before it's called when bouncer is unlocked. Fixes an issue where we call updateState everytime expansion changes. This was causing an overstack flow issue. Also it's super not performant to call this everytime expansion changes. Bug: 240298500 Test: Passed flicker tests for cts test. Passed presubmit. Test: Tested showing the bouncer. Test: Tested opening notification from lockscreen. Change-Id: Iede30acdf11689c5da7abd2d5e5b4619aa10b34a --- .../systemui/keyguard/data/BouncerView.kt | 6 ++++++ .../repository/KeyguardBouncerRepository.kt | 7 ------- .../domain/interactor/BouncerInteractor.kt | 9 ++------ .../ui/binder/KeyguardBouncerViewBinder.kt | 21 +++++++++++-------- .../ui/viewmodel/KeyguardBouncerViewModel.kt | 4 ---- .../phone/StatusBarKeyguardViewManager.java | 1 - .../interactor/BouncerInteractorTest.kt | 13 ++++++------ 7 files changed, 26 insertions(+), 35 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/BouncerView.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/BouncerView.kt index 99ae85d7a5489..80c6130955c50 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/data/BouncerView.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/BouncerView.kt @@ -18,6 +18,7 @@ package com.android.systemui.keyguard.data import android.view.KeyEvent import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.plugins.ActivityStarter import java.lang.ref.WeakReference import javax.inject.Inject @@ -45,4 +46,9 @@ interface BouncerViewDelegate { fun dispatchBackKeyEventPreIme(): Boolean fun showNextSecurityScreenOrFinish(): Boolean fun resume() + fun setDismissAction( + onDismissAction: ActivityStarter.OnDismissAction?, + cancelAction: Runnable?, + ) + fun willDismissWithActions(): Boolean } diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardBouncerRepository.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardBouncerRepository.kt index 543389e0a7cd0..7acbc843659c8 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardBouncerRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/KeyguardBouncerRepository.kt @@ -21,7 +21,6 @@ import com.android.keyguard.KeyguardUpdateMonitor import com.android.keyguard.KeyguardUpdateMonitorCallback import com.android.keyguard.ViewMediatorCallback import com.android.systemui.dagger.SysUISingleton -import com.android.systemui.keyguard.shared.model.BouncerCallbackActionsModel import com.android.systemui.keyguard.shared.model.BouncerShowMessageModel import com.android.systemui.keyguard.shared.model.KeyguardBouncerModel import com.android.systemui.statusbar.phone.KeyguardBouncer.EXPANSION_HIDDEN @@ -54,8 +53,6 @@ constructor( val hide = _hide.asStateFlow() private val _startingToHide = MutableStateFlow(false) val startingToHide = _startingToHide.asStateFlow() - private val _onDismissAction = MutableStateFlow(null) - val onDismissAction = _onDismissAction.asStateFlow() private val _disappearAnimation = MutableStateFlow(null) val startingDisappearAnimation = _disappearAnimation.asStateFlow() private val _keyguardPosition = MutableStateFlow(0f) @@ -120,10 +117,6 @@ constructor( _startingToHide.value = startingToHide } - fun setOnDismissAction(bouncerCallbackActionsModel: BouncerCallbackActionsModel?) { - _onDismissAction.value = bouncerCallbackActionsModel - } - fun setStartDisappearAnimation(runnable: Runnable?) { _disappearAnimation.value = runnable } diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractor.kt b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractor.kt index 2af9318d92ec3..aa01b74225b52 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractor.kt @@ -30,7 +30,6 @@ import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.keyguard.DismissCallbackRegistry import com.android.systemui.keyguard.data.BouncerView import com.android.systemui.keyguard.data.repository.KeyguardBouncerRepository -import com.android.systemui.keyguard.shared.model.BouncerCallbackActionsModel import com.android.systemui.keyguard.shared.model.BouncerShowMessageModel import com.android.systemui.keyguard.shared.model.KeyguardBouncerModel import com.android.systemui.plugins.ActivityStarter @@ -94,8 +93,6 @@ constructor( val showMessage: Flow = repository.showMessage.filterNotNull() val startingDisappearAnimation: Flow = repository.startingDisappearAnimation.filterNotNull() - val onDismissAction: Flow = - repository.onDismissAction.filterNotNull() val resourceUpdateRequests: Flow = repository.resourceUpdateRequests.filter { it } val keyguardPosition: Flow = repository.keyguardPosition @@ -149,7 +146,6 @@ constructor( } keyguardStateController.notifyBouncerShowing(true) callbackInteractor.dispatchStartingToShow() - Trace.endSection() } @@ -168,7 +164,6 @@ constructor( keyguardStateController.notifyBouncerShowing(false /* showing */) cancelShowRunnable() repository.setShowingSoon(false) - repository.setOnDismissAction(null) repository.setVisible(false) repository.setHide(true) repository.setShow(null) @@ -227,7 +222,7 @@ constructor( onDismissAction: ActivityStarter.OnDismissAction?, cancelAction: Runnable? ) { - repository.setOnDismissAction(BouncerCallbackActionsModel(onDismissAction, cancelAction)) + bouncerView.delegate?.setDismissAction(onDismissAction, cancelAction) } /** Update the resources of the views. */ @@ -305,7 +300,7 @@ constructor( /** Return whether bouncer will dismiss with actions */ fun willDismissWithAction(): Boolean { - return repository.onDismissAction.value?.onDismissAction != null + return bouncerView.delegate?.willDismissWithActions() == true } /** Returns whether the bouncer should be full screen. */ diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBouncerViewBinder.kt b/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBouncerViewBinder.kt index df260148751cb..a22958b74bb94 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBouncerViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBouncerViewBinder.kt @@ -29,6 +29,7 @@ import com.android.keyguard.dagger.KeyguardBouncerComponent import com.android.systemui.keyguard.data.BouncerViewDelegate import com.android.systemui.keyguard.ui.viewmodel.KeyguardBouncerViewModel import com.android.systemui.lifecycle.repeatWhenAttached +import com.android.systemui.plugins.ActivityStarter import com.android.systemui.statusbar.phone.KeyguardBouncer.EXPANSION_VISIBLE import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.collect @@ -75,6 +76,17 @@ object KeyguardBouncerViewBinder { hostViewController.showPrimarySecurityScreen() hostViewController.onResume() } + + override fun setDismissAction( + onDismissAction: ActivityStarter.OnDismissAction?, + cancelAction: Runnable? + ) { + hostViewController.setOnDismissAction(onDismissAction, cancelAction) + } + + override fun willDismissWithActions(): Boolean { + return hostViewController.hasDismissActions() + } } view.repeatWhenAttached { repeatOnLifecycle(Lifecycle.State.STARTED) { @@ -121,15 +133,6 @@ object KeyguardBouncerViewBinder { viewModel.startingToHide.collect { hostViewController.onStartingToHide() } } - launch { - viewModel.setDismissAction.collect { - hostViewController.setOnDismissAction( - it.onDismissAction, - it.cancelAction - ) - } - } - launch { viewModel.startDisappearAnimation.collect { hostViewController.startDisappearAnimation(it) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBouncerViewModel.kt b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBouncerViewModel.kt index 9ad52117bfc62..eeec3d0dc7909 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBouncerViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBouncerViewModel.kt @@ -20,7 +20,6 @@ import android.view.View import com.android.systemui.keyguard.data.BouncerView import com.android.systemui.keyguard.data.BouncerViewDelegate import com.android.systemui.keyguard.domain.interactor.BouncerInteractor -import com.android.systemui.keyguard.shared.model.BouncerCallbackActionsModel import com.android.systemui.keyguard.shared.model.BouncerShowMessageModel import com.android.systemui.keyguard.shared.model.KeyguardBouncerModel import com.android.systemui.statusbar.phone.KeyguardBouncer.EXPANSION_VISIBLE @@ -63,9 +62,6 @@ constructor( /** Observe whether bouncer is starting to hide. */ val startingToHide: Flow = interactor.startingToHide - /** Observe whether we want to set the dismiss action to the bouncer. */ - val setDismissAction: Flow = interactor.onDismissAction - /** Observe whether we want to start the disappear animation. */ val startDisappearAnimation: Flow = interactor.startingDisappearAnimation diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java index ccb5d8800ddb4..b101b138d61d3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java @@ -172,7 +172,6 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb if (mBouncerAnimating) { mCentralSurfaces.setBouncerHiddenFraction(expansion); } - updateStates(); } @Override diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractorTest.kt index e6c8dd87d9824..b5870f77b6ddb 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/domain/interactor/BouncerInteractorTest.kt @@ -27,8 +27,8 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.classifier.FalsingCollector import com.android.systemui.keyguard.DismissCallbackRegistry import com.android.systemui.keyguard.data.BouncerView +import com.android.systemui.keyguard.data.BouncerViewDelegate import com.android.systemui.keyguard.data.repository.KeyguardBouncerRepository -import com.android.systemui.keyguard.shared.model.BouncerCallbackActionsModel import com.android.systemui.keyguard.shared.model.BouncerShowMessageModel import com.android.systemui.keyguard.shared.model.KeyguardBouncerModel import com.android.systemui.plugins.ActivityStarter @@ -57,6 +57,7 @@ class BouncerInteractorTest : SysuiTestCase() { @Mock(answer = Answers.RETURNS_DEEP_STUBS) private lateinit var repository: KeyguardBouncerRepository @Mock(answer = Answers.RETURNS_DEEP_STUBS) private lateinit var bouncerView: BouncerView + @Mock private lateinit var bouncerViewDelegate: BouncerViewDelegate @Mock private lateinit var keyguardStateController: KeyguardStateController @Mock private lateinit var keyguardSecurityModel: KeyguardSecurityModel @Mock private lateinit var bouncerCallbackInteractor: BouncerCallbackInteractor @@ -86,6 +87,7 @@ class BouncerInteractorTest : SysuiTestCase() { ) `when`(repository.startingDisappearAnimation.value).thenReturn(null) `when`(repository.show.value).thenReturn(null) + `when`(bouncerView.delegate).thenReturn(bouncerViewDelegate) } @Test @@ -124,7 +126,6 @@ class BouncerInteractorTest : SysuiTestCase() { verify(falsingCollector).onBouncerHidden() verify(keyguardStateController).notifyBouncerShowing(false) verify(repository).setShowingSoon(false) - verify(repository).setOnDismissAction(null) verify(repository).setVisible(false) verify(repository).setHide(true) verify(repository).setShow(null) @@ -178,8 +179,7 @@ class BouncerInteractorTest : SysuiTestCase() { val onDismissAction = mock(ActivityStarter.OnDismissAction::class.java) val cancelAction = mock(Runnable::class.java) bouncerInteractor.setDismissAction(onDismissAction, cancelAction) - verify(repository) - .setOnDismissAction(BouncerCallbackActionsModel(onDismissAction, cancelAction)) + verify(bouncerViewDelegate).setDismissAction(onDismissAction, cancelAction) } @Test @@ -269,10 +269,9 @@ class BouncerInteractorTest : SysuiTestCase() { @Test fun testWillDismissWithAction() { - `when`(repository.onDismissAction.value?.onDismissAction) - .thenReturn(mock(ActivityStarter.OnDismissAction::class.java)) + `when`(bouncerViewDelegate.willDismissWithActions()).thenReturn(true) assertThat(bouncerInteractor.willDismissWithAction()).isTrue() - `when`(repository.onDismissAction.value?.onDismissAction).thenReturn(null) + `when`(bouncerViewDelegate.willDismissWithActions()).thenReturn(false) assertThat(bouncerInteractor.willDismissWithAction()).isFalse() } }