diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java index e3318127af931..46243c0111c5f 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java @@ -2090,7 +2090,8 @@ public final class NotificationPanelViewController { animator.start(); } - private void onFlingEnd(boolean cancelled) { + @VisibleForTesting + void onFlingEnd(boolean cancelled) { mIsFlinging = false; // No overshoot when the animation ends setOverExpansionInternal(0, false /* isFromGesture */); @@ -3825,12 +3826,14 @@ public final class NotificationPanelViewController { } } - private void setIsClosing(boolean isClosing) { + @VisibleForTesting + void setIsClosing(boolean isClosing) { boolean wasClosing = isClosing(); mClosing = isClosing; if (wasClosing != isClosing) { mPanelEventsEmitter.notifyPanelCollapsingChanged(isClosing); } + mAmbientState.setIsClosing(isClosing); } private void updateDozingVisibilities(boolean animate) { @@ -4621,14 +4624,16 @@ public final class NotificationPanelViewController { Log.v(TAG, (mViewName != null ? (mViewName + ": ") : "") + String.format(fmt, args)); } - private void notifyExpandingStarted() { + @VisibleForTesting + void notifyExpandingStarted() { if (!mExpanding) { mExpanding = true; onExpandingStarted(); } } - private void notifyExpandingFinished() { + @VisibleForTesting + void notifyExpandingFinished() { endClosing(); if (mExpanding) { mExpanding = false; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AmbientState.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AmbientState.java index 2719dd88b7be0..b2628e40e77e2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AmbientState.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AmbientState.java @@ -142,6 +142,11 @@ public class AmbientState implements Dumpable { */ private boolean mIsFlingRequiredAfterLockScreenSwipeUp = false; + /** + * Whether the shade is currently closing. + */ + private boolean mIsClosing; + @VisibleForTesting public boolean isFlingRequiredAfterLockScreenSwipeUp() { return mIsFlingRequiredAfterLockScreenSwipeUp; @@ -717,6 +722,20 @@ public class AmbientState implements Dumpable { && mStatusBarKeyguardViewManager.isBouncerInTransit(); } + /** + * @param isClosing Whether the shade is currently closing. + */ + public void setIsClosing(boolean isClosing) { + mIsClosing = isClosing; + } + + /** + * @return Whether the shade is currently closing. + */ + public boolean isClosing() { + return mIsClosing; + } + @Override public void dump(PrintWriter pw, String[] args) { pw.println("mTopPadding=" + mTopPadding); @@ -761,5 +780,6 @@ public class AmbientState implements Dumpable { + mIsFlingRequiredAfterLockScreenSwipeUp); pw.println("mZDistanceBetweenElements=" + mZDistanceBetweenElements); pw.println("mBaseZHeight=" + mBaseZHeight); + pw.println("mIsClosing=" + mIsClosing); } } 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 55c577f1ea39e..2272411b43144 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 @@ -255,7 +255,6 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable private boolean mClearAllInProgress; private FooterClearAllListener mFooterClearAllListener; private boolean mFlingAfterUpEvent; - /** * Was the scroller scrolled to the top when the down motion was observed? */ @@ -4020,8 +4019,9 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable setOwnScrollY(0); } + @VisibleForTesting @ShadeViewRefactor(RefactorComponent.COORDINATOR) - private void setIsExpanded(boolean isExpanded) { + void setIsExpanded(boolean isExpanded) { boolean changed = isExpanded != mIsExpanded; mIsExpanded = isExpanded; mStackScrollAlgorithm.setIsExpanded(isExpanded); @@ -4842,13 +4842,21 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable } } + @VisibleForTesting @ShadeViewRefactor(RefactorComponent.COORDINATOR) - private void setOwnScrollY(int ownScrollY) { + void setOwnScrollY(int ownScrollY) { setOwnScrollY(ownScrollY, false /* animateScrollChangeListener */); } @ShadeViewRefactor(RefactorComponent.COORDINATOR) private void setOwnScrollY(int ownScrollY, boolean animateStackYChangeListener) { + // Avoid Flicking during clear all + // when the shade finishes closing, onExpansionStopped will call + // resetScrollPosition to setOwnScrollY to 0 + if (mAmbientState.isClosing()) { + return; + } + if (ownScrollY != mOwnScrollY) { // We still want to call the normal scrolled changed for accessibility reasons onScrollChanged(mScrollX, ownScrollY, mScrollX, mOwnScrollY); diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerTest.java index 37be3439fdc8b..853f81f20e926 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerTest.java @@ -38,6 +38,7 @@ import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -173,6 +174,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.stubbing.Answer; @@ -1506,6 +1508,33 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { ); } + + /** + * When shade is flinging to close and this fling is not intercepted, + * {@link AmbientState#setIsClosing(boolean)} should be called before + * {@link NotificationStackScrollLayoutController#onExpansionStopped()} + * to ensure scrollY can be correctly set to be 0 + */ + @Test + public void onShadeFlingClosingEnd_mAmbientStateSetClose_thenOnExpansionStopped() { + // Given: Shade is expanded + mNotificationPanelViewController.notifyExpandingFinished(); + mNotificationPanelViewController.setIsClosing(false); + + // When: Shade flings to close not canceled + mNotificationPanelViewController.notifyExpandingStarted(); + mNotificationPanelViewController.setIsClosing(true); + mNotificationPanelViewController.onFlingEnd(false); + + // Then: AmbientState's mIsClosing should be set to false + // before mNotificationStackScrollLayoutController.onExpansionStopped() is called + // to ensure NotificationStackScrollLayout.resetScrollPosition() -> resetScrollPosition + // -> setOwnScrollY(0) can set scrollY to 0 when shade is closed + InOrder inOrder = inOrder(mAmbientState, mNotificationStackScrollLayoutController); + inOrder.verify(mAmbientState).setIsClosing(false); + inOrder.verify(mNotificationStackScrollLayoutController).onExpansionStopped(); + } + private static MotionEvent createMotionEvent(int x, int y, int action) { return MotionEvent.obtain( /* downTime= */ 0, /* eventTime= */ 0, action, x, y, /* metaState= */ 0); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/AmbientStateTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/AmbientStateTest.kt index 11798a7a4f964..87f4c323b7ccb 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/AmbientStateTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/AmbientStateTest.kt @@ -361,6 +361,22 @@ class AmbientStateTest : SysuiTestCase() { assertThat(sut.isOnKeyguard).isFalse() } // endregion + + // region mIsClosing + @Test + fun isClosing_whenShadeClosing_shouldReturnTrue() { + sut.setIsClosing(true) + + assertThat(sut.isClosing).isTrue() + } + + @Test + fun isClosing_whenShadeFinishClosing_shouldReturnFalse() { + sut.setIsClosing(false) + + assertThat(sut.isClosing).isFalse() + } + // endregion } // region Arrange helper methods. 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 43530365360b4..35c8b61b63837 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 @@ -728,6 +728,57 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { verify(mNotificationStackSizeCalculator).computeHeight(any(), anyInt(), anyFloat()); } + @Test + public void testSetOwnScrollY_shadeNotClosing_scrollYChanges() { + // Given: shade is not closing, scrollY is 0 + mAmbientState.setScrollY(0); + assertEquals(0, mAmbientState.getScrollY()); + mAmbientState.setIsClosing(false); + + // When: call NotificationStackScrollLayout.setOwnScrollY to set scrollY to 1 + mStackScroller.setOwnScrollY(1); + + // Then: scrollY should be set to 1 + assertEquals(1, mAmbientState.getScrollY()); + + // Reset scrollY back to 0 to avoid interfering with other tests + mStackScroller.setOwnScrollY(0); + assertEquals(0, mAmbientState.getScrollY()); + } + + @Test + public void testSetOwnScrollY_shadeClosing_scrollYDoesNotChange() { + // Given: shade is closing, scrollY is 0 + mAmbientState.setScrollY(0); + assertEquals(0, mAmbientState.getScrollY()); + mAmbientState.setIsClosing(true); + + // When: call NotificationStackScrollLayout.setOwnScrollY to set scrollY to 1 + mStackScroller.setOwnScrollY(1); + + // Then: scrollY should not change, it should still be 0 + assertEquals(0, mAmbientState.getScrollY()); + + // Reset scrollY and mAmbientState.mIsClosing to avoid interfering with other tests + mAmbientState.setIsClosing(false); + mStackScroller.setOwnScrollY(0); + assertEquals(0, mAmbientState.getScrollY()); + } + + @Test + public void onShadeFlingClosingEnd_scrollYShouldBeSetToZero() { + // Given: mAmbientState.mIsClosing is set to be true + // mIsExpanded is set to be false + mAmbientState.setIsClosing(true); + mStackScroller.setIsExpanded(false); + + // When: onExpansionStopped is called + mStackScroller.onExpansionStopped(); + + // Then: mAmbientState.scrollY should be set to be 0 + assertEquals(mAmbientState.getScrollY(), 0); + } + private void setBarStateForTest(int state) { // Can't inject this through the listener or we end up on the actual implementation // rather than the mock because the spy just coppied the anonymous inner /shruggie.