From 716dcea6bdfaf33aecf02e822e980db1a56bb36e Mon Sep 17 00:00:00 2001 From: Josh Tsuji Date: Wed, 19 Oct 2022 15:41:08 -0400 Subject: [PATCH] Delay keyguard hiding until after occlusion animation ends. The keyguard should remain visible during the occlusion animation since the occluding activity is launching over it. The hideKeyguard call is also extremely janky and causes the animation to be nearly imperceptible through the jank. See https://b.corp.google.com/issues/250542279#comment15 for more context. Fixes: 250542279 Test: atest KeyguardViewMediatorTest Test: atest CentralSurfacesImplTest Test: launch camera, notice far less flickering Change-Id: Ic3f7e62a42829481f6809d32ee1aa4690927ece8 --- .../keyguard/KeyguardViewMediator.java | 28 ++++++++- .../statusbar/phone/CentralSurfacesImpl.java | 5 +- .../keyguard/KeyguardViewMediatorTest.java | 25 ++++++++ .../phone/CentralSurfacesImplTest.java | 60 +++++++++++++++++-- 4 files changed, 111 insertions(+), 7 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index a3632705d865d..8db18c9dd12e2 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -91,6 +91,7 @@ import android.view.animation.AnimationUtils; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import com.android.internal.jank.InteractionJankMonitor; import com.android.internal.jank.InteractionJankMonitor.Configuration; @@ -322,6 +323,12 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, // true if the keyguard is hidden by another window private boolean mOccluded = false; + /** + * Whether the {@link #mOccludeAnimationController} is currently playing the occlusion + * animation. + */ + private boolean mOccludeAnimationPlaying = false; + private boolean mWakeAndUnlocking = false; /** @@ -831,15 +838,22 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, /** * Animation launch controller for activities that occlude the keyguard. */ - private final ActivityLaunchAnimator.Controller mOccludeAnimationController = + @VisibleForTesting + final ActivityLaunchAnimator.Controller mOccludeAnimationController = new ActivityLaunchAnimator.Controller() { @Override - public void onLaunchAnimationStart(boolean isExpandingFullyAbove) {} + public void onLaunchAnimationStart(boolean isExpandingFullyAbove) { + mOccludeAnimationPlaying = true; + } @Override public void onLaunchAnimationCancelled(@Nullable Boolean newKeyguardOccludedState) { Log.d(TAG, "Occlude launch animation cancelled. Occluded state is now: " + mOccluded); + mOccludeAnimationPlaying = false; + + // Ensure keyguard state is set correctly if we're cancelled. + mCentralSurfaces.updateIsKeyguard(); } @Override @@ -848,6 +862,12 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, mCentralSurfaces.instantCollapseNotificationPanel(); } + mOccludeAnimationPlaying = false; + + // Hide the keyguard now that we're done launching the occluding activity over + // it. + mCentralSurfaces.updateIsKeyguard(); + mInteractionJankMonitor.end(CUJ_LOCKSCREEN_OCCLUSION); } @@ -1760,6 +1780,10 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, return mShowing && !mOccluded; } + public boolean isOccludeAnimationPlaying() { + return mOccludeAnimationPlaying; + } + /** * Notify us when the keyguard is occluded by another window */ 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 604e1467f9507..d881c0a75194a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -2987,7 +2987,10 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { // * When phone is unlocked: we still don't want to execute hiding of the keyguard // as the animation could prepare 'fake AOD' interface (without actually // transitioning to keyguard state) and this might reset the view states - if (!mScreenOffAnimationController.isKeyguardHideDelayed()) { + if (!mScreenOffAnimationController.isKeyguardHideDelayed() + // If we're animating occluded, there's an activity launching over the keyguard + // UI. Wait to hide it until after the animation concludes. + && !mKeyguardViewMediator.isOccludeAnimationPlaying()) { return hideKeyguardImpl(forceStateChange); } } 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 4c986bffd1721..2c3ddd574b0fd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java @@ -60,6 +60,7 @@ import com.android.systemui.navigationbar.NavigationModeController; import com.android.systemui.statusbar.NotificationShadeDepthController; import com.android.systemui.statusbar.NotificationShadeWindowController; import com.android.systemui.statusbar.SysuiStatusBarStateController; +import com.android.systemui.statusbar.phone.CentralSurfaces; import com.android.systemui.statusbar.phone.DozeParameters; import com.android.systemui.statusbar.phone.ScreenOffAnimationController; import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager; @@ -112,6 +113,8 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { private FalsingCollectorFake mFalsingCollector; + private @Mock CentralSurfaces mCentralSurfaces; + @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -258,6 +261,26 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { verify(mKeyguardStateController).notifyKeyguardGoingAway(false); } + @Test + public void testUpdateIsKeyguardAfterOccludeAnimationEnds() { + mViewMediator.mOccludeAnimationController.onLaunchAnimationEnd( + false /* isExpandingFullyAbove */); + + // Since the updateIsKeyguard call is delayed during the animation, ensure it's called once + // it ends. + verify(mCentralSurfaces).updateIsKeyguard(); + } + + @Test + public void testUpdateIsKeyguardAfterOccludeAnimationIsCancelled() { + mViewMediator.mOccludeAnimationController.onLaunchAnimationCancelled( + null /* newKeyguardOccludedState */); + + // Since the updateIsKeyguard call is delayed during the animation, ensure it's called if + // it's cancelled. + verify(mCentralSurfaces).updateIsKeyguard(); + } + private void createAndStartViewMediator() { mViewMediator = new KeyguardViewMediator( mContext, @@ -287,5 +310,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mNotificationShadeWindowControllerLazy, () -> mActivityLaunchAnimator); mViewMediator.start(); + + mViewMediator.registerCentralSurfaces(mCentralSurfaces, null, null, null, null, null); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java index ad497a2ec1e12..920b342a9b10b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java @@ -233,6 +233,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { @Mock private NavigationBarController mNavigationBarController; @Mock private AccessibilityFloatingMenuController mAccessibilityFloatingMenuController; @Mock private SysuiColorExtractor mColorExtractor; + private WakefulnessLifecycle mWakefulnessLifecycle; @Mock private ColorExtractor.GradientColors mGradientColors; @Mock private PulseExpansionHandler mPulseExpansionHandler; @Mock private NotificationWakeUpCoordinator mNotificationWakeUpCoordinator; @@ -363,10 +364,10 @@ public class CentralSurfacesImplTest extends SysuiTestCase { return null; }).when(mStatusBarKeyguardViewManager).addAfterKeyguardGoneRunnable(any()); - WakefulnessLifecycle wakefulnessLifecycle = + mWakefulnessLifecycle = new WakefulnessLifecycle(mContext, mIWallpaperManager, mDumpManager); - wakefulnessLifecycle.dispatchStartedWakingUp(PowerManager.WAKE_REASON_UNKNOWN); - wakefulnessLifecycle.dispatchFinishedWakingUp(); + mWakefulnessLifecycle.dispatchStartedWakingUp(PowerManager.WAKE_REASON_UNKNOWN); + mWakefulnessLifecycle.dispatchFinishedWakingUp(); when(mGradientColors.supportsDarkText()).thenReturn(true); when(mColorExtractor.getNeutralColors()).thenReturn(mGradientColors); @@ -425,7 +426,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { mBatteryController, mColorExtractor, new ScreenLifecycle(mDumpManager), - wakefulnessLifecycle, + mWakefulnessLifecycle, mStatusBarStateController, Optional.of(mBubbles), mDeviceProvisionedController, @@ -504,6 +505,8 @@ public class CentralSurfacesImplTest extends SysuiTestCase { mCentralSurfaces.mKeyguardIndicationController = mKeyguardIndicationController; mCentralSurfaces.mBarService = mBarService; mCentralSurfaces.mStackScroller = mStackScroller; + mCentralSurfaces.mGestureWakeLock = mPowerManager.newWakeLock( + PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "sysui:GestureWakeLock"); mCentralSurfaces.startKeyguard(); mInitController.executePostInitTasks(); notificationLogger.setUpWithContainer(mNotificationListContainer); @@ -1068,6 +1071,55 @@ public class CentralSurfacesImplTest extends SysuiTestCase { assertThat(onDismissActionCaptor.getValue().onDismiss()).isFalse(); } + @Test + public void testKeyguardHideDelayedIfOcclusionAnimationRunning() { + // Show the keyguard and verify we've done so. + setKeyguardShowingAndOccluded(true /* showing */, false /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.KEYGUARD); + + // Request to hide the keyguard, but while the occlude animation is playing. We should delay + // this hide call, since we're playing the occlude animation over the keyguard and thus want + // it to remain visible. + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(true); + setKeyguardShowingAndOccluded(false /* showing */, true /* occluded */); + verify(mStatusBarStateController, never()).setState(StatusBarState.SHADE); + + // Once the animation ends, verify that the keyguard is actually hidden. + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(false); + setKeyguardShowingAndOccluded(false /* showing */, true /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.SHADE); + } + + @Test + public void testKeyguardHideNotDelayedIfOcclusionAnimationNotRunning() { + // Show the keyguard and verify we've done so. + setKeyguardShowingAndOccluded(true /* showing */, false /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.KEYGUARD); + + // Hide the keyguard while the occlusion animation is not running. Verify that we + // immediately hide the keyguard. + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(false); + setKeyguardShowingAndOccluded(false /* showing */, true /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.SHADE); + } + + /** + * Configures the appropriate mocks and then calls {@link CentralSurfacesImpl#updateIsKeyguard} + * to reconfigure the keyguard to reflect the requested showing/occluded states. + */ + private void setKeyguardShowingAndOccluded(boolean showing, boolean occluded) { + when(mStatusBarStateController.isKeyguardRequested()).thenReturn(showing); + when(mKeyguardStateController.isOccluded()).thenReturn(occluded); + + // If we want to show the keyguard, make sure that we think we're awake and not unlocking. + if (showing) { + when(mBiometricUnlockController.isWakeAndUnlock()).thenReturn(false); + mWakefulnessLifecycle.dispatchStartedWakingUp(PowerManager.WAKE_REASON_UNKNOWN); + } + + mCentralSurfaces.updateIsKeyguard(false /* forceStateChange */); + } + private void setDeviceState(int state) { ArgumentCaptor callbackCaptor = ArgumentCaptor.forClass(DeviceStateManager.DeviceStateCallback.class);