From b1f7b0017b613a9711cd1e3da00ebe055f119cfa Mon Sep 17 00:00:00 2001 From: Beverly Date: Thu, 9 Mar 2023 21:48:47 +0000 Subject: [PATCH] RESTRICT AUTOMERGE Fix alternateBouncer => primaryBouncer flicker This involves a series of fixes: 1. Don't update the scrim state between hiding the alternate bouncer and showing the primary bouncer. This can cause the scrim state to transition to the UNLOCKED scrim between AUTH_SCRIMMED_SHADE and BOUNCER_SCRIMMED. 2. Handle the AlternateBouncer touch on action up. The AlternateBouncer intercepts all touch events when its visible. We don't want to prematurely stop intercepting the touch after the initial down event, or else the touch can be sent to its parent view (notification shade) while the alternate bouncer is going away which can cause flicker. Therefore, don't trigger the transition to the primary bouncer until action UP. Test: atest StatusBarKeyguardViewManagerTest Test: bring up the altnerate bouncer over the shade over an occluding activity, and then tap anywhere to bring up the primary bouncer. Observe there's no flicker. Bug: 272350664 Change-Id: I1c68ceb9bb77b0b664ca351af1a897b835e80367 --- .../keyguard/KeyguardViewController.java | 2 +- ...NotificationShadeWindowViewController.java | 4 +- .../statusbar/phone/CentralSurfacesImpl.java | 6 --- .../phone/StatusBarKeyguardViewManager.java | 22 +++++---- ...tificationShadeWindowViewControllerTest.kt | 14 ++++++ .../StatusBarKeyguardViewManagerTest.java | 46 +++++++++++++++++++ 6 files changed, 77 insertions(+), 17 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardViewController.java index 6c3c246e7fb9d..7661b8d0c1446 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardViewController.java @@ -168,7 +168,7 @@ public interface KeyguardViewController { /** * Stop showing the alternate bouncer, if showing. */ - void hideAlternateBouncer(boolean forceUpdateScrim); + void hideAlternateBouncer(boolean updateScrim); // TODO: Deprecate registerStatusBar in KeyguardViewController interface. It is currently // only used for testing purposes in StatusBarKeyguardViewManager, and it prevents us from diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java b/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java index c130b3913b640..e5dc97d705fa0 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationShadeWindowViewController.java @@ -240,7 +240,9 @@ public class NotificationShadeWindowViewController { mFalsingCollector.onTouchEvent(ev); mPulsingWakeupGestureHandler.onTouchEvent(ev); - mStatusBarKeyguardViewManager.onTouch(ev); + if (mStatusBarKeyguardViewManager.onTouch(ev)) { + return true; + } if (mBrightnessMirror != null && mBrightnessMirror.getVisibility() == View.VISIBLE) { // Disallow new pointers while the brightness mirror is visible. This is so that 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 dcd219ad94b97..5131772e34a03 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -605,7 +605,6 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { private Runnable mLaunchTransitionEndRunnable; private Runnable mLaunchTransitionCancelRunnable; - private boolean mLaunchingAffordance; private boolean mLaunchCameraWhenFinishedWaking; private boolean mLaunchCameraOnFinishedGoingToSleep; private boolean mLaunchEmergencyActionWhenFinishedWaking; @@ -3744,8 +3743,6 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { mScrimController.setExpansionAffectsAlpha(!unlocking); - boolean launchingAffordanceWithPreview = mLaunchingAffordance; - mScrimController.setLaunchingAffordanceWithPreview(launchingAffordanceWithPreview); if (mAlternateBouncerInteractor.isVisibleState()) { if ((!isOccluded() || isPanelExpanded()) && (mState == StatusBarState.SHADE || mState == StatusBarState.SHADE_LOCKED @@ -3764,9 +3761,6 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { ScrimState state = mStatusBarKeyguardViewManager.primaryBouncerNeedsScrimming() ? ScrimState.BOUNCER_SCRIMMED : ScrimState.BOUNCER; mScrimController.transitionTo(state); - } else if (launchingAffordanceWithPreview) { - // We want to avoid animating when launching with a preview. - mScrimController.transitionTo(ScrimState.UNLOCKED, mUnlockScrimCallback); } else if (mBrightnessMirrorVisible) { mScrimController.transitionTo(ScrimState.BRIGHTNESS_MIRROR); } else if (mState == StatusBarState.SHADE_LOCKED) { 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 b6a3ba80da15c..bea779336b363 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java @@ -405,14 +405,14 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb } /** - * Sets a new legacy alternate bouncer. Only used if mdoern alternate bouncer is NOT enable. + * Sets a new legacy alternate bouncer. Only used if modern alternate bouncer is NOT enabled. */ public void setLegacyAlternateBouncer(@NonNull LegacyAlternateBouncer alternateBouncerLegacy) { if (!mIsModernAlternateBouncerEnabled) { if (!Objects.equals(mAlternateBouncerInteractor.getLegacyAlternateBouncer(), alternateBouncerLegacy)) { mAlternateBouncerInteractor.setLegacyAlternateBouncer(alternateBouncerLegacy); - hideAlternateBouncer(false); + hideAlternateBouncer(true); } } @@ -640,8 +640,7 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb */ public void showPrimaryBouncer(boolean scrimmed) { hideAlternateBouncer(false); - - if (mKeyguardStateController.isShowing() && !isBouncerShowing()) { + if (mKeyguardStateController.isShowing() && !isBouncerShowing()) { mPrimaryBouncerInteractor.show(scrimmed); } updateStates(); @@ -734,7 +733,7 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb showBouncerOrKeyguard(hideBouncerWhenShowing); } if (hideBouncerWhenShowing) { - hideAlternateBouncer(false); + hideAlternateBouncer(true); } mKeyguardUpdateManager.sendKeyguardReset(); updateStates(); @@ -742,8 +741,8 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb } @Override - public void hideAlternateBouncer(boolean forceUpdateScrim) { - updateAlternateBouncerShowing(mAlternateBouncerInteractor.hide() || forceUpdateScrim); + public void hideAlternateBouncer(boolean updateScrim) { + updateAlternateBouncerShowing(mAlternateBouncerInteractor.hide() && updateScrim); } private void updateAlternateBouncerShowing(boolean updateScrim) { @@ -1448,16 +1447,21 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb * For any touches on the NPVC, show the primary bouncer if the alternate bouncer is currently * showing. */ - public void onTouch(MotionEvent event) { - if (mAlternateBouncerInteractor.isVisibleState() + public boolean onTouch(MotionEvent event) { + boolean handledTouch = false; + if (event.getAction() == MotionEvent.ACTION_UP + && mAlternateBouncerInteractor.isVisibleState() && mAlternateBouncerInteractor.hasAlternateBouncerShownWithMinTime()) { showPrimaryBouncer(true); + handledTouch = true; } // Forward NPVC touches to callbacks in case they want to respond to touches for (KeyguardViewManagerCallback callback: mCallbacks) { callback.onTouch(event); } + + return handledTouch; } /** Update keyguard position based on a tapped X coordinate. */ diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationShadeWindowViewControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationShadeWindowViewControllerTest.kt index 82a57438052fa..315663c393222 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationShadeWindowViewControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationShadeWindowViewControllerTest.kt @@ -276,6 +276,20 @@ class NotificationShadeWindowViewControllerTest : SysuiTestCase() { underTest.keyguardMessageArea verify(view).findViewById(R.id.keyguard_message_area) } + + @Test + fun handleDispatchTouchEvent_statusBarViewControllerOnTouch_returnsTrue() { + underTest.setStatusBarViewController(phoneStatusBarViewController) + + // GIVEN the statusBarKeyguardViewManager will handle any touches + whenever(statusBarKeyguardViewManager.onTouch(any())).thenReturn(true) + + // WHEN a touch is dispatched + val returnVal = interactionEventHandler.handleDispatchTouchEvent(downEv) + + // THEN handleDispatchTouchEvent returns true + assertThat(returnVal).isTrue() + } } private val downEv = MotionEvent.obtain(0L, 0L, MotionEvent.ACTION_DOWN, 0f, 0f, 0) 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 e2019b2814d91..31462623ce2d9 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 @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.phone; import static com.android.systemui.keyguard.shared.constants.KeyguardBouncerConstants.EXPANSION_HIDDEN; import static com.android.systemui.keyguard.shared.constants.KeyguardBouncerConstants.EXPANSION_VISIBLE; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -35,6 +36,7 @@ import static org.mockito.Mockito.when; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; +import android.view.MotionEvent; import android.view.View; import android.view.ViewGroup; import android.view.ViewRootImpl; @@ -709,4 +711,48 @@ public class StatusBarKeyguardViewManagerTest extends SysuiTestCase { // THEN alternate bouncer is NOT hidden verify(mAlternateBouncerInteractor, never()).hide(); } + + @Test + public void testAlternateBouncerToShowPrimaryBouncer_updatesScrimControllerOnce() { + // GIVEN the alternate bouncer has shown and calls to hide() will result in successfully + // hiding it + when(mAlternateBouncerInteractor.hide()).thenReturn(true); + when(mKeyguardStateController.isShowing()).thenReturn(true); + when(mPrimaryBouncerInteractor.isFullyShowing()).thenReturn(false); + when(mAlternateBouncerInteractor.isVisibleState()).thenReturn(false); + + // WHEN request to show primary bouncer + mStatusBarKeyguardViewManager.showPrimaryBouncer(true); + + // THEN the scrim isn't updated from StatusBarKeyguardViewManager + verify(mCentralSurfaces, never()).updateScrimController(); + } + + @Test + public void testAlternateBouncerOnTouch_actionDown_doesNotHandleTouch() { + // GIVEN the alternate bouncer has shown for a minimum amount of time + when(mAlternateBouncerInteractor.hasAlternateBouncerShownWithMinTime()).thenReturn(true); + when(mAlternateBouncerInteractor.isVisibleState()).thenReturn(true); + + // WHEN ACTION_DOWN touch event comes + boolean touchHandled = mStatusBarKeyguardViewManager.onTouch( + MotionEvent.obtain(0L, 0L, MotionEvent.ACTION_DOWN, 0f, 0f, 0)); + + // THEN the touch is not handled + assertFalse(touchHandled); + } + + @Test + public void testAlternateBouncerOnTouch_actionUp_handlesTouch() { + // GIVEN the alternate bouncer has shown for a minimum amount of time + when(mAlternateBouncerInteractor.hasAlternateBouncerShownWithMinTime()).thenReturn(true); + when(mAlternateBouncerInteractor.isVisibleState()).thenReturn(true); + + // WHEN ACTION_UP touch event comes + boolean touchHandled = mStatusBarKeyguardViewManager.onTouch( + MotionEvent.obtain(0L, 0L, MotionEvent.ACTION_UP, 0f, 0f, 0)); + + // THEN the touch is handled + assertTrue(touchHandled); + } }