From 12f1c42723758b73171e7bba0a47b6fb59be6251 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 31 May 2023 19:49:11 +0000 Subject: [PATCH 1/2] [Central Surfaces] Add flag for fetching bouncerShowing from diff place. Bug: 285174336 Test: compiles Change-Id: I9f8a6402733ab78162d3d5f7d8b19077197ecc57 --- packages/SystemUI/src/com/android/systemui/flags/Flags.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt index 4b994c9b63bde..601badbe233c4 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt @@ -742,4 +742,10 @@ object Flags { @JvmField val BIGPICTURE_NOTIFICATION_LAZY_LOADING = unreleasedFlag(283447257, "bigpicture_notification_lazy_loading") + + // 2900 - CentralSurfaces-related flags + + // TODO(b/285174336): Tracking Bug + @JvmField + val USE_REPOS_FOR_BOUNCER_SHOWING = unreleasedFlag(2900, "use_repos_for_bouncer_showing") } From 7d60690e52dd7a972bb73a0f37edd6180ceb92bb Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Fri, 26 May 2023 16:36:59 +0000 Subject: [PATCH 2/2] [Central Surfaces] NSSLC uses BouncerInteractor for bouncer showing. Removes a CentralSurfaces reference by asking PrimaryBouncerInteractor directly for the status. Note: Based on logs, once the user has successfully dismissed the bouncer, `PrimaryBouncerInteractor#isBouncerShowing` updates to false ~10-200ms before `CentralSurfaces#isBouncerShowing` updates to false. However, I don't see any regressions with b/267060171 after making this change. The change is protected by a flag in case we do notice regerssions on other devices. Bug: 277762009 Flag: USE_REPOS_FOR_BOUNCER_SHOWING Test: manual: on tablet, pull up bouncer from dreaming and verify "No new notifications" is not showing behind bouncer (aka, verify b/267060171 has not regressed). Once PIN is successfully entered, verify "No new notifications" is not showing during the transition either. Test: atest NotificationStackScrollLayoutControllerTest Change-Id: I05874ca2a770642f7e3d868eeaa3638b5fe630ee --- .../interactor/PrimaryBouncerInteractor.kt | 8 +- ...tificationStackScrollLayoutController.java | 35 ++++++++- .../statusbar/phone/CentralSurfacesImpl.java | 5 +- ...cationStackScrollLayoutControllerTest.java | 74 +++++++++++++++++-- .../phone/CentralSurfacesImplTest.java | 1 + 5 files changed, 112 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/PrimaryBouncerInteractor.kt b/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/PrimaryBouncerInteractor.kt index e1545a4d432ec..4add5171770a2 100644 --- a/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/PrimaryBouncerInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/bouncer/domain/interactor/PrimaryBouncerInteractor.kt @@ -48,6 +48,7 @@ import com.android.systemui.statusbar.policy.KeyguardStateController import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterNotNull @@ -86,7 +87,7 @@ constructor( } val keyguardAuthenticated: Flow = repository.keyguardAuthenticated.filterNotNull() - val isShowing: Flow = repository.primaryBouncerShow + val isShowing: StateFlow = repository.primaryBouncerShow val startingToHide: Flow = repository.primaryBouncerStartingToHide.filter { it }.map {} val isBackButtonEnabled: Flow = repository.isBackButtonEnabled.filterNotNull() val showMessage: Flow = repository.showMessage.filterNotNull() @@ -383,8 +384,9 @@ constructor( mainHandler.removeCallbacks(showRunnable) } - private fun isBouncerShowing(): Boolean { - return repository.primaryBouncerShow.value + /** Returns whether the primary bouncer is currently showing. */ + fun isBouncerShowing(): Boolean { + return isShowing.value } /** Whether we want to wait to show the bouncer in case passive auth succeeds. */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java index a70862aead0f1..aef623ae8fd03 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java @@ -56,11 +56,13 @@ import com.android.internal.logging.UiEventLogger; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.systemui.ExpandHelper; import com.android.systemui.Gefingerpoken; +import com.android.systemui.bouncer.domain.interactor.PrimaryBouncerInteractor; import com.android.systemui.classifier.Classifier; import com.android.systemui.classifier.FalsingCollector; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dump.DumpManager; import com.android.systemui.flags.FeatureFlags; +import com.android.systemui.flags.Flags; import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor; import com.android.systemui.media.controls.ui.KeyguardMediaController; import com.android.systemui.plugins.ActivityStarter; @@ -175,6 +177,7 @@ public class NotificationStackScrollLayoutController { private final SysuiStatusBarStateController mStatusBarStateController; private final KeyguardBypassController mKeyguardBypassController; private final KeyguardInteractor mKeyguardInteractor; + private final PrimaryBouncerInteractor mPrimaryBouncerInteractor; private final NotificationLockscreenUserManager mLockscreenUserManager; // TODO: CentralSurfaces should be encapsulated behind a Controller private final CentralSurfaces mCentralSurfaces; @@ -195,6 +198,7 @@ public class NotificationStackScrollLayoutController { @Nullable private Boolean mHistoryEnabled; private int mBarState; + private boolean mIsBouncerShowingFromCentralSurfaces; private HeadsUpAppearanceController mHeadsUpAppearanceController; private final FeatureFlags mFeatureFlags; private final NotificationTargetsHelper mNotificationTargetsHelper; @@ -631,6 +635,7 @@ public class NotificationStackScrollLayoutController { KeyguardMediaController keyguardMediaController, KeyguardBypassController keyguardBypassController, KeyguardInteractor keyguardInteractor, + PrimaryBouncerInteractor primaryBouncerInteractor, ZenModeController zenModeController, NotificationLockscreenUserManager lockscreenUserManager, Optional nsslViewModel, @@ -680,6 +685,7 @@ public class NotificationStackScrollLayoutController { mKeyguardMediaController = keyguardMediaController; mKeyguardBypassController = keyguardBypassController; mKeyguardInteractor = keyguardInteractor; + mPrimaryBouncerInteractor = primaryBouncerInteractor; mZenModeController = zenModeController; mLockscreenUserManager = lockscreenUserManager; mViewModel = nsslViewModel; @@ -1205,6 +1211,14 @@ public class NotificationStackScrollLayoutController { return mView.getFooterViewHeightWithPadding(); } + /** + * Sets whether the bouncer is currently showing. Should only be called from + * {@link CentralSurfaces}. + */ + public void setBouncerShowingFromCentralSurfaces(boolean bouncerShowing) { + mIsBouncerShowingFromCentralSurfaces = bouncerShowing; + } + /** * Set the visibility of the view, and propagate it to specific children. * @@ -1236,13 +1250,32 @@ public class NotificationStackScrollLayoutController { // That avoids "No Notifications" to blink when transitioning to AOD. // For more details, see: b/228790482 && !isInTransitionToKeyguard() - && !mCentralSurfaces.isBouncerShowing(); + // Don't show any notification content if the bouncer is showing. See b/267060171. + && !isBouncerShowing(); mView.updateEmptyShadeView(shouldShow, mZenModeController.areNotificationsHiddenInShade()); Trace.endSection(); } + /** + * Returns whether the bouncer is currently showing. + * + * There's a possible timing difference between when CentralSurfaces marks the bouncer as not + * showing and when PrimaryBouncerInteractor marks the bouncer as not showing. (CentralSurfaces + * appears to mark the bouncer as showing for 10-200ms longer than PrimaryBouncerInteractor.) + * + * This timing difference could be load bearing, which is why we have a feature flag protecting + * where we fetch the value from. This flag is intended to be short-lived. + */ + private boolean isBouncerShowing() { + if (mFeatureFlags.isEnabled(Flags.USE_REPOS_FOR_BOUNCER_SHOWING)) { + return mPrimaryBouncerInteractor.isBouncerShowing(); + } else { + return mIsBouncerShowingFromCentralSurfaces; + } + } + /** * Update the importantForAccessibility of NotificationStackScrollLayout. *

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 81048d60c6d9a..a1440767eab76 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -648,7 +648,9 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { private final Optional mStartingSurfaceOptional; private final ActivityIntentHelper mActivityIntentHelper; - private NotificationStackScrollLayoutController mStackScrollerController; + + @VisibleForTesting + protected NotificationStackScrollLayoutController mStackScrollerController; private final ColorExtractor.OnColorsChangedListener mOnColorsChangedListener = (extractor, which) -> updateTheme(); @@ -2922,6 +2924,7 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { mBouncerShowing = bouncerShowing; mKeyguardBypassController.setBouncerShowing(bouncerShowing); mPulseExpansionHandler.setBouncerShowing(bouncerShowing); + mStackScrollerController.setBouncerShowingFromCentralSurfaces(bouncerShowing); setBouncerShowingForStatusBarComponents(bouncerShowing); mStatusBarHideIconsForBouncerManager.setBouncerShowingAndTriggerUpdate(bouncerShowing); mCommandQueue.recomputeDisableFlags(mDisplayId, true /* animate */); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutControllerTest.java index 0e966dc655f70..18cf8765c4726 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutControllerTest.java @@ -44,10 +44,12 @@ import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.UiEventLogger; import com.android.internal.logging.nano.MetricsProto; import com.android.systemui.SysuiTestCase; +import com.android.systemui.bouncer.domain.interactor.PrimaryBouncerInteractor; import com.android.systemui.classifier.FalsingCollectorFake; import com.android.systemui.classifier.FalsingManagerFake; import com.android.systemui.dump.DumpManager; -import com.android.systemui.flags.FeatureFlags; +import com.android.systemui.flags.FakeFeatureFlags; +import com.android.systemui.flags.Flags; import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor; import com.android.systemui.media.controls.ui.KeyguardMediaController; import com.android.systemui.plugins.ActivityStarter; @@ -122,6 +124,7 @@ public class NotificationStackScrollLayoutControllerTest extends SysuiTestCase { @Mock private SysuiStatusBarStateController mSysuiStatusBarStateController; @Mock private KeyguardBypassController mKeyguardBypassController; @Mock private KeyguardInteractor mKeyguardInteractor; + @Mock private PrimaryBouncerInteractor mPrimaryBouncerInteractor; @Mock private NotificationLockscreenUserManager mNotificationLockscreenUserManager; @Mock private MetricsLogger mMetricsLogger; @Mock private DumpManager mDumpManager; @@ -145,7 +148,7 @@ public class NotificationStackScrollLayoutControllerTest extends SysuiTestCase { @Mock private StackStateLogger mStackLogger; @Mock private NotificationStackScrollLogger mLogger; @Mock private NotificationStackSizeCalculator mNotificationStackSizeCalculator; - @Mock private FeatureFlags mFeatureFlags; + private FakeFeatureFlags mFeatureFlags = new FakeFeatureFlags(); @Mock private NotificationTargetsHelper mNotificationTargetsHelper; @Mock private SecureSettings mSecureSettings; @Mock private NotificationIconAreaController mIconAreaController; @@ -163,6 +166,8 @@ public class NotificationStackScrollLayoutControllerTest extends SysuiTestCase { public void setUp() { MockitoAnnotations.initMocks(this); + mFeatureFlags.set(Flags.USE_REPOS_FOR_BOUNCER_SHOWING, false); + when(mNotificationSwipeHelperBuilder.build()).thenReturn(mNotificationSwipeHelper); } @@ -262,28 +267,84 @@ public class NotificationStackScrollLayoutControllerTest extends SysuiTestCase { } @Test - public void testUpdateEmptyShadeView_bouncerShowing_hideEmptyView() { + public void testUpdateEmptyShadeView_bouncerShowing_flagOff_hideEmptyView() { when(mZenModeController.areNotificationsHiddenInShade()).thenReturn(false); initController(/* viewIsAttached= */ true); - when(mCentralSurfaces.isBouncerShowing()).thenReturn(true); + // WHEN the flag is off and *only* CentralSurfaces has bouncer as showing + mFeatureFlags.set(Flags.USE_REPOS_FOR_BOUNCER_SHOWING, false); + mController.setBouncerShowingFromCentralSurfaces(true); + when(mPrimaryBouncerInteractor.isBouncerShowing()).thenReturn(false); + setupShowEmptyShadeViewState(true); reset(mNotificationStackScrollLayout); mController.updateShowEmptyShadeView(); + + // THEN the CentralSurfaces value is used. Since the bouncer is showing, we hide the empty + // view. verify(mNotificationStackScrollLayout).updateEmptyShadeView( /* visible= */ false, /* areNotificationsHiddenInShade= */ false); } @Test - public void testUpdateEmptyShadeView_bouncerNotShowing_showEmptyView() { + public void testUpdateEmptyShadeView_bouncerShowing_flagOn_hideEmptyView() { when(mZenModeController.areNotificationsHiddenInShade()).thenReturn(false); initController(/* viewIsAttached= */ true); - when(mCentralSurfaces.isBouncerShowing()).thenReturn(false); + // WHEN the flag is on and *only* PrimaryBouncerInteractor has bouncer as showing + mFeatureFlags.set(Flags.USE_REPOS_FOR_BOUNCER_SHOWING, true); + when(mPrimaryBouncerInteractor.isBouncerShowing()).thenReturn(true); + mController.setBouncerShowingFromCentralSurfaces(false); + setupShowEmptyShadeViewState(true); reset(mNotificationStackScrollLayout); mController.updateShowEmptyShadeView(); + + // THEN the PrimaryBouncerInteractor value is used. Since the bouncer is showing, we + // hide the empty view. + verify(mNotificationStackScrollLayout).updateEmptyShadeView( + /* visible= */ false, + /* areNotificationsHiddenInShade= */ false); + } + + @Test + public void testUpdateEmptyShadeView_bouncerNotShowing_flagOff_showEmptyView() { + when(mZenModeController.areNotificationsHiddenInShade()).thenReturn(false); + initController(/* viewIsAttached= */ true); + + // WHEN the flag is off and *only* CentralSurfaces has bouncer as not showing + mFeatureFlags.set(Flags.USE_REPOS_FOR_BOUNCER_SHOWING, false); + mController.setBouncerShowingFromCentralSurfaces(false); + when(mPrimaryBouncerInteractor.isBouncerShowing()).thenReturn(true); + + setupShowEmptyShadeViewState(true); + reset(mNotificationStackScrollLayout); + mController.updateShowEmptyShadeView(); + + // THEN the CentralSurfaces value is used. Since the bouncer isn't showing, we can show the + // empty view. + verify(mNotificationStackScrollLayout).updateEmptyShadeView( + /* visible= */ true, + /* areNotificationsHiddenInShade= */ false); + } + + @Test + public void testUpdateEmptyShadeView_bouncerNotShowing_flagOn_showEmptyView() { + when(mZenModeController.areNotificationsHiddenInShade()).thenReturn(false); + initController(/* viewIsAttached= */ true); + + // WHEN the flag is on and *only* PrimaryBouncerInteractor has bouncer as not showing + mFeatureFlags.set(Flags.USE_REPOS_FOR_BOUNCER_SHOWING, true); + when(mPrimaryBouncerInteractor.isBouncerShowing()).thenReturn(false); + mController.setBouncerShowingFromCentralSurfaces(true); + + setupShowEmptyShadeViewState(true); + reset(mNotificationStackScrollLayout); + mController.updateShowEmptyShadeView(); + + // THEN the PrimaryBouncerInteractor value is used. Since the bouncer isn't showing, we + // can show the empty view. verify(mNotificationStackScrollLayout).updateEmptyShadeView( /* visible= */ true, /* areNotificationsHiddenInShade= */ false); @@ -548,6 +609,7 @@ public class NotificationStackScrollLayoutControllerTest extends SysuiTestCase { mKeyguardMediaController, mKeyguardBypassController, mKeyguardInteractor, + mPrimaryBouncerInteractor, mZenModeController, mNotificationLockscreenUserManager, Optional.empty(), 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 5ed9a865de930..771af11459345 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 @@ -580,6 +580,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { mCentralSurfaces.mPresenter = mNotificationPresenter; mCentralSurfaces.mKeyguardIndicationController = mKeyguardIndicationController; mCentralSurfaces.mBarService = mBarService; + mCentralSurfaces.mStackScrollerController = mStackScrollerController; mCentralSurfaces.mStackScroller = mStackScroller; mCentralSurfaces.mGestureWakeLock = mPowerManager.newWakeLock( PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "sysui:GestureWakeLock");