From 807bad8e737ab757c6228afebc4915276be4488c Mon Sep 17 00:00:00 2001 From: Matt Pietal Date: Fri, 9 Aug 2024 11:34:50 +0000 Subject: Fix race condition with remote animation cancel/finish When the animation is canceled, a call to finish is also sent for clean up purposes. If the user unlocks with an activity animation and then hits the power button immediately, make sure the keyguard state gets correctly set to shown. Also, add more logging strings about why they WM keyguard state is being updated. Also, remove the errant call to show bouncer, which could pop up in this scenario. Fixes: 343327511 Test: manual - race condition when hitting power after using UDFPS to unlock after tapping notification Flag: com.android.systemui.relock_with_power_button_immediately Change-Id: Iad62842ab1045d3c600e0bc97d3139ce431eefc6 --- packages/SystemUI/aconfig/systemui.aconfig | 10 ++++ .../systemui/keyguard/KeyguardViewMediator.java | 60 +++++++++++++--------- .../statusbar/phone/CentralSurfacesImpl.java | 13 +++-- .../keyguard/KeyguardViewMediatorTest.java | 28 +++++----- 4 files changed, 67 insertions(+), 44 deletions(-) diff --git a/packages/SystemUI/aconfig/systemui.aconfig b/packages/SystemUI/aconfig/systemui.aconfig index 9046d4e328f4..2d09c98a158a 100644 --- a/packages/SystemUI/aconfig/systemui.aconfig +++ b/packages/SystemUI/aconfig/systemui.aconfig @@ -1236,6 +1236,16 @@ flag { } } +flag { + name: "relock_with_power_button_immediately" + namespace: "systemui" + description: "UDFPS unlock followed by immediate power button push should relock" + bug: "343327511" + metadata { + purpose: PURPOSE_BUGFIX + } +} + flag { name: "lockscreen_preview_renderer_create_on_main_thread" namespace: "systemui" diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index 3f9c98d6a5d4..d80c398dad69 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -42,6 +42,7 @@ import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STR import static com.android.systemui.DejankUtils.whitelistIpcs; import static com.android.systemui.Flags.notifyPowerManagerUserActivityBackground; import static com.android.systemui.Flags.refactorGetCurrentUser; +import static com.android.systemui.Flags.relockWithPowerButtonImmediately; import static com.android.systemui.Flags.translucentOccludingActivityFix; import static com.android.systemui.keyguard.ui.viewmodel.LockscreenToDreamingTransitionViewModel.DREAMING_ANIMATION_DURATION_MS; @@ -477,6 +478,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, private boolean mUnlockingAndWakingFromDream = false; private boolean mHideAnimationRun = false; private boolean mHideAnimationRunning = false; + private boolean mIsKeyguardExitAnimationCanceled = false; private SoundPool mLockSounds; private int mLockSoundId; @@ -1588,10 +1590,11 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, setShowingLocked(!shouldWaitForProvisioning() && !mLockPatternUtils.isLockScreenDisabled( mSelectedUserInteractor.getSelectedUserId()), - true /* forceCallbacks */); + true /* forceCallbacks */, "setupLocked - keyguard service enabled"); } else { // The system's keyguard is disabled or missing. - setShowingLocked(false /* showing */, true /* forceCallbacks */); + setShowingLocked(false /* showing */, true /* forceCallbacks */, + "setupLocked - keyguard service disabled"); } mKeyguardTransitions.register( @@ -2827,9 +2830,10 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, playSound(mTrustedSoundId); } - private void updateActivityLockScreenState(boolean showing, boolean aodShowing) { + private void updateActivityLockScreenState(boolean showing, boolean aodShowing, String reason) { mUiBgExecutor.execute(() -> { - Log.d(TAG, "updateActivityLockScreenState(" + showing + ", " + aodShowing + ")"); + Log.d(TAG, "updateActivityLockScreenState(" + showing + ", " + aodShowing + ", " + + reason + ")"); if (KeyguardWmStateRefactor.isEnabled()) { // Handled in WmLockscreenVisibilityManager if flag is enabled. @@ -2889,7 +2893,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, // Force if we're showing in the middle of unlocking, to ensure we end up in the // correct state. - setShowingLocked(true, hidingOrGoingAway /* force */); + setShowingLocked(true, hidingOrGoingAway /* force */, "handleShowInner"); mHiding = false; if (!KeyguardWmStateRefactor.isEnabled()) { @@ -3061,15 +3065,14 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, mHiding = true; mKeyguardGoingAwayRunnable.run(); } else { - Log.d(TAG, "Hiding keyguard while occluded. Just hide the keyguard view and exit."); - if (!KeyguardWmStateRefactor.isEnabled()) { mKeyguardViewControllerLazy.get().hide( mSystemClock.uptimeMillis() + mHideAnimation.getStartOffset(), mHideAnimation.getDuration()); } - onKeyguardExitFinished(); + onKeyguardExitFinished("Hiding keyguard while occluded. Just hide the keyguard " + + "view and exit."); } // It's possible that the device was unlocked (via BOUNCER or Fingerprint) while @@ -3100,6 +3103,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, Log.d(TAG, "handleStartKeyguardExitAnimation startTime=" + startTime + " fadeoutDuration=" + fadeoutDuration); synchronized (KeyguardViewMediator.this) { + mIsKeyguardExitAnimationCanceled = false; // Tell ActivityManager that we canceled the keyguard animation if // handleStartKeyguardExitAnimation was called, but we're not hiding the keyguard, // unless we're animating the surface behind the keyguard and will be hiding the @@ -3119,7 +3123,8 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, Slog.w(TAG, "Failed to call onAnimationFinished", e); } } - setShowingLocked(mShowing, true /* force */); + setShowingLocked(mShowing, true /* force */, + "handleStartKeyguardExitAnimation - canceled"); return; } mHiding = false; @@ -3143,9 +3148,11 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, Slog.w(TAG, "Failed to call onAnimationFinished", e); } } - onKeyguardExitFinished(); - mKeyguardViewControllerLazy.get().hide(0 /* startTime */, - 0 /* fadeoutDuration */); + if (!mIsKeyguardExitAnimationCanceled) { + onKeyguardExitFinished("onRemoteAnimationFinished"); + mKeyguardViewControllerLazy.get().hide(0 /* startTime */, + 0 /* fadeoutDuration */); + } mInteractionJankMonitor.end(CUJ_LOCKSCREEN_UNLOCK_ANIMATION); } @@ -3282,12 +3289,12 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, anim.start(); }); - onKeyguardExitFinished(); + onKeyguardExitFinished("remote animation disabled"); } } } - private void onKeyguardExitFinished() { + private void onKeyguardExitFinished(String reason) { if (DEBUG) Log.d(TAG, "onKeyguardExitFinished()"); // only play "unlock" noises if not on a call (since the incall UI // disables the keyguard) @@ -3295,7 +3302,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, playSounds(false); } - setShowingLocked(false); + setShowingLocked(false, "onKeyguardExitFinished: " + reason); mWakeAndUnlocking = false; mDismissCallbackRegistry.notifyDismissSucceeded(); resetKeyguardDonePendingLocked(); @@ -3343,6 +3350,9 @@ 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(). + if (relockWithPowerButtonImmediately()) { + mIsKeyguardExitAnimationCanceled = true; + } finishSurfaceBehindRemoteAnimation(true /* showKeyguard */); maybeHandlePendingLock(); } else { @@ -3391,12 +3401,13 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, doKeyguardLocked(null); finishSurfaceBehindRemoteAnimation(true /* showKeyguard */); // Ensure WM is notified that we made a decision to show - setShowingLocked(true /* showing */, true /* force */); + setShowingLocked(true /* showing */, true /* force */, + "exitKeyguardAndFinishSurfaceBehindRemoteAnimation - relocked"); return; } - onKeyguardExitFinished(); + onKeyguardExitFinished("exitKeyguardAndFinishSurfaceBehindRemoteAnimation"); if (mKeyguardStateController.isDismissingFromSwipe() || wasShowing) { Log.d(TAG, "onKeyguardExitRemoteAnimationFinished" @@ -3453,7 +3464,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, mSurfaceBehindRemoteAnimationRequested = false; mKeyguardStateController.notifyKeyguardGoingAway(false); if (mShowing) { - setShowingLocked(true, true); + setShowingLocked(true, true, "hideSurfaceBehindKeyguard"); } } @@ -3799,7 +3810,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, // update lock screen state in ATMS here, otherwise ATMS tries to resume activities when // enabling doze state. if (mShowing || !mPendingLock || !mDozeParameters.canControlUnlockedScreenOff()) { - setShowingLocked(mShowing); + setShowingLocked(mShowing, "setDozing"); } } @@ -3809,7 +3820,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, // is 1f), then show the activity lock screen. if (mAnimatingScreenOff && mDozing && linear == 1f) { mAnimatingScreenOff = false; - setShowingLocked(mShowing, true); + setShowingLocked(mShowing, true, "onDozeAmountChanged"); } } @@ -3847,11 +3858,11 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } } - void setShowingLocked(boolean showing) { - setShowingLocked(showing, false /* forceCallbacks */); + void setShowingLocked(boolean showing, String reason) { + setShowingLocked(showing, false /* forceCallbacks */, reason); } - private void setShowingLocked(boolean showing, boolean forceCallbacks) { + private void setShowingLocked(boolean showing, boolean forceCallbacks, String reason) { final boolean aodShowing = mDozing && !mWakeAndUnlocking; final boolean notifyDefaultDisplayCallbacks = showing != mShowing || forceCallbacks; final boolean updateActivityLockScreenState = showing != mShowing @@ -3862,9 +3873,8 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, notifyDefaultDisplayCallbacks(showing); } if (updateActivityLockScreenState) { - updateActivityLockScreenState(showing, aodShowing); + updateActivityLockScreenState(showing, aodShowing, reason); } - } private void notifyDefaultDisplayCallbacks(boolean showing) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java index ada3c52b16ab..c4fbc37b2dd5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -30,6 +30,7 @@ import static com.android.systemui.Dependency.TIME_TICK_HANDLER_NAME; import static com.android.systemui.Flags.keyboardShortcutHelperRewrite; import static com.android.systemui.Flags.lightRevealMigration; import static com.android.systemui.Flags.newAodTransition; +import static com.android.systemui.Flags.relockWithPowerButtonImmediately; import static com.android.systemui.charging.WirelessChargingAnimation.UNKNOWN_BATTERY_LEVEL; import static com.android.systemui.flags.Flags.SHORTCUT_LIST_SEARCH_LAYOUT; import static com.android.systemui.statusbar.NotificationLockscreenUserManager.PERMISSION_SELF; @@ -2352,11 +2353,13 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { } else if (mState == StatusBarState.KEYGUARD && !mStatusBarKeyguardViewManager.primaryBouncerIsOrWillBeShowing() && mStatusBarKeyguardViewManager.isSecure()) { - Log.d(TAG, "showBouncerOrLockScreenIfKeyguard, showingBouncer"); - if (SceneContainerFlag.isEnabled()) { - mStatusBarKeyguardViewManager.showPrimaryBouncer(true /* scrimmed */); - } else { - mStatusBarKeyguardViewManager.showBouncer(true /* scrimmed */); + if (!relockWithPowerButtonImmediately()) { + Log.d(TAG, "showBouncerOrLockScreenIfKeyguard, showingBouncer"); + if (SceneContainerFlag.isEnabled()) { + mStatusBarKeyguardViewManager.showPrimaryBouncer(true /* scrimmed */); + } else { + mStatusBarKeyguardViewManager.showBouncer(true /* scrimmed */); + } } } } 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 e3a38a8d6763..37f1a3d73b0c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java @@ -443,7 +443,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mViewMediator.onSystemReady(); TestableLooper.get(this).processAllMessages(); - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); TestableLooper.get(this).processAllMessages(); mViewMediator.onStartedGoingToSleep(OFF_BECAUSE_OF_USER); @@ -463,7 +463,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mViewMediator.onSystemReady(); TestableLooper.get(this).processAllMessages(); - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); TestableLooper.get(this).processAllMessages(); mViewMediator.onStartedGoingToSleep(OFF_BECAUSE_OF_USER); @@ -570,7 +570,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { // When showing and provisioned mViewMediator.onSystemReady(); when(mUpdateMonitor.isDeviceProvisioned()).thenReturn(true); - mViewMediator.setShowingLocked(true); + mViewMediator.setShowingLocked(true, ""); // and a SIM becomes locked and requires a PIN mViewMediator.mUpdateCallback.onSimStateChanged( @@ -579,7 +579,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { TelephonyManager.SIM_STATE_PIN_REQUIRED); // and the keyguard goes away - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); when(mKeyguardStateController.isShowing()).thenReturn(false); mViewMediator.mUpdateCallback.onKeyguardVisibilityChanged(false); @@ -595,7 +595,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { // When showing and provisioned mViewMediator.onSystemReady(); when(mUpdateMonitor.isDeviceProvisioned()).thenReturn(true); - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); // and a SIM becomes locked and requires a PIN mViewMediator.mUpdateCallback.onSimStateChanged( @@ -604,7 +604,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { TelephonyManager.SIM_STATE_PIN_REQUIRED); // and the keyguard goes away - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); when(mKeyguardStateController.isShowing()).thenReturn(false); mViewMediator.mUpdateCallback.onKeyguardVisibilityChanged(false); @@ -843,7 +843,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mViewMediator.onSystemReady(); TestableLooper.get(this).processAllMessages(); - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); TestableLooper.get(this).processAllMessages(); mViewMediator.onStartedGoingToSleep(OFF_BECAUSE_OF_USER); @@ -863,7 +863,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mViewMediator.onSystemReady(); TestableLooper.get(this).processAllMessages(); - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); TestableLooper.get(this).processAllMessages(); mViewMediator.onStartedGoingToSleep(OFF_BECAUSE_OF_USER); @@ -978,7 +978,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { @Test @TestableLooper.RunWithLooper(setAsMainLooper = true) public void testDoKeyguardWhileInteractive_resets() { - mViewMediator.setShowingLocked(true); + mViewMediator.setShowingLocked(true, ""); when(mKeyguardStateController.isShowing()).thenReturn(true); TestableLooper.get(this).processAllMessages(); @@ -992,7 +992,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { @Test @TestableLooper.RunWithLooper(setAsMainLooper = true) public void testDoKeyguardWhileNotInteractive_showsInsteadOfResetting() { - mViewMediator.setShowingLocked(true); + mViewMediator.setShowingLocked(true, ""); when(mKeyguardStateController.isShowing()).thenReturn(true); TestableLooper.get(this).processAllMessages(); @@ -1051,7 +1051,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mViewMediator.onSystemReady(); processAllMessagesAndBgExecutorMessages(); - mViewMediator.setShowingLocked(true); + mViewMediator.setShowingLocked(true, ""); RemoteAnimationTarget[] apps = new RemoteAnimationTarget[]{ mock(RemoteAnimationTarget.class) @@ -1123,7 +1123,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { @Test @TestableLooper.RunWithLooper(setAsMainLooper = true) public void testNotStartingKeyguardWhenFlagIsDisabled() { - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); when(mKeyguardStateController.isShowing()).thenReturn(false); mViewMediator.onDreamingStarted(); @@ -1133,7 +1133,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { @Test @TestableLooper.RunWithLooper(setAsMainLooper = true) public void testStartingKeyguardWhenFlagIsEnabled() { - mViewMediator.setShowingLocked(true); + mViewMediator.setShowingLocked(true, ""); when(mKeyguardStateController.isShowing()).thenReturn(true); mViewMediator.onDreamingStarted(); @@ -1174,7 +1174,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { TestableLooper.get(this).processAllMessages(); // WHEN keyguard visibility becomes FALSE - mViewMediator.setShowingLocked(false); + mViewMediator.setShowingLocked(false, ""); keyguardUpdateMonitorCallback.onKeyguardVisibilityChanged(false); TestableLooper.get(this).processAllMessages(); -- cgit v1.2.3-59-g8ed1b