From 262cef4f10e3d551082351ab402391541caac89d Mon Sep 17 00:00:00 2001 From: Andreas Miko Date: Tue, 28 Mar 2023 00:23:09 +0200 Subject: [PATCH] Show bouncer after user switch Caution: There is different behavior based on if you switch to or from the owner. Also there are various race conditions in the KeyguardViewMediator queue caused by waiting on async layouts to happen. This CL tries to mitigate this issue by adding a delay, but more thorough investigation and cleanup of incoming signals and side effects is necessary to remove jank and make the queues order of execution predictable. Test: Manually tested various user switch scenarios with different bouncers. PIN/SWIPE/NONE/FINGER. Bug: b/266711330 Change-Id: I7c59ae07d43140cd55611d18d08e2f44cad9e562 --- .../KeyguardSecurityContainerController.java | 1 - .../keyguard/KeyguardViewMediator.java | 46 ++++++++++--------- .../phone/StatusBarKeyguardViewManager.java | 35 ++++++++++---- .../StatusBarKeyguardViewManagerTest.java | 30 ++++++++++-- 4 files changed, 76 insertions(+), 36 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java index 1a572b729a6e4..49dd3b3946525 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java @@ -524,7 +524,6 @@ public class KeyguardSecurityContainerController extends ViewController dismiss(null /* callback */, null /* message */), 500); } @Override @@ -2198,58 +2182,72 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, private Handler mHandler = new Handler(Looper.myLooper(), null, true /*async*/) { @Override public void handleMessage(Message msg) { + String message = ""; switch (msg.what) { case SHOW: + message = "SHOW"; handleShow((Bundle) msg.obj); break; case HIDE: + message = "HIDE"; handleHide(); break; case RESET: + message = "RESET"; handleReset(); break; case VERIFY_UNLOCK: + message = "VERIFY_UNLOCK"; Trace.beginSection("KeyguardViewMediator#handleMessage VERIFY_UNLOCK"); handleVerifyUnlock(); Trace.endSection(); break; case NOTIFY_STARTED_GOING_TO_SLEEP: + message = "NOTIFY_STARTED_GOING_TO_SLEEP"; handleNotifyStartedGoingToSleep(); break; case NOTIFY_FINISHED_GOING_TO_SLEEP: + message = "NOTIFY_FINISHED_GOING_TO_SLEEP"; handleNotifyFinishedGoingToSleep(); break; case NOTIFY_STARTED_WAKING_UP: + message = "NOTIFY_STARTED_WAKING_UP"; Trace.beginSection( "KeyguardViewMediator#handleMessage NOTIFY_STARTED_WAKING_UP"); handleNotifyStartedWakingUp(); Trace.endSection(); break; case KEYGUARD_DONE: + message = "KEYGUARD_DONE"; Trace.beginSection("KeyguardViewMediator#handleMessage KEYGUARD_DONE"); handleKeyguardDone(); Trace.endSection(); break; case KEYGUARD_DONE_DRAWING: + message = "KEYGUARD_DONE_DRAWING"; Trace.beginSection("KeyguardViewMediator#handleMessage KEYGUARD_DONE_DRAWING"); handleKeyguardDoneDrawing(); Trace.endSection(); break; case SET_OCCLUDED: + message = "SET_OCCLUDED"; Trace.beginSection("KeyguardViewMediator#handleMessage SET_OCCLUDED"); handleSetOccluded(msg.arg1 != 0, msg.arg2 != 0); Trace.endSection(); break; case KEYGUARD_TIMEOUT: + message = "KEYGUARD_TIMEOUT"; synchronized (KeyguardViewMediator.this) { doKeyguardLocked((Bundle) msg.obj); } break; case DISMISS: - final DismissMessage message = (DismissMessage) msg.obj; - handleDismiss(message.getCallback(), message.getMessage()); + message = "DISMISS"; + final DismissMessage dismissMsg = (DismissMessage) msg.obj; + handleDismiss(dismissMsg.getCallback(), dismissMsg.getMessage()); break; case START_KEYGUARD_EXIT_ANIM: + message = "START_KEYGUARD_EXIT_ANIM"; Trace.beginSection( "KeyguardViewMediator#handleMessage START_KEYGUARD_EXIT_ANIM"); synchronized (KeyguardViewMediator.this) { @@ -2267,21 +2265,25 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, Trace.endSection(); break; case CANCEL_KEYGUARD_EXIT_ANIM: + message = "CANCEL_KEYGUARD_EXIT_ANIM"; Trace.beginSection( "KeyguardViewMediator#handleMessage CANCEL_KEYGUARD_EXIT_ANIM"); handleCancelKeyguardExitAnimation(); Trace.endSection(); break; case KEYGUARD_DONE_PENDING_TIMEOUT: + message = "KEYGUARD_DONE_PENDING_TIMEOUT"; Trace.beginSection("KeyguardViewMediator#handleMessage" + " KEYGUARD_DONE_PENDING_TIMEOUT"); Log.w(TAG, "Timeout while waiting for activity drawn!"); Trace.endSection(); break; case SYSTEM_READY: + message = "SYSTEM_READY"; handleSystemReady(); break; } + Log.d(TAG, "KeyguardViewMediator queue processing message: " + message); } }; 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 f06b5db845885..9b6c6d20ab194 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java @@ -69,6 +69,7 @@ import com.android.systemui.navigationbar.NavigationBarView; import com.android.systemui.navigationbar.NavigationModeController; import com.android.systemui.navigationbar.TaskbarDelegate; import com.android.systemui.plugins.statusbar.StatusBarStateController; +import com.android.systemui.settings.UserTracker; import com.android.systemui.shade.NotificationPanelViewController; import com.android.systemui.shade.ShadeController; import com.android.systemui.shade.ShadeExpansionChangeEvent; @@ -286,8 +287,11 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb private final boolean mUdfpsNewTouchDetectionEnabled; private final UdfpsOverlayInteractor mUdfpsOverlayInteractor; - private OnDismissAction mAfterKeyguardGoneAction; - private Runnable mKeyguardGoneCancelAction; + @VisibleForTesting + OnDismissAction mAfterKeyguardGoneAction; + + @VisibleForTesting + Runnable mKeyguardGoneCancelAction; private boolean mDismissActionWillAnimateOnKeyguard; private final ArrayList mAfterKeyguardGoneRunnables = new ArrayList<>(); @@ -303,6 +307,8 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb @Nullable private KeyguardBypassController mBypassController; @Nullable private OccludingAppBiometricUI mOccludingAppBiometricUI; + private UserTracker mUserTracker; + @Nullable private TaskbarDelegate mTaskbarDelegate; private final KeyguardUpdateMonitorCallback mUpdateMonitorCallback = new KeyguardUpdateMonitorCallback() { @@ -340,7 +346,8 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb PrimaryBouncerInteractor primaryBouncerInteractor, BouncerView primaryBouncerView, AlternateBouncerInteractor alternateBouncerInteractor, - UdfpsOverlayInteractor udfpsOverlayInteractor + UdfpsOverlayInteractor udfpsOverlayInteractor, + UserTracker userTracker ) { mContext = context; mViewMediatorCallback = callback; @@ -369,6 +376,7 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb featureFlags.isEnabled(Flags.WM_ENABLE_PREDICTIVE_BACK_BOUNCER_ANIM); mUdfpsNewTouchDetectionEnabled = featureFlags.isEnabled(Flags.UDFPS_NEW_TOUCH_DETECTION); mUdfpsOverlayInteractor = udfpsOverlayInteractor; + mUserTracker = userTracker; } @Override @@ -693,14 +701,21 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb if (afterKeyguardGone) { // we'll handle the dismiss action after keyguard is gone, so just show the // bouncer - mPrimaryBouncerInteractor.show(/* isScrimmed= */true); + if (mLockPatternUtils.isSecure(mUserTracker.getUserId())) { + mPrimaryBouncerInteractor.show(true); + } } else { - // after authentication success, run dismiss action with the option to defer - // hiding the keyguard based on the return value of the OnDismissAction - mPrimaryBouncerInteractor.setDismissAction( - mAfterKeyguardGoneAction, mKeyguardGoneCancelAction); - mPrimaryBouncerInteractor.show(/* isScrimmed= */true); - // bouncer will handle the dismiss action, so we no longer need to track it here + if (mLockPatternUtils.isSecure(mUserTracker.getUserId())) { + // after authentication success, run dismiss action with the option to defer + // hiding the keyguard based on the return value of the OnDismissAction + mPrimaryBouncerInteractor.setDismissAction( + mAfterKeyguardGoneAction, mKeyguardGoneCancelAction); + mPrimaryBouncerInteractor.show(true); + } else { + if (mAfterKeyguardGoneAction != null) { + mAfterKeyguardGoneAction.onDismiss(); + } + } mAfterKeyguardGoneAction = null; mKeyguardGoneCancelAction = null; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManagerTest.java index 14aee4e13a8f1..baeb188cd4a5b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManagerTest.java @@ -20,6 +20,7 @@ import static com.android.systemui.keyguard.shared.constants.KeyguardBouncerCons import static com.android.systemui.keyguard.shared.constants.KeyguardBouncerConstants.EXPANSION_VISIBLE; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -73,6 +74,7 @@ import com.android.systemui.keyguard.domain.interactor.PrimaryBouncerInteractor; import com.android.systemui.navigationbar.NavigationModeController; import com.android.systemui.navigationbar.TaskbarDelegate; import com.android.systemui.plugins.ActivityStarter.OnDismissAction; +import com.android.systemui.settings.UserTracker; import com.android.systemui.shade.NotificationPanelViewController; import com.android.systemui.shade.NotificationShadeWindowView; import com.android.systemui.shade.ShadeController; @@ -192,7 +194,8 @@ public class StatusBarKeyguardViewManagerTest extends SysuiTestCase { mPrimaryBouncerInteractor, mBouncerView, mAlternateBouncerInteractor, - mUdfpsOverlayInteractor) { + mUdfpsOverlayInteractor, + mock(UserTracker.class)) { @Override public ViewRootImpl getViewRootImpl() { return mViewRootImpl; @@ -216,14 +219,34 @@ public class StatusBarKeyguardViewManagerTest extends SysuiTestCase { } @Test - public void dismissWithAction_AfterKeyguardGoneSetToFalse() { + public void dismissWithAction_AfterKeyguardGoneSetToFalse_DismissAction_Set() { OnDismissAction action = () -> false; Runnable cancelAction = () -> { }; + when(mLockPatternUtils.isSecure(anyInt())).thenReturn(true); + mStatusBarKeyguardViewManager.dismissWithAction( action, cancelAction, false /* afterKeyguardGone */); + verify(mPrimaryBouncerInteractor).setDismissAction(eq(action), eq(cancelAction)); verify(mPrimaryBouncerInteractor).show(eq(true)); + assertNull(mStatusBarKeyguardViewManager.mAfterKeyguardGoneAction); + assertNull(mStatusBarKeyguardViewManager.mKeyguardGoneCancelAction); + } + + @Test + public void dismissWithAction_AfterKeyguardGoneSetToFalse_DismissAction_Called() { + OnDismissAction action = mock(OnDismissAction.class); + Runnable cancelAction = () -> { + }; + when(mLockPatternUtils.isSecure(anyInt())).thenReturn(false); + + mStatusBarKeyguardViewManager.dismissWithAction( + action, cancelAction, false /* afterKeyguardGone */); + + verify(action).onDismiss(); + assertNull(mStatusBarKeyguardViewManager.mAfterKeyguardGoneAction); + assertNull(mStatusBarKeyguardViewManager.mKeyguardGoneCancelAction); } @Test @@ -680,7 +703,8 @@ public class StatusBarKeyguardViewManagerTest extends SysuiTestCase { mPrimaryBouncerInteractor, mBouncerView, mAlternateBouncerInteractor, - mUdfpsOverlayInteractor) { + mUdfpsOverlayInteractor, + mock(UserTracker.class)) { @Override public ViewRootImpl getViewRootImpl() { return mViewRootImpl;