From 655f0a37c0d0a75054de82aed0d40af2f9929ce9 Mon Sep 17 00:00:00 2001 From: András Kurucz Date: Wed, 15 Feb 2023 14:03:15 +0000 Subject: Avoid duplicate calls to the FalsingManager upon Notification dismissals Upon handling the touch events in NotificationSwipeHelper we were calling isDismissGesture(event) multiple times for the same event. It resulted in multiple calls to the FalsingManager#isFalseTouch() which then recalculated the results for each call. Bug: 236197248 Test: atest NotificationSwipeHelperTest Change-Id: Ie0e24a1b3107564f7e0810501739fe2a67d95b5b --- .../stack/NotificationSwipeHelper.java | 4 +- .../stack/NotificationSwipeHelperTest.java | 149 ++++++++++++++++++++- 2 files changed, 147 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java index aaf9300e7cc8..c6f56d482d43 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java @@ -251,13 +251,13 @@ class NotificationSwipeHelper extends SwipeHelper implements NotificationSwipeAc || (isFastNonDismissGesture && isAbleToShowMenu); int menuSnapTarget = menuRow.getMenuSnapTarget(); boolean isNonFalseMenuRevealingGesture = - !isFalseGesture() && isMenuRevealingGestureAwayFromMenu; + isMenuRevealingGestureAwayFromMenu && !isFalseGesture(); if ((isNonDismissGestureTowardsMenu || isNonFalseMenuRevealingGesture) && menuSnapTarget != 0) { // Menu has not been snapped to previously and this is menu revealing gesture snapOpen(animView, menuSnapTarget, velocity); menuRow.onSnapOpen(); - } else if (isDismissGesture(ev) && !gestureTowardsMenu) { + } else if (isDismissGesture && !gestureTowardsMenu) { dismiss(animView, velocity); menuRow.onDismiss(); } else { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelperTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelperTest.java index 680a32375988..78da78269ac4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelperTest.java @@ -20,6 +20,7 @@ import static junit.framework.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyFloat; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; @@ -227,14 +228,154 @@ public class NotificationSwipeHelperTest extends SysuiTestCase { } @Test - public void testHandleUpEvent_menuRow() { - when(mSwipeHelper.getCurrentMenuRow()).thenReturn(mMenuRow); - doNothing().when(mSwipeHelper).handleMenuRowSwipe(mEvent, mView, 0, mMenuRow); + public void testHandleUpEvent_menuRowWithoutMenu_dismiss() { + doNothing().when(mSwipeHelper).dismiss(any(), anyFloat()); + doReturn(true).when(mSwipeHelper).isDismissGesture(any()); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + when(mMenuRow.shouldShowMenu()).thenReturn(true); + mSwipeHelper.setCurrentMenuRow(mMenuRow); + + assertTrue("Menu row exists", + mSwipeHelper.handleUpEvent(mEvent, mView, 0, 0)); + verify(mMenuRow, times(1)).onTouchEnd(); + verify(mSwipeHelper, times(1)).isDismissGesture(mEvent); + verify(mSwipeHelper, times(1)).dismiss(mView, 0); + verify(mSwipeHelper, never()).isFalseGesture(); + } + + @Test + public void testHandleUpEvent_menuRowWithoutMenu_snapback() { + doNothing().when(mSwipeHelper).snapChild(any(), anyInt(), anyFloat()); + doReturn(false).when(mSwipeHelper).isDismissGesture(any()); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + when(mMenuRow.shouldShowMenu()).thenReturn(true); + mSwipeHelper.setCurrentMenuRow(mMenuRow); + + assertTrue("Menu row exists", + mSwipeHelper.handleUpEvent(mEvent, mView, 0, 0)); + verify(mMenuRow, times(1)).onTouchEnd(); + verify(mSwipeHelper, times(1)).isDismissGesture(mEvent); + verify(mSwipeHelper, times(1)).snapClosed(mView, 0); + verify(mMenuRow, times(1)).onSnapClosed(); + verify(mSwipeHelper, never()).isFalseGesture(); + } + + @Test + public void testHandleUpEvent_menuRowWithOpenMenu_dismissed() { + doNothing().when(mSwipeHelper).dismiss(any(), anyFloat()); + doReturn(true).when(mSwipeHelper).isDismissGesture(any()); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + when(mMenuRow.shouldShowMenu()).thenReturn(true); + when(mMenuRow.isSnappedAndOnSameSide()).thenReturn(true); + mSwipeHelper.setCurrentMenuRow(mMenuRow); + + assertTrue("Menu row exists", + mSwipeHelper.handleUpEvent(mEvent, mView, 0, 0)); + verify(mMenuRow, times(1)).onTouchEnd(); + verify(mSwipeHelper, times(1)).isDismissGesture(mEvent); + verify(mSwipeHelper, times(1)).dismiss(mView, 0); + verify(mSwipeHelper, never()).isFalseGesture(); + } + + @Test + public void testHandleUpEvent_menuRowWithOpenMenu_snapback() { + doNothing().when(mSwipeHelper).snapChild(any(), anyInt(), anyFloat()); + doReturn(false).when(mSwipeHelper).isDismissGesture(any()); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + when(mMenuRow.shouldShowMenu()).thenReturn(true); + when(mMenuRow.isSnappedAndOnSameSide()).thenReturn(true); + mSwipeHelper.setCurrentMenuRow(mMenuRow); + + assertTrue("Menu row exists", + mSwipeHelper.handleUpEvent(mEvent, mView, 0, 0)); + verify(mMenuRow, times(1)).onTouchEnd(); + verify(mSwipeHelper, times(1)).isDismissGesture(mEvent); + verify(mSwipeHelper, times(1)).snapClosed(mView, 0); + verify(mMenuRow, times(1)).onSnapClosed(); + verify(mSwipeHelper, never()).isFalseGesture(); + } + + @Test + public void testHandleUpEvent_menuRowWithClosedMenu_dismissed() { + doNothing().when(mSwipeHelper).dismiss(any(), anyFloat()); + doReturn(true).when(mSwipeHelper).isDismissGesture(any()); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + when(mMenuRow.shouldShowMenu()).thenReturn(true); + when(mMenuRow.isSnappedAndOnSameSide()).thenReturn(false); + mSwipeHelper.setCurrentMenuRow(mMenuRow); assertTrue("Menu row exists", mSwipeHelper.handleUpEvent(mEvent, mView, 0, 0)); verify(mMenuRow, times(1)).onTouchEnd(); - verify(mSwipeHelper, times(1)).handleMenuRowSwipe(mEvent, mView, 0, mMenuRow); + verify(mSwipeHelper, times(1)).isDismissGesture(mEvent); + verify(mSwipeHelper, times(1)).dismiss(mView, 0); + verify(mSwipeHelper, never()).isFalseGesture(); + } + + @Test + public void testHandleUpEvent_menuRowWithClosedMenu_snapback() { + doNothing().when(mSwipeHelper).snapChild(any(), anyInt(), anyFloat()); + doReturn(false).when(mSwipeHelper).isDismissGesture(any()); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + when(mMenuRow.shouldShowMenu()).thenReturn(true); + when(mMenuRow.isSnappedAndOnSameSide()).thenReturn(false); + mSwipeHelper.setCurrentMenuRow(mMenuRow); + + assertTrue("Menu row exists", + mSwipeHelper.handleUpEvent(mEvent, mView, 0, 0)); + verify(mMenuRow, times(1)).onTouchEnd(); + verify(mSwipeHelper, times(1)).isDismissGesture(mEvent); + verify(mSwipeHelper, times(1)).snapClosed(mView, 0); + verify(mMenuRow, times(1)).onSnapClosed(); + verify(mSwipeHelper, never()).isFalseGesture(); + } + + @Test + public void testIsDismissGesture() { + doReturn(false).when(mSwipeHelper).isFalseGesture(); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + doReturn(true).when(mSwipeHelper).swipedFastEnough(); + when(mCallback.canChildBeDismissedInDirection(any(), anyBoolean())).thenReturn(true); + when(mEvent.getActionMasked()).thenReturn(MotionEvent.ACTION_UP); + + assertTrue("Should be a dismiss gesture", mSwipeHelper.isDismissGesture(mEvent)); + verify(mSwipeHelper, times(1)).isFalseGesture(); + } + + @Test + public void testIsDismissGesture_falseGesture() { + doReturn(true).when(mSwipeHelper).isFalseGesture(); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + doReturn(true).when(mSwipeHelper).swipedFastEnough(); + when(mCallback.canChildBeDismissedInDirection(any(), anyBoolean())).thenReturn(true); + when(mEvent.getActionMasked()).thenReturn(MotionEvent.ACTION_UP); + + assertFalse("False gesture should stop dismissal", mSwipeHelper.isDismissGesture(mEvent)); + verify(mSwipeHelper, times(1)).isFalseGesture(); + } + + @Test + public void testIsDismissGesture_farEnough() { + doReturn(false).when(mSwipeHelper).isFalseGesture(); + doReturn(true).when(mSwipeHelper).swipedFarEnough(); + doReturn(false).when(mSwipeHelper).swipedFastEnough(); + when(mCallback.canChildBeDismissedInDirection(any(), anyBoolean())).thenReturn(true); + when(mEvent.getActionMasked()).thenReturn(MotionEvent.ACTION_UP); + + assertTrue("Should be a dismissal", mSwipeHelper.isDismissGesture(mEvent)); + verify(mSwipeHelper, times(1)).isFalseGesture(); + } + + @Test + public void testIsDismissGesture_notFarOrFastEnough() { + doReturn(false).when(mSwipeHelper).isFalseGesture(); + doReturn(false).when(mSwipeHelper).swipedFarEnough(); + doReturn(false).when(mSwipeHelper).swipedFastEnough(); + when(mCallback.canChildBeDismissedInDirection(any(), anyBoolean())).thenReturn(true); + when(mEvent.getActionMasked()).thenReturn(MotionEvent.ACTION_UP); + + assertFalse("Should not be a dismissal", mSwipeHelper.isDismissGesture(mEvent)); + verify(mSwipeHelper, times(1)).isFalseGesture(); } @Test -- cgit v1.2.3-59-g8ed1b