From 68b9787f2b2e0f370cc91aba12bb0619b8546db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20Kurucz?= Date: Mon, 22 May 2023 17:04:51 +0000 Subject: [PATCH] Reset the clearAllInProgress flag when the shade is collapsed The NSSL tracks if the clear-all animation is ongoing for things like preventing to launch another dismiss, updating roundness, updating scrolling behaviour and the visibility of the footer. Currently this flag is only resetted if a StackStateAnimator generated animation has finished, but there are some scenarios when a clear-all is not finalized by these animations, because they didn't run. This results in hiding the footer, when it needs to be shown and not letting a manual dismiss to finish (leaving a blank view translated out of the visible area). This CL resets the clear-all animationn flags, when the shade is collapsed. It is addressing the scenario, when the StackSctateAnimator animations were not started, because the shade is closed by the time we get the new list from the pipeline. Fixes: 273989305 Bug: 283078415 Test: atest NotificationStackScrollLayoutTest Test: close the shade while the clear-all animation is running, post some new notifications and see how they behave Change-Id: Iaff096f037b45a7675db9bf85a39633d9f1837f5 --- .../stack/NotificationStackScrollLayout.java | 25 +++++-- .../NotificationStackScrollLayoutTest.java | 65 +++++++++++++++++-- 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index b81cb2be1c4d6..64fcfd8674299 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -4079,6 +4079,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable if (!mIsExpansionChanging) { cancelActiveSwipe(); } + finalizeClearAllAnimation(); } updateNotificationAnimationStates(); updateChronometers(); @@ -4174,20 +4175,30 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable runAnimationFinishedRunnables(); clearTransient(); clearHeadsUpDisappearRunning(); + finalizeClearAllAnimation(); + } + private void finalizeClearAllAnimation() { if (mAmbientState.isClearAllInProgress()) { setClearAllInProgress(false); if (mShadeNeedsToClose) { mShadeNeedsToClose = false; - postDelayed( - () -> { - mShadeController.animateCollapseShade(CommandQueue.FLAG_EXCLUDE_NONE); - }, - DELAY_BEFORE_SHADE_CLOSE /* delayMillis */); + if (mIsExpanded) { + collapseShadeDelayed(); + } } } } + private void collapseShadeDelayed() { + postDelayed( + () -> { + mShadeController.animateCollapseShade( + CommandQueue.FLAG_EXCLUDE_NONE); + }, + DELAY_BEFORE_SHADE_CLOSE /* delayMillis */); + } + private void clearHeadsUpDisappearRunning() { for (int i = 0; i < getChildCount(); i++) { View view = getChildAt(i); @@ -4535,6 +4546,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable mFooterView.setFooterLabelVisible(mHasFilteredOutSeenNotifications); } + @VisibleForTesting public void setClearAllInProgress(boolean clearAllInProgress) { mClearAllInProgress = clearAllInProgress; mAmbientState.setClearAllInProgress(clearAllInProgress); @@ -5151,7 +5163,8 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable mHeadsUpAppearanceController = headsUpAppearanceController; } - private boolean isVisible(View child) { + @VisibleForTesting + public boolean isVisible(View child) { boolean hasClipBounds = child.getClipBounds(mTmpRect); return child.getVisibility() == View.VISIBLE && (!hasClipBounds || mTmpRect.height() > 0); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java index a4ee349f5b710..a9cfe7f272028 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java @@ -41,6 +41,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -575,10 +576,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { mStackScroller.inflateFooterView(); // add notification - ExpandableNotificationRow row = mock(ExpandableNotificationRow.class); - NotificationEntry entry = mock(NotificationEntry.class); - when(row.getEntry()).thenReturn(entry); - when(entry.isClearable()).thenReturn(true); + ExpandableNotificationRow row = createClearableRow(); mStackScroller.addContainerView(row); mStackScroller.onUpdateRowStates(); @@ -647,6 +645,50 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { assertEquals(ROWS_GENTLE, selected[0]); } + @Test + public void testClearNotifications_clearAllInProgress() { + ExpandableNotificationRow row = createClearableRow(); + when(row.getEntry().hasFinishedInitialization()).thenReturn(true); + doReturn(true).when(mStackScroller).isVisible(row); + mStackScroller.addContainerView(row); + + mStackScroller.clearNotifications(ROWS_ALL, false); + + assertClearAllInProgress(true); + verify(mNotificationRoundnessManager).setClearAllInProgress(true); + } + + @Test + public void testOnChildAnimationFinished_resetsClearAllInProgress() { + mStackScroller.setClearAllInProgress(true); + + mStackScroller.onChildAnimationFinished(); + + assertClearAllInProgress(false); + verify(mNotificationRoundnessManager).setClearAllInProgress(false); + } + + @Test + public void testShadeCollapsed_resetsClearAllInProgress() { + mStackScroller.setClearAllInProgress(true); + + mStackScroller.setIsExpanded(false); + + assertClearAllInProgress(false); + verify(mNotificationRoundnessManager).setClearAllInProgress(false); + } + + @Test + public void testShadeExpanded_doesntChangeClearAllInProgress() { + mStackScroller.setClearAllInProgress(true); + clearInvocations(mNotificationRoundnessManager); + + mStackScroller.setIsExpanded(true); + + assertClearAllInProgress(true); + verify(mNotificationRoundnessManager, never()).setClearAllInProgress(anyBoolean()); + } + @Test public void testAddNotificationUpdatesSpeedBumpIndex() { // initial state calculated == 0 @@ -896,6 +938,21 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { mStackScroller.setStatusBarState(state); } + private ExpandableNotificationRow createClearableRow() { + ExpandableNotificationRow row = mock(ExpandableNotificationRow.class); + NotificationEntry entry = mock(NotificationEntry.class); + when(row.canViewBeCleared()).thenReturn(true); + when(row.getEntry()).thenReturn(entry); + when(entry.isClearable()).thenReturn(true); + + return row; + } + + private void assertClearAllInProgress(boolean expected) { + assertEquals(expected, mStackScroller.getClearAllInProgress()); + assertEquals(expected, mAmbientState.isClearAllInProgress()); + } + private static void mockBoundsOnScreen(View view, Rect bounds) { doAnswer(invocation -> { Rect out = invocation.getArgument(0);