From a181d6658c73ef7d34b135000c98db2357ad1ade Mon Sep 17 00:00:00 2001 From: Josh Tsuji Date: Tue, 3 Jan 2023 16:02:09 -0500 Subject: [PATCH] Don't always showSurfaceBehindKeyguard if keyguard exit is cancelled. The exit animation can be cancelled for several reasons: (1) Timeout during swipe to unlock: we should end up unlocked (2) App transition begins during unlock: we should end up unlocked (3) Power button pressed at very specific time during unlock, cancelling the animation: we should end up *locked*, since we were cancelled due to a power button press intending to lock the device. ag/14934054, back in 2021, modified this logic to fix issues with (1) and (2) above, ensuring we'd end up unlocked if the animation was cancelled. However, it caused issues with (3). This timing was nearly impossible to trigger at the time, however new devices with in-button fingerprint readers make it far easier to simultaneously trigger power off and fingerprint unlock at exactly the same time. We can fix this by checking for a pending lock, and only ensuring we end up unlocked if there is *not* a pending lock. Otherwise, we should just end the surface animation but not re-show the keyguard. Bug: 260094933 Fixes: 232002936 Fixes: 250946719 Test: on lockscreen, mash the power button with an enrolled finger Test: swipe to unlock and hold at 50%, wait 10 seconds for cancellation Test: atest KeyguardViewMediatorTest Change-Id: Ib54e8781562ceda2d0c66a60b4a5d0e325f3f866 --- .../KeyguardUnlockAnimationController.kt | 9 ++- .../keyguard/KeyguardViewMediator.java | 57 ++++++++------ .../KeyguardUnlockAnimationControllerTest.kt | 6 +- .../keyguard/KeyguardViewMediatorTest.java | 74 ++++++++++++++++++- 4 files changed, 117 insertions(+), 29 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt index 53070a0ab8a7a..358c8bc887dd1 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt @@ -344,7 +344,7 @@ class KeyguardUnlockAnimationController @Inject constructor( override fun onAnimationEnd(animation: Animator) { Log.d(TAG, "surfaceBehindEntryAnimator#onAnimationEnd") playingCannedUnlockAnimation = false - keyguardViewMediator.get().onKeyguardExitRemoteAnimationFinished( + keyguardViewMediator.get().exitKeyguardAndFinishSurfaceBehindRemoteAnimation( false /* cancelled */ ) } @@ -579,7 +579,7 @@ class KeyguardUnlockAnimationController @Inject constructor( biometricUnlockControllerLazy.get().isWakeAndUnlock -> { Log.d(TAG, "playCannedUnlockAnimation, isWakeAndUnlock") setSurfaceBehindAppearAmount(1f) - keyguardViewMediator.get().onKeyguardExitRemoteAnimationFinished( + keyguardViewMediator.get().exitKeyguardAndFinishSurfaceBehindRemoteAnimation( false /* cancelled */) } @@ -627,7 +627,7 @@ class KeyguardUnlockAnimationController @Inject constructor( return@postDelayed } - keyguardViewMediator.get().onKeyguardExitRemoteAnimationFinished( + keyguardViewMediator.get().exitKeyguardAndFinishSurfaceBehindRemoteAnimation( false /* cancelled */) }, CANNED_UNLOCK_START_DELAY) } @@ -745,7 +745,8 @@ class KeyguardUnlockAnimationController @Inject constructor( !keyguardStateController.isFlingingToDismissKeyguardDuringSwipeGesture && dismissAmount >= DISMISS_AMOUNT_EXIT_KEYGUARD_THRESHOLD)) { setSurfaceBehindAppearAmount(1f) - keyguardViewMediator.get().onKeyguardExitRemoteAnimationFinished(false /* cancelled */) + keyguardViewMediator.get().exitKeyguardAndFinishSurfaceBehindRemoteAnimation( + false /* cancelled */) } } diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index 8aada1f12b0c3..bb2141d3fa166 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -144,12 +144,12 @@ import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.systemui.statusbar.policy.UserSwitcherController; import com.android.systemui.util.DeviceConfigProxy; -import dagger.Lazy; - import java.io.PrintWriter; import java.util.ArrayList; import java.util.concurrent.Executor; +import dagger.Lazy; + /** * Mediates requests related to the keyguard. This includes queries about the * state of the keyguard, power management events that effect whether the keyguard @@ -2725,27 +2725,42 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } /** - * Called if the keyguard exit animation has been cancelled, and we should dismiss to the - * keyguard. + * Called if the keyguard exit animation has been cancelled. * * This can happen due to the system cancelling the RemoteAnimation (due to a timeout, a new - * app transition before finishing the current RemoteAnimation). + * app transition before finishing the current RemoteAnimation, or the keyguard being re-shown). */ private void handleCancelKeyguardExitAnimation() { - showSurfaceBehindKeyguard(); - onKeyguardExitRemoteAnimationFinished(true /* cancelled */); + if (mPendingLock) { + Log.d(TAG, "#handleCancelKeyguardExitAnimation: keyguard exit animation cancelled. " + + "There's a pending lock, so we were cancelled because the device was locked " + + "again during the unlock sequence. We should end up locked."); + + // A lock is pending, meaning the keyguard exit animation was cancelled because we're + // re-locking. We should just end the surface-behind animation without exiting the + // keyguard. The pending lock will be handled by onFinishedGoingToSleep(). + finishSurfaceBehindRemoteAnimation(true); + } else { + Log.d(TAG, "#handleCancelKeyguardExitAnimation: keyguard exit animation cancelled. " + + "No pending lock, we should end up unlocked with the app/launcher visible."); + + // No lock is pending, so the animation was cancelled during the unlock sequence, but + // we should end up unlocked. Show the surface and exit the keyguard. + showSurfaceBehindKeyguard(); + exitKeyguardAndFinishSurfaceBehindRemoteAnimation(true /* cancelled */); + } } /** - * Called when we're done running the keyguard exit animation. + * Called when we're done running the keyguard exit animation, we should now end up unlocked. * - * This will call {@link #mSurfaceBehindRemoteAnimationFinishedCallback} to let WM know that - * we're done with the RemoteAnimation, actually hide the keyguard, and clean up state related - * to the keyguard exit animation. + * This will call {@link #handleCancelKeyguardExitAnimation()} to let WM know that we're done + * with the RemoteAnimation, actually hide the keyguard, and clean up state related to the + * keyguard exit animation. * * @param cancelled {@code true} if the animation was cancelled before it finishes. */ - public void onKeyguardExitRemoteAnimationFinished(boolean cancelled) { + public void exitKeyguardAndFinishSurfaceBehindRemoteAnimation(boolean cancelled) { Log.d(TAG, "onKeyguardExitRemoteAnimationFinished"); if (!mSurfaceBehindRemoteAnimationRunning && !mSurfaceBehindRemoteAnimationRequested) { Log.d(TAG, "skip onKeyguardExitRemoteAnimationFinished cancelled=" + cancelled @@ -2774,10 +2789,6 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } finishSurfaceBehindRemoteAnimation(cancelled); - mSurfaceBehindRemoteAnimationRequested = false; - - // The remote animation is over, so we're not going away anymore. - mKeyguardStateController.notifyKeyguardGoingAway(false); // Dispatch the callback on animation finishes. mUpdateMonitor.dispatchKeyguardDismissAnimationFinished(); @@ -2836,13 +2847,17 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, return mSurfaceBehindRemoteAnimationRunning; } - /** If it's running, finishes the RemoteAnimation on the surface behind the keyguard. */ + /** + * If it's running, finishes the RemoteAnimation on the surface behind the keyguard and resets + * related state. + * + * This does not set keyguard state to either locked or unlocked, it simply ends the remote + * animation on the surface behind the keyguard. This can be called by + */ void finishSurfaceBehindRemoteAnimation(boolean cancelled) { - if (!mSurfaceBehindRemoteAnimationRunning) { - return; - } - + mSurfaceBehindRemoteAnimationRequested = false; mSurfaceBehindRemoteAnimationRunning = false; + mKeyguardStateController.notifyKeyguardGoingAway(false); if (mSurfaceBehindRemoteAnimationFinishedCallback != null) { try { diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardUnlockAnimationControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardUnlockAnimationControllerTest.kt index a78c902a1f305..837481c0472a5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardUnlockAnimationControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardUnlockAnimationControllerTest.kt @@ -124,7 +124,7 @@ class KeyguardUnlockAnimationControllerTest : SysuiTestCase() { // Also expect we've immediately asked the keyguard view mediator to finish the remote // animation. - verify(keyguardViewMediator, times(1)).onKeyguardExitRemoteAnimationFinished( + verify(keyguardViewMediator, times(1)).exitKeyguardAndFinishSurfaceBehindRemoteAnimation( false /* cancelled */) verifyNoMoreInteractions(surfaceTransactionApplier) @@ -144,7 +144,7 @@ class KeyguardUnlockAnimationControllerTest : SysuiTestCase() { ) // Since the animation is running, we should not have finished the remote animation. - verify(keyguardViewMediator, times(0)).onKeyguardExitRemoteAnimationFinished( + verify(keyguardViewMediator, times(0)).exitKeyguardAndFinishSurfaceBehindRemoteAnimation( false /* cancelled */) } @@ -272,7 +272,7 @@ class KeyguardUnlockAnimationControllerTest : SysuiTestCase() { assertTrue(remainingTargets.isEmpty()) // Since the animation is running, we should not have finished the remote animation. - verify(keyguardViewMediator, times(0)).onKeyguardExitRemoteAnimationFinished( + verify(keyguardViewMediator, times(0)).exitKeyguardAndFinishSurfaceBehindRemoteAnimation( false /* cancelled */) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java index 804960dc3b181..dc258e781c0b1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java @@ -57,6 +57,7 @@ import com.android.keyguard.KeyguardDisplayManager; import com.android.keyguard.KeyguardSecurityView; import com.android.keyguard.KeyguardUpdateMonitor; import com.android.keyguard.mediator.ScreenOnCoordinator; +import com.android.systemui.DejankUtils; import com.android.systemui.SysuiTestCase; import com.android.systemui.animation.ActivityLaunchAnimator; import com.android.systemui.biometrics.AuthController; @@ -157,6 +158,8 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mColorExtractor, mDumpManager, mKeyguardStateController, mScreenOffAnimationController, mAuthController, mShadeExpansionStateManager); + DejankUtils.setImmediate(true); + createAndStartViewMediator(); } @@ -352,9 +355,69 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { verify(mCentralSurfaces).updateIsKeyguard(); } + @Test + @TestableLooper.RunWithLooper(setAsMainLooper = true) + public void testCancelKeyguardExitAnimation_noPendingLock_keyguardWillNotBeShowing() { + startMockKeyguardExitAnimation(); + cancelMockKeyguardExitAnimation(); + + // There should not be a pending lock, but try to handle it anyway to ensure one isn't set. + mViewMediator.maybeHandlePendingLock(); + TestableLooper.get(this).processAllMessages(); + + assertFalse(mViewMediator.isShowingAndNotOccluded()); + } + + @Test + @TestableLooper.RunWithLooper(setAsMainLooper = true) + public void testCancelKeyguardExitAnimationDueToSleep_withPendingLock_keyguardWillBeShowing() { + startMockKeyguardExitAnimation(); + + mViewMediator.onStartedGoingToSleep(PowerManager.GO_TO_SLEEP_REASON_POWER_BUTTON); + mViewMediator.onFinishedGoingToSleep(PowerManager.GO_TO_SLEEP_REASON_POWER_BUTTON, false); + + cancelMockKeyguardExitAnimation(); + + mViewMediator.maybeHandlePendingLock(); + TestableLooper.get(this).processAllMessages(); + + assertTrue(mViewMediator.isShowingAndNotOccluded()); + } + + @Test + @TestableLooper.RunWithLooper(setAsMainLooper = true) + public void testCancelKeyguardExitAnimationThenSleep_withPendingLock_keyguardWillBeShowing() { + startMockKeyguardExitAnimation(); + cancelMockKeyguardExitAnimation(); + + mViewMediator.maybeHandlePendingLock(); + TestableLooper.get(this).processAllMessages(); + + mViewMediator.onStartedGoingToSleep(PowerManager.GO_TO_SLEEP_REASON_POWER_BUTTON); + mViewMediator.onFinishedGoingToSleep(PowerManager.GO_TO_SLEEP_REASON_POWER_BUTTON, false); + + mViewMediator.maybeHandlePendingLock(); + TestableLooper.get(this).processAllMessages(); + + assertTrue(mViewMediator.isShowingAndNotOccluded()); + } + @Test @TestableLooper.RunWithLooper(setAsMainLooper = true) public void testStartKeyguardExitAnimation_expectSurfaceBehindRemoteAnimation() { + startMockKeyguardExitAnimation(); + assertTrue(mViewMediator.isAnimatingBetweenKeyguardAndSurfaceBehind()); + } + + /** + * Configures mocks appropriately, then starts the keyguard exit animation. + */ + private void startMockKeyguardExitAnimation() { + mViewMediator.onSystemReady(); + TestableLooper.get(this).processAllMessages(); + + mViewMediator.setShowingLocked(true); + RemoteAnimationTarget[] apps = new RemoteAnimationTarget[]{ mock(RemoteAnimationTarget.class) }; @@ -363,10 +426,19 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { }; IRemoteAnimationFinishedCallback callback = mock(IRemoteAnimationFinishedCallback.class); + when(mKeyguardStateController.isKeyguardGoingAway()).thenReturn(true); mViewMediator.startKeyguardExitAnimation(TRANSIT_OLD_KEYGUARD_GOING_AWAY, apps, wallpapers, null, callback); TestableLooper.get(this).processAllMessages(); - assertTrue(mViewMediator.isAnimatingBetweenKeyguardAndSurfaceBehind()); + } + + /** + * Configures mocks appropriately, then cancels the keyguard exit animation. + */ + private void cancelMockKeyguardExitAnimation() { + when(mKeyguardStateController.isKeyguardGoingAway()).thenReturn(false); + mViewMediator.cancelKeyguardExitAnimation(); + TestableLooper.get(this).processAllMessages(); } private void createAndStartViewMediator() {