From dcdd876c006678cc0fc491afb93499667994befe Mon Sep 17 00:00:00 2001 From: Matt Pietal Date: Wed, 9 Aug 2023 12:03:23 +0000 Subject: Fix stuck unlock animation state When using face unlock without bypass, the user can swipe up to dimiss the lock screen which will trigger the keyguard going away remote animation. However, if this remote animation is canceled, the 'playingCannedAnimation' state remains true essentially forever. When combined with logic in NotificationShadeWindowViewController that cancels all touches during this animation, the device is left in a unrecoverable state as sysui cannot be interacted with. Rename the cancelled parameter to showKeyguard, to make it clear what the end state will be. Fixes: 288505944 Test: atest KeyguardViewMediatorTest KeyguardUnlockAnimationControllerTest Test: manual - Use face unlock without bypass, then swipe up and hit power at the same time (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:42b8b340fef08e7d0a0a4e69adc25c15456d92c8) Merged-In: I762853d930f02ab0ee7f8cf0216b5aa6d01cf38c Change-Id: I762853d930f02ab0ee7f8cf0216b5aa6d01cf38c --- .../keyguard/KeyguardUnlockAnimationController.kt | 44 ++++++++++++---------- .../systemui/keyguard/KeyguardViewMediator.java | 24 ++++++------ .../NotificationShadeWindowViewController.java | 1 + .../keyguard/KeyguardViewMediatorTest.java | 12 +++++- 4 files changed, 50 insertions(+), 31 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt index 489d2ab4d342..9a09df4828d3 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardUnlockAnimationController.kt @@ -929,28 +929,41 @@ class KeyguardUnlockAnimationController @Inject constructor( /** * Called by [KeyguardViewMediator] to let us know that the remote animation has finished, and - * we should clean up all of our state. + * we should clean up all of our state. [showKeyguard] will tell us which surface should be + * visible after the animation has been completed or canceled. * * This is generally triggered by us, calling * [KeyguardViewMediator.finishSurfaceBehindRemoteAnimation]. */ - fun notifyFinishedKeyguardExitAnimation(cancelled: Boolean) { + fun notifyFinishedKeyguardExitAnimation(showKeyguard: Boolean) { // Cancel any pending actions. handler.removeCallbacksAndMessages(null) - // Make sure we made the surface behind fully visible, just in case. It should already be - // fully visible. The exit animation is finished, and we should not hold the leash anymore, - // so forcing it to 1f. - surfaceBehindAlpha = 1f - setSurfaceBehindAppearAmount(1f) + // The lockscreen surface is gone, so it is now safe to re-show the smartspace. + if (lockscreenSmartspace?.visibility == View.INVISIBLE) { + lockscreenSmartspace?.visibility = View.VISIBLE + } + + if (!showKeyguard) { + // Make sure we made the surface behind fully visible, just in case. It should already be + // fully visible. The exit animation is finished, and we should not hold the leash anymore, + // so forcing it to 1f. + surfaceBehindAlpha = 1f + setSurfaceBehindAppearAmount(1f) + + try { + launcherUnlockController?.setUnlockAmount(1f, false /* forceIfAnimating */) + } catch (e: RemoteException) { + Log.e(TAG, "Remote exception in notifyFinishedKeyguardExitAnimation", e) + } + } + + listeners.forEach { it.onUnlockAnimationFinished() } + + // Reset all state surfaceBehindAlphaAnimator.cancel() surfaceBehindEntryAnimator.cancel() wallpaperCannedUnlockAnimator.cancel() - try { - launcherUnlockController?.setUnlockAmount(1f, false /* forceIfAnimating */) - } catch (e: RemoteException) { - Log.e(TAG, "Remote exception in notifyFinishedKeyguardExitAnimation", e) - } // That target is no longer valid since the animation finished, null it out. surfaceBehindRemoteAnimationTargets = null @@ -960,13 +973,6 @@ class KeyguardUnlockAnimationController @Inject constructor( dismissAmountThresholdsReached = false willUnlockWithInWindowLauncherAnimations = false willUnlockWithSmartspaceTransition = false - - // The lockscreen surface is gone, so it is now safe to re-show the smartspace. - if (lockscreenSmartspace?.visibility == View.INVISIBLE) { - lockscreenSmartspace?.visibility = View.VISIBLE - } - - listeners.forEach { it.onUnlockAnimationFinished() } } /** diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index 66de371ab6e0..f861d5e94540 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -2561,7 +2561,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } else if (mSurfaceBehindRemoteAnimationRunning) { // We're already running the keyguard exit animation, likely due to an in-progress swipe // to unlock. - exitKeyguardAndFinishSurfaceBehindRemoteAnimation(false /* cancelled */); + exitKeyguardAndFinishSurfaceBehindRemoteAnimation(false /* showKeyguard */); } else if (!mHideAnimationRun) { if (DEBUG) Log.d(TAG, "tryKeyguardDone: starting pre-hide animation"); mHideAnimationRun = true; @@ -3003,7 +3003,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, mContext.getMainExecutor().execute(() -> { if (finishedCallback == null) { mKeyguardUnlockAnimationControllerLazy.get() - .notifyFinishedKeyguardExitAnimation(false /* cancelled */); + .notifyFinishedKeyguardExitAnimation(false /* showKeyguard */); mInteractionJankMonitor.end(CUJ_LOCKSCREEN_UNLOCK_ANIMATION); return; } @@ -3120,7 +3120,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, // 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); + finishSurfaceBehindRemoteAnimation(true /* showKeyguard */); maybeHandlePendingLock(); } else { Log.d(TAG, "#handleCancelKeyguardExitAnimation: keyguard exit animation cancelled. " @@ -3129,7 +3129,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, // 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 */); + exitKeyguardAndFinishSurfaceBehindRemoteAnimation(false /* showKeyguard */); } } @@ -3140,12 +3140,13 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, * 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. + * @param showKeyguard {@code true} if the animation was cancelled and keyguard should remain + * visible */ - public void exitKeyguardAndFinishSurfaceBehindRemoteAnimation(boolean cancelled) { + public void exitKeyguardAndFinishSurfaceBehindRemoteAnimation(boolean showKeyguard) { Log.d(TAG, "onKeyguardExitRemoteAnimationFinished"); if (!mSurfaceBehindRemoteAnimationRunning && !mSurfaceBehindRemoteAnimationRequested) { - Log.d(TAG, "skip onKeyguardExitRemoteAnimationFinished cancelled=" + cancelled + Log.d(TAG, "skip onKeyguardExitRemoteAnimationFinished showKeyguard=" + showKeyguard + " surfaceAnimationRunning=" + mSurfaceBehindRemoteAnimationRunning + " surfaceAnimationRequested=" + mSurfaceBehindRemoteAnimationRequested); return; @@ -3170,9 +3171,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, + " wasShowing=" + wasShowing); } - mKeyguardUnlockAnimationControllerLazy.get() - .notifyFinishedKeyguardExitAnimation(cancelled); - finishSurfaceBehindRemoteAnimation(cancelled); + finishSurfaceBehindRemoteAnimation(showKeyguard); // Dispatch the callback on animation finishes. mUpdateMonitor.dispatchKeyguardDismissAnimationFinished(); @@ -3236,7 +3235,10 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, * 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) { + void finishSurfaceBehindRemoteAnimation(boolean showKeyguard) { + mKeyguardUnlockAnimationControllerLazy.get() + .notifyFinishedKeyguardExitAnimation(showKeyguard); + mSurfaceBehindRemoteAnimationRequested = false; mSurfaceBehindRemoteAnimationRunning = false; mKeyguardStateController.notifyKeyguardGoingAway(false); diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java b/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java index 18e964462189..5b5785e36ccc 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java @@ -508,6 +508,7 @@ public class NotificationShadeWindowViewController { MotionEvent.ACTION_CANCEL, 0.0f, 0.0f, 0); event.setSource(InputDevice.SOURCE_TOUCHSCREEN); } + Log.w(TAG, "Canceling current touch event (should be very rare)"); mView.dispatchTouchEvent(event); event.recycle(); mTouchCancelled = true; 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 28328c2e6fae..847d58b8cd83 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java @@ -634,6 +634,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { TestableLooper.get(this).processAllMessages(); assertFalse(mViewMediator.isShowingAndNotOccluded()); + verify(mKeyguardUnlockAnimationController).notifyFinishedKeyguardExitAnimation(false); } @Test @@ -650,6 +651,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { TestableLooper.get(this).processAllMessages(); assertTrue(mViewMediator.isShowingAndNotOccluded()); + verify(mKeyguardUnlockAnimationController).notifyFinishedKeyguardExitAnimation(true); } @Test @@ -658,6 +660,9 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { startMockKeyguardExitAnimation(); cancelMockKeyguardExitAnimation(); + // Calling cancel above results in keyguard not visible, as there is no pending lock + verify(mKeyguardUnlockAnimationController).notifyFinishedKeyguardExitAnimation(false); + mViewMediator.maybeHandlePendingLock(); TestableLooper.get(this).processAllMessages(); @@ -672,10 +677,15 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { @Test @TestableLooper.RunWithLooper(setAsMainLooper = true) - public void testStartKeyguardExitAnimation_expectSurfaceBehindRemoteAnimation() { + public void testStartKeyguardExitAnimation_expectSurfaceBehindRemoteAnimationAndExits() { startMockKeyguardExitAnimation(); assertTrue(mViewMediator.isAnimatingBetweenKeyguardAndSurfaceBehind()); + mViewMediator.mViewMediatorCallback.keyguardDonePending(true, + mUpdateMonitor.getCurrentUser()); + mViewMediator.mViewMediatorCallback.readyForKeyguardDone(); + TestableLooper.get(this).processAllMessages(); + verify(mKeyguardUnlockAnimationController).notifyFinishedKeyguardExitAnimation(false); } @Test -- cgit v1.2.3-59-g8ed1b