Fix Clear all Notifications Animation to avoid QS Panel Flicking

Avoid AmbientState.mScrollY from being set to 0(happens multiple times) during the clear all animation so that QS Panel would not flicking to show up when mScrollY > 0 before clear all.

Bug: 214214514
Test: `atest AmbientStateTest NotificationStackScrollLayoutTest NotificationPanelViewControllerTest` and manual: receive multiple notifications on phone so that there's scroll to show the clear all button. Click clear all, NotificationStackScrollLayout should not scroll to 0 suddenly before shade's closing.
Change-Id: Ie9a0a8aa51278077999873e08cd0c7a0b50f91fb
This commit is contained in:
Yining Liu
2022-10-03 22:07:15 +00:00
parent 4ed760213f
commit 2a83564369
6 changed files with 136 additions and 7 deletions

View File

@@ -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;

View File

@@ -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);
}
}

View File

@@ -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);

View File

@@ -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);

View File

@@ -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.

View File

@@ -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.