From c5755b2970d6074f630471a368a40e47b95db877 Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Thu, 1 Apr 2021 17:03:29 -0400 Subject: [PATCH] Incorporate Falsing Belief sooner. Process gestures in the FalsingManager as soon as they complete, instead of waiting for the next gesture to begin. This provides more accurate and useful feedback about falsing belief. It means that a falsing "event" will be fired as soon as the indicated threshold is crossed, instead of on the next, possibly intentional gesture. Bug: 184042853 Test: atest SystemUITests && manual Change-Id: Idd9227de3a03c52dabe31f52dbb45ff890615ded --- packages/SystemUI/docs/falsing.md | 8 +- .../systemui/classifier/FalsingCollector.java | 14 +- .../classifier/FalsingCollectorFake.java | 4 + .../classifier/FalsingCollectorImpl.java | 5 + .../classifier/FalsingDataProvider.java | 13 +- .../phone/NotificationShadeWindowView.java | 13 +- ...NotificationShadeWindowViewController.java | 5 + .../classifier/FalsingDataProviderTest.java | 167 +++++++++++------- 8 files changed, 157 insertions(+), 72 deletions(-) diff --git a/packages/SystemUI/docs/falsing.md b/packages/SystemUI/docs/falsing.md index e224ca80616ab..09215ac813b5e 100644 --- a/packages/SystemUI/docs/falsing.md +++ b/packages/SystemUI/docs/falsing.md @@ -143,10 +143,10 @@ are ready to act on the owner's action, and then query the `FalsingManager`. The will update its belief in pocket dialing based only on the last call made, so multiple calls per gesture are not well defined. -The `FalsingManager` does not update its belief in pocket-dialing until a new -gesture starts. That is to say, if the owner makes a bad tap on your feature, -the belief in pocket dialing will not incorporate this new data until the -following gesture begins. +The `FalsingManager` does not update its belief in pocket-dialing until after a gesture completes. +That is to say, if the owner makes a bad tap on your feature, the "belief" in pocket dialing will +not incorporate this new data after processing on the final `ACTION_UP` or `ACTION_CANCEL` event +occurs. If you expect a mix of taps, double taps, and swipes on your feature, segment them accordingly. Figure out which `FalsingManager` method you need to call first, rather than relying diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollector.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollector.java index bb037202d9852..3871248eccd59 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollector.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollector.java @@ -112,9 +112,21 @@ public interface FalsingCollector { /** */ void onBouncerHidden(); - /** */ + /** + * Call this to record a MotionEvent in the {@link com.android.systemui.plugins.FalsingManager}. + * + * Be sure to call {@link #onMotionEventComplete()} after the rest of SystemUI is done with the + * MotionEvent. + */ void onTouchEvent(MotionEvent ev); + /** + * Call this once SystemUI has completed all processing of a given MotionEvent. + * + * See {@link #onTouchEvent(MotionEvent)}. + */ + void onMotionEventComplete(); + /** */ void avoidGesture(); diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorFake.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorFake.java index 939b45a2b4a59..28aac051c66d0 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorFake.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorFake.java @@ -146,6 +146,10 @@ public class FalsingCollectorFake implements FalsingCollector { public void onTouchEvent(MotionEvent ev) { } + @Override + public void onMotionEventComplete() { + } + @Override public void avoidGesture() { } diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java index cf6169703dca5..aaea9ce983598 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java @@ -274,6 +274,11 @@ class FalsingCollectorImpl implements FalsingCollector { } } + @Override + public void onMotionEventComplete() { + mFalsingDataProvider.onMotionEventComplete(); + } + @Override public void avoidGesture() { mAvoidGesture = true; diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java index 1aaa139eb7cec..2e60a65c3bd10 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java @@ -81,8 +81,8 @@ public class FalsingDataProvider { } if (motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { + // Ensure prior gesture was completed. May be a no-op. completePriorGesture(); - mRecentMotionEvents = new TimeLimitedMotionEventBuffer(MOTION_EVENT_AGE_MS); } mRecentMotionEvents.addAll(motionEvents); @@ -100,12 +100,23 @@ public class FalsingDataProvider { mDirty = true; } + void onMotionEventComplete() { + if (mRecentMotionEvents.isEmpty()) { + return; + } + int action = mRecentMotionEvents.get(mRecentMotionEvents.size() - 1).getActionMasked(); + if (action == MotionEvent.ACTION_UP || action == MotionEvent.ACTION_CANCEL) { + completePriorGesture(); + } + } + private void completePriorGesture() { if (!mRecentMotionEvents.isEmpty()) { mGestureFinalizedListeners.forEach(listener -> listener.onGestureFinalized( mRecentMotionEvents.get(mRecentMotionEvents.size() - 1).getEventTime())); mPriorMotionEvents = mRecentMotionEvents; + mRecentMotionEvents = new TimeLimitedMotionEventBuffer(MOTION_EVENT_AGE_MS); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowView.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowView.java index af595b60daa12..72f169564c10d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowView.java @@ -49,7 +49,6 @@ import android.view.WindowInsets; import android.view.WindowInsetsController; import android.widget.FrameLayout; -import com.android.internal.jank.InteractionJankMonitor; import com.android.internal.view.FloatingActionMode; import com.android.internal.widget.FloatingToolbar; import com.android.systemui.R; @@ -174,7 +173,11 @@ public class NotificationShadeWindowView extends FrameLayout { public boolean dispatchTouchEvent(MotionEvent ev) { Boolean result = mInteractionEventHandler.handleDispatchTouchEvent(ev); - return result != null ? result : super.dispatchTouchEvent(ev); + result = result != null ? result : super.dispatchTouchEvent(ev); + + mInteractionEventHandler.dispatchTouchEventComplete(); + + return result; } @Override @@ -345,6 +348,12 @@ public class NotificationShadeWindowView extends FrameLayout { */ Boolean handleDispatchTouchEvent(MotionEvent ev); + /** + * Called after all dispatching is done. + */ + + void dispatchTouchEventComplete(); + /** * Returns if the view should intercept the touch event. * diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowViewController.java index 5595ae7ed820a..2ff7c9933edfc 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationShadeWindowViewController.java @@ -291,6 +291,11 @@ public class NotificationShadeWindowViewController { return null; } + @Override + public void dispatchTouchEventComplete() { + mFalsingCollector.onMotionEventComplete(); + } + @Override public boolean shouldInterceptTouchEvent(MotionEvent ev) { if (mStatusBarStateController.isDozing() && !mService.isPulsing() diff --git a/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingDataProviderTest.java b/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingDataProviderTest.java index ebdda67a90d0f..42387bcf9adc6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingDataProviderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingDataProviderTest.java @@ -16,9 +16,12 @@ package com.android.systemui.classifier; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.Matchers.closeTo; -import static org.junit.Assert.assertThat; +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import android.testing.AndroidTestingRunner; import android.util.DisplayMetrics; @@ -26,6 +29,7 @@ import android.view.MotionEvent; import androidx.test.filters.SmallTest; +import com.android.systemui.classifier.FalsingDataProvider.GestureFinalizedListener; import com.android.systemui.utils.leaks.FakeBatteryController; import org.junit.After; @@ -67,19 +71,19 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(appendUpEvent(6, 5)); List motionEventList = mDataProvider.getRecentMotionEvents(); - assertThat(motionEventList.size(), is(3)); - assertThat(motionEventList.get(0).getActionMasked(), is(MotionEvent.ACTION_DOWN)); - assertThat(motionEventList.get(1).getActionMasked(), is(MotionEvent.ACTION_MOVE)); - assertThat(motionEventList.get(2).getActionMasked(), is(MotionEvent.ACTION_UP)); - assertThat(motionEventList.get(0).getEventTime(), is(1L)); - assertThat(motionEventList.get(1).getEventTime(), is(2L)); - assertThat(motionEventList.get(2).getEventTime(), is(3L)); - assertThat(motionEventList.get(0).getX(), is(2f)); - assertThat(motionEventList.get(1).getX(), is(4f)); - assertThat(motionEventList.get(2).getX(), is(6f)); - assertThat(motionEventList.get(0).getY(), is(9f)); - assertThat(motionEventList.get(1).getY(), is(7f)); - assertThat(motionEventList.get(2).getY(), is(5f)); + assertThat(motionEventList.size()).isEqualTo(3); + assertThat(motionEventList.get(0).getActionMasked()).isEqualTo(MotionEvent.ACTION_DOWN); + assertThat(motionEventList.get(1).getActionMasked()).isEqualTo(MotionEvent.ACTION_MOVE); + assertThat(motionEventList.get(2).getActionMasked()).isEqualTo(MotionEvent.ACTION_UP); + assertThat(motionEventList.get(0).getEventTime()).isEqualTo(1L); + assertThat(motionEventList.get(1).getEventTime()).isEqualTo(2L); + assertThat(motionEventList.get(2).getEventTime()).isEqualTo(3L); + assertThat(motionEventList.get(0).getX()).isEqualTo(2f); + assertThat(motionEventList.get(1).getX()).isEqualTo(4f); + assertThat(motionEventList.get(2).getX()).isEqualTo(6f); + assertThat(motionEventList.get(0).getY()).isEqualTo(9f); + assertThat(motionEventList.get(1).getY()).isEqualTo(7f); + assertThat(motionEventList.get(2).getY()).isEqualTo(5f); } @Test @@ -88,28 +92,28 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(appendMoveEvent(4, 7, 800)); List motionEventList = mDataProvider.getRecentMotionEvents(); - assertThat(motionEventList.size(), is(2)); - assertThat(motionEventList.get(0).getActionMasked(), is(MotionEvent.ACTION_DOWN)); - assertThat(motionEventList.get(1).getActionMasked(), is(MotionEvent.ACTION_MOVE)); - assertThat(motionEventList.get(0).getEventTime(), is(1L)); - assertThat(motionEventList.get(1).getEventTime(), is(800L)); - assertThat(motionEventList.get(0).getX(), is(2f)); - assertThat(motionEventList.get(1).getX(), is(4f)); - assertThat(motionEventList.get(0).getY(), is(9f)); - assertThat(motionEventList.get(1).getY(), is(7f)); + assertThat(motionEventList.size()).isEqualTo(2); + assertThat(motionEventList.get(0).getActionMasked()).isEqualTo(MotionEvent.ACTION_DOWN); + assertThat(motionEventList.get(1).getActionMasked()).isEqualTo(MotionEvent.ACTION_MOVE); + assertThat(motionEventList.get(0).getEventTime()).isEqualTo(1L); + assertThat(motionEventList.get(1).getEventTime()).isEqualTo(800L); + assertThat(motionEventList.get(0).getX()).isEqualTo(2f); + assertThat(motionEventList.get(1).getX()).isEqualTo(4f); + assertThat(motionEventList.get(0).getY()).isEqualTo(9f); + assertThat(motionEventList.get(1).getY()).isEqualTo(7f); mDataProvider.onMotionEvent(appendUpEvent(6, 5, 1200)); // Still two events, but event a is gone. - assertThat(motionEventList.size(), is(2)); - assertThat(motionEventList.get(0).getActionMasked(), is(MotionEvent.ACTION_MOVE)); - assertThat(motionEventList.get(1).getActionMasked(), is(MotionEvent.ACTION_UP)); - assertThat(motionEventList.get(0).getEventTime(), is(800L)); - assertThat(motionEventList.get(1).getEventTime(), is(1200L)); - assertThat(motionEventList.get(0).getX(), is(4f)); - assertThat(motionEventList.get(1).getX(), is(6f)); - assertThat(motionEventList.get(0).getY(), is(7f)); - assertThat(motionEventList.get(1).getY(), is(5f)); + assertThat(motionEventList.size()).isEqualTo(2); + assertThat(motionEventList.get(0).getActionMasked()).isEqualTo(MotionEvent.ACTION_MOVE); + assertThat(motionEventList.get(1).getActionMasked()).isEqualTo(MotionEvent.ACTION_UP); + assertThat(motionEventList.get(0).getEventTime()).isEqualTo(800L); + assertThat(motionEventList.get(1).getEventTime()).isEqualTo(1200L); + assertThat(motionEventList.get(0).getX()).isEqualTo(4f); + assertThat(motionEventList.get(1).getX()).isEqualTo(6f); + assertThat(motionEventList.get(0).getY()).isEqualTo(7f); + assertThat(motionEventList.get(1).getY()).isEqualTo(5f); } @Test @@ -126,19 +130,19 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(motionEventA); List motionEventList = mDataProvider.getRecentMotionEvents(); - assertThat(motionEventList.size(), is(3)); - assertThat(motionEventList.get(0).getActionMasked(), is(MotionEvent.ACTION_MOVE)); - assertThat(motionEventList.get(1).getActionMasked(), is(MotionEvent.ACTION_MOVE)); - assertThat(motionEventList.get(2).getActionMasked(), is(MotionEvent.ACTION_MOVE)); - assertThat(motionEventList.get(0).getEventTime(), is(1L)); - assertThat(motionEventList.get(1).getEventTime(), is(2L)); - assertThat(motionEventList.get(2).getEventTime(), is(3L)); - assertThat(motionEventList.get(0).getX(), is(2f)); - assertThat(motionEventList.get(1).getX(), is(4f)); - assertThat(motionEventList.get(2).getX(), is(6f)); - assertThat(motionEventList.get(0).getY(), is(9f)); - assertThat(motionEventList.get(1).getY(), is(7f)); - assertThat(motionEventList.get(2).getY(), is(5f)); + assertThat(motionEventList.size()).isEqualTo(3); + assertThat(motionEventList.get(0).getActionMasked()).isEqualTo(MotionEvent.ACTION_MOVE); + assertThat(motionEventList.get(1).getActionMasked()).isEqualTo(MotionEvent.ACTION_MOVE); + assertThat(motionEventList.get(2).getActionMasked()).isEqualTo(MotionEvent.ACTION_MOVE); + assertThat(motionEventList.get(0).getEventTime()).isEqualTo(1L); + assertThat(motionEventList.get(1).getEventTime()).isEqualTo(2L); + assertThat(motionEventList.get(2).getEventTime()).isEqualTo(3L); + assertThat(motionEventList.get(0).getX()).isEqualTo(2f); + assertThat(motionEventList.get(1).getX()).isEqualTo(4f); + assertThat(motionEventList.get(2).getX()).isEqualTo(6f); + assertThat(motionEventList.get(0).getY()).isEqualTo(9f); + assertThat(motionEventList.get(1).getY()).isEqualTo(7f); + assertThat(motionEventList.get(2).getY()).isEqualTo(5f); } @Test @@ -147,18 +151,18 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(1, 1)); - assertThat((double) mDataProvider.getAngle(), closeTo(Math.PI / 4, .001)); + assertThat((double) mDataProvider.getAngle()).isWithin(.001).of(Math.PI / 4); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(-1, -1)); - assertThat((double) mDataProvider.getAngle(), closeTo(5 * Math.PI / 4, .001)); + assertThat((double) mDataProvider.getAngle()).isWithin(.001).of(5 * Math.PI / 4); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(2, 0)); - assertThat((double) mDataProvider.getAngle(), closeTo(0, .001)); + assertThat((double) mDataProvider.getAngle()).isWithin(.001).of(0); mDataProvider.onSessionEnd(); } @@ -168,17 +172,17 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(1, 1)); - assertThat(mDataProvider.isHorizontal(), is(false)); + assertThat(mDataProvider.isHorizontal()).isFalse(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(2, 1)); - assertThat(mDataProvider.isHorizontal(), is(true)); + assertThat(mDataProvider.isHorizontal()).isTrue(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(-3, -1)); - assertThat(mDataProvider.isHorizontal(), is(true)); + assertThat(mDataProvider.isHorizontal()).isTrue(); mDataProvider.onSessionEnd(); } @@ -188,17 +192,17 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(1, 0)); - assertThat(mDataProvider.isVertical(), is(false)); + assertThat(mDataProvider.isVertical()).isFalse(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(0, 1)); - assertThat(mDataProvider.isVertical(), is(true)); + assertThat(mDataProvider.isVertical()).isTrue(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(-3, -10)); - assertThat(mDataProvider.isVertical(), is(true)); + assertThat(mDataProvider.isVertical()).isTrue(); mDataProvider.onSessionEnd(); } @@ -208,17 +212,17 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(1, 1)); - assertThat(mDataProvider.isRight(), is(true)); + assertThat(mDataProvider.isRight()).isTrue(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(0, 1)); - assertThat(mDataProvider.isRight(), is(false)); + assertThat(mDataProvider.isRight()).isFalse(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(-3, -10)); - assertThat(mDataProvider.isRight(), is(false)); + assertThat(mDataProvider.isRight()).isFalse(); mDataProvider.onSessionEnd(); } @@ -230,25 +234,60 @@ public class FalsingDataProviderTest extends ClassifierTest { mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(1, -1)); - assertThat(mDataProvider.isUp(), is(true)); + assertThat(mDataProvider.isUp()).isTrue(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(0, 0)); - assertThat(mDataProvider.isUp(), is(false)); + assertThat(mDataProvider.isUp()).isFalse(); mDataProvider.onSessionEnd(); mDataProvider.onMotionEvent(motionEventOrigin); mDataProvider.onMotionEvent(appendMoveEvent(-3, 10)); - assertThat(mDataProvider.isUp(), is(false)); + assertThat(mDataProvider.isUp()).isFalse(); mDataProvider.onSessionEnd(); } @Test public void test_isWirelessCharging() { - assertThat(mDataProvider.isWirelessCharging(), is(false)); + assertThat(mDataProvider.isWirelessCharging()).isFalse(); mFakeBatteryController.setWirelessCharging(true); - assertThat(mDataProvider.isWirelessCharging(), is(true)); + assertThat(mDataProvider.isWirelessCharging()).isTrue(); + } + + @Test + public void test_GestureFinalizedListener() { + GestureFinalizedListener listener = mock(GestureFinalizedListener.class); + + mDataProvider.addGestureCompleteListener(listener); + + mDataProvider.onMotionEvent(appendDownEvent(0, 0)); + mDataProvider.onMotionEventComplete(); + verify(listener, never()).onGestureFinalized(anyLong()); + mDataProvider.onMotionEvent(appendMoveEvent(0, 0)); + mDataProvider.onMotionEventComplete(); + verify(listener, never()).onGestureFinalized(anyLong()); + mDataProvider.onMotionEvent(appendUpEvent(0, 0, 100)); + verify(listener, never()).onGestureFinalized(anyLong()); + + mDataProvider.onMotionEventComplete(); + verify(listener).onGestureFinalized(100); + } + + @Test + public void test_GestureFinalizedListener_SkipCompletion() { + GestureFinalizedListener listener = mock(GestureFinalizedListener.class); + + mDataProvider.addGestureCompleteListener(listener); + + mDataProvider.onMotionEvent(appendDownEvent(0, 0)); + mDataProvider.onMotionEvent(appendMoveEvent(0, 0)); + mDataProvider.onMotionEvent(appendUpEvent(0, 0, 100)); + verify(listener, never()).onGestureFinalized(anyLong()); + + // The start of a new gesture should finalized the prior one. + mDataProvider.onMotionEvent(appendDownEvent(0, 200)); + verify(listener).onGestureFinalized(100); } }