From 1a99bc681816725f144f8e5d4d95491e130b0847 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Wed, 18 Aug 2021 00:10:01 -0700 Subject: [PATCH] Address communal UI glitches. This changelist addresses two sources of UI glitches: - The CommunalHostViewController is now initialized once, preventing multiple, potentially stale, signals to the communal code. - Bouncer presence has been removed from the invalid states as the communal view can be shown as the same time as the bouncer during transition. Bug: 195601027 Test: atest CommunalHostViewControllerTest#testNoShowInvocationOnBouncer Change-Id: I64893d9e57cba8dbbdb6085e7a6e2b677455f273 --- .../communal/CommunalHostViewController.java | 6 +-- .../NotificationPanelViewController.java | 24 ++++++--- .../CommunalHostViewControllerTest.java | 51 +++++++++---------- .../phone/NotificationPanelViewTest.java | 3 +- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java index b06f4430d0e9e..73e5afe1a2dc1 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java @@ -81,7 +81,7 @@ public class CommunalHostViewController extends ViewController // Only show communal view when keyguard is showing and not dozing. private static final int SHOW_COMMUNAL_VIEW_REQUIRED_STATES = STATE_KEYGUARD_SHOWING; private static final int SHOW_COMMUNAL_VIEW_INVALID_STATES = - STATE_DOZING | STATE_BOUNCER_SHOWING | STATE_KEYGUARD_OCCLUDED; + STATE_DOZING | STATE_KEYGUARD_OCCLUDED; private final KeyguardVisibilityHelper mKeyguardVisibilityHelper; @@ -216,9 +216,7 @@ public class CommunalHostViewController extends ViewController } } @Override - public void init() { - super.init(); - + public void onInit() { setState(STATE_KEYGUARD_SHOWING, mKeyguardStateController.isShowing()); setState(STATE_DOZING, mStatusBarStateController.isDozing()); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index d0c9962ba5b4f..f0a35d4cc6bc6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -119,6 +119,7 @@ import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.doze.DozeLog; import com.android.systemui.fragments.FragmentHostManager.FragmentListener; import com.android.systemui.fragments.FragmentService; +import com.android.systemui.idle.IdleHostView; import com.android.systemui.idle.IdleHostViewController; import com.android.systemui.idle.dagger.IdleViewComponent; import com.android.systemui.media.KeyguardMediaController; @@ -902,10 +903,20 @@ public class NotificationPanelViewController extends PanelViewController { mIdleHostViewController = idleViewComponent.getIdleHostViewController(); mIdleHostViewController.init(); + if (mCommunalView != null) { + CommunalViewComponent communalViewComponent = + mCommunalViewComponentFactory.build(mCommunalView); + mCommunalViewController = + communalViewComponent.getCommunalHostViewController(); + mCommunalViewController.init(); + } + + updateViewControllers( mView.findViewById(R.id.keyguard_status_view), userAvatarView, keyguardUserSwitcherView, + mView.findViewById(R.id.idle_host_view), mCommunalView); mNotificationContainerParent = mView.findViewById(R.id.notification_container_parent); NotificationStackScrollLayout stackScrollLayout = mView.findViewById( @@ -996,6 +1007,7 @@ public class NotificationPanelViewController extends PanelViewController { private void updateViewControllers(KeyguardStatusView keyguardStatusView, UserAvatarView userAvatarView, KeyguardUserSwitcherView keyguardUserSwitcherView, + IdleHostView idleHostView, CommunalHostView communalView) { // Re-associate the KeyguardStatusViewController KeyguardStatusViewComponent statusViewComponent = @@ -1003,13 +1015,9 @@ public class NotificationPanelViewController extends PanelViewController { mKeyguardStatusViewController = statusViewComponent.getKeyguardStatusViewController(); mKeyguardStatusViewController.init(); - if (communalView != null) { - CommunalViewComponent communalViewComponent = - mCommunalViewComponentFactory.build(communalView); - mCommunalViewController = - communalViewComponent.getCommunalHostViewController(); - mCommunalViewController.init(); - } + IdleViewComponent idleViewComponent = mIdleViewComponentFactory.build(idleHostView); + mIdleHostViewController = idleViewComponent.getIdleHostViewController(); + mIdleHostViewController.init(); if (mKeyguardUserSwitcherController != null) { // Try to close the switcher so that callbacks are triggered if necessary. @@ -1174,7 +1182,7 @@ public class NotificationPanelViewController extends PanelViewController { mBigClockContainer.removeAllViews(); updateViewControllers(mView.findViewById(R.id.keyguard_status_view), userAvatarView, - keyguardUserSwitcherView, mCommunalView); + keyguardUserSwitcherView, mView.findViewById(R.id.idle_host_view), mCommunalView); // Update keyguard bottom area int index = mView.indexOfChild(mKeyguardBottomArea); diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java index ea00c6f7fb104..6cfa40af32663 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java @@ -16,6 +16,7 @@ package com.android.systemui.communal; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.never; @@ -120,33 +121,6 @@ public class CommunalHostViewControllerTest extends SysuiTestCase { verify(mCommunalView).setVisibility(View.INVISIBLE); } - @Test - public void testHideOnBouncer() { - ArgumentCaptor callbackCapture = - ArgumentCaptor.forClass(KeyguardUpdateMonitorCallback.class); - - // Capture callback value for later use. - verify(mKeyguardUpdateMonitor).registerCallback(callbackCapture.capture()); - - // Establish a visible communal view. - mController.show(new WeakReference<>(mCommunalSource)); - mFakeExecutor.runAllReady(); - verify(mCommunalView).setVisibility(View.VISIBLE); - Mockito.clearInvocations(mCommunalView); - - // Trigger bouncer. - Mockito.clearInvocations(mCommunalView); - callbackCapture.getValue().onKeyguardBouncerChanged(true); - mFakeExecutor.runAllReady(); - verify(mCommunalView).setVisibility(View.INVISIBLE); - - // Hide bouncer - Mockito.clearInvocations(mCommunalView); - callbackCapture.getValue().onKeyguardBouncerChanged(false); - mFakeExecutor.runAllReady(); - verify(mCommunalView).setVisibility(View.VISIBLE); - } - @Test public void testHideOnOcclude() { ArgumentCaptor callbackCapture = @@ -242,4 +216,27 @@ public class CommunalHostViewControllerTest extends SysuiTestCase { mFakeExecutor.runAllReady(); verify(mCommunalSource, never()).requestCommunalView(any()); } + + @Test + public void testNoShowInvocationOnBouncer() { + ArgumentCaptor callbackCapture = + ArgumentCaptor.forClass(KeyguardUpdateMonitorCallback.class); + + // Capture callback value for later use. + verify(mKeyguardUpdateMonitor).registerCallback(callbackCapture.capture()); + + // Set source so it will be cleared if in invalid state. + mController.show(new WeakReference<>(mCommunalSource)); + mFakeExecutor.runAllReady(); + clearInvocations(mCommunalStateController, mCommunalView); + + // Change bouncer to showing. + callbackCapture.getValue().onKeyguardBouncerChanged(true); + mFakeExecutor.runAllReady(); + + // Verify that there were no requests to remove all child views or set the communal + // state to not showing. + verify(mCommunalStateController, never()).setCommunalViewShowing(eq(false)); + verify(mCommunalView, never()).removeAllViews(); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java index 99f6354b27cee..a5c952725f62f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java @@ -851,9 +851,10 @@ public class NotificationPanelViewTest extends SysuiTestCase { @Test public void testCommunalhostViewControllerInit() { + verify(mCommunalHostViewController, times(1)).init(); clearInvocations(mCommunalHostViewController); givenViewAttached(); - verify(mCommunalHostViewController).init(); + verify(mCommunalHostViewController, never()).init(); } @Test