From 7860852d3b2c1a52cbfb31403951a316fceb3d2d Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Tue, 15 Dec 2020 12:23:37 -0500 Subject: [PATCH] Connect FalsingManager to HistoryTracker. With this change, the analysis of gestures actually gets added to our HistoryTracker. Prior to this, HistoryTracker was only ever being exercised in tests. The one trick that this addresses is that invalid single-taps can't immediately be added to the HistoryTracker, as they may become _valid_ double taps. We don't want double taps to be penalized. Bug: 172655679 Test: atest SystemUITests Change-Id: I2e5ece6af82eb20f053b6b17298dcd9002236e39 --- .../KeyguardPinBasedInputViewController.java | 6 + .../classifier/BrightLineFalsingManager.java | 132 +++++++++++++----- .../classifier/FalsingCollectorImpl.java | 2 +- .../classifier/FalsingDataProvider.java | 31 ++-- .../classifier/BrightLineClassifierTest.java | 86 ++++++++++-- 5 files changed, 192 insertions(+), 65 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputViewController.java index df8709c12442e..f2479488db0f8 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputViewController.java @@ -120,6 +120,12 @@ public abstract class KeyguardPinBasedInputViewController singleTapResults = mPriorResults; + mSingleTapHistoryCanceller = mDelayableExecutor.executeDelayed( + () -> { + mSingleTapHistoryCanceller = null; + mHistoryTracker.addResults(singleTapResults, completionTimeMs); + }, + mDoubleTapTimeMs); + mClassifyAsSingleTap = false; // Don't treat things as single taps by default. + } else { + mHistoryTracker.addResults(mPriorResults, completionTimeMs); + } + mPriorResults = null; + } else { + // Gestures that were not classified get treated as a false. + mHistoryTracker.addResults( + Collections.singleton( + FalsingClassifier.Result.falsed(.8, "unclassified")), + completionTimeMs); + } } }; - private boolean mPreviousResult = false; + private Collection mPriorResults; + private boolean mClassifyAsSingleTap; + private Runnable mSingleTapHistoryCanceller; @Inject public BrightLineFalsingManager(FalsingDataProvider falsingDataProvider, DockManager dockManager, MetricsLogger metricsLogger, @Named(BRIGHT_LINE_GESTURE_CLASSIFERS) Set classifiers, SingleTapClassifier singleTapClassifier, DoubleTapClassifier doubleTapClassifier, - HistoryTracker historyTracker, SystemClock systemClock, + HistoryTracker historyTracker, @Main DelayableExecutor delayableExecutor, + @Named(DOUBLE_TAP_TIMEOUT_MS) long doubleTapTimeMs, @TestHarness boolean testHarness) { mDataProvider = falsingDataProvider; mDockManager = dockManager; @@ -115,7 +141,8 @@ public class BrightLineFalsingManager implements FalsingManager { mSingleTapClassifier = singleTapClassifier; mDoubleTapClassifier = doubleTapClassifier; mHistoryTracker = historyTracker; - mSystemClock = systemClock; + mDelayableExecutor = delayableExecutor; + mDoubleTapTimeMs = doubleTapTimeMs; mTestHarness = testHarness; mDataProvider.addSessionListener(mSessionListener); @@ -129,38 +156,51 @@ public class BrightLineFalsingManager implements FalsingManager { @Override public boolean isFalseTouch(@Classifier.InteractionType int interactionType) { + boolean result; + + mClassifyAsSingleTap = false; mDataProvider.setInteractionType(interactionType); - if (!mDataProvider.isDirty()) { - return mPreviousResult; + + if (!mTestHarness && !mDataProvider.isJustUnlockedWithFace() && !mDockManager.isDocked()) { + Stream results = + mClassifiers.stream().map(falsingClassifier -> { + FalsingClassifier.Result classifierResult = + falsingClassifier.classifyGesture( + mHistoryTracker.falsePenalty(), + mHistoryTracker.falseConfidence()); + if (classifierResult.isFalse()) { + logInfo(String.format( + (Locale) null, + "{classifier=%s, interactionType=%d}", + falsingClassifier.getClass().getName(), + mDataProvider.getInteractionType())); + String reason = classifierResult.getReason(); + if (reason != null) { + logInfo(reason); + } + } else { + logDebug(falsingClassifier.getClass().getName() + ": false"); + } + return classifierResult; + }); + mPriorResults = new ArrayList<>(); + final boolean[] localResult = {false}; + results.forEach(classifierResult -> { + localResult[0] |= classifierResult.isFalse(); + mPriorResults.add(classifierResult); + }); + result = localResult[0]; + } else { + result = false; + mPriorResults = Collections.singleton(FalsingClassifier.Result.passed(1)); } - mPreviousResult = !mTestHarness - && !mDataProvider.isJustUnlockedWithFace() && !mDockManager.isDocked() - && mClassifiers.stream().anyMatch(falsingClassifier -> { - FalsingClassifier.Result result = falsingClassifier.classifyGesture( - mHistoryTracker.falsePenalty(), mHistoryTracker.falseConfidence()); - if (result.isFalse()) { - logInfo(String.format( - (Locale) null, - "{classifier=%s, interactionType=%d}", - falsingClassifier.getClass().getName(), - mDataProvider.getInteractionType())); - String reason = result.getReason(); - if (reason != null) { - logInfo(reason); - } - } else { - logDebug(falsingClassifier.getClass().getName() + ": false"); - } - return result.isFalse(); - }); - - logDebug("Is false touch? " + mPreviousResult); + logDebug("Is false touch? " + result); if (Build.IS_ENG || Build.IS_USERDEBUG) { // Copy motion events, as the passed in list gets emptied out elsewhere in the code. RECENT_SWIPES.add(new DebugSwipeRecord( - mPreviousResult, + result, mDataProvider.getInteractionType(), mDataProvider.getRecentMotionEvents().stream().map( motionEvent -> new XYDt( @@ -173,13 +213,16 @@ public class BrightLineFalsingManager implements FalsingManager { } } - return mPreviousResult; + return result; } @Override public boolean isFalseTap(boolean robustCheck) { + mClassifyAsSingleTap = true; + FalsingClassifier.Result singleTapResult = mSingleTapClassifier.isTap(mDataProvider.getRecentMotionEvents()); + mPriorResults = Collections.singleton(singleTapResult); if (singleTapResult.isFalse()) { logInfo(String.format( (Locale) null, "{classifier=%s}", mSingleTapClassifier.getClass().getName())); @@ -192,7 +235,12 @@ public class BrightLineFalsingManager implements FalsingManager { // TODO(b/172655679): More heuristics to come. For now, allow touches through if face-authed if (robustCheck) { - return !mDataProvider.isJustUnlockedWithFace(); + boolean result = !mDataProvider.isJustUnlockedWithFace(); + mPriorResults = Collections.singleton( + result ? FalsingClassifier.Result.falsed(0.1, "no face detected") + : FalsingClassifier.Result.passed(1)); + + return result; } return false; @@ -200,7 +248,9 @@ public class BrightLineFalsingManager implements FalsingManager { @Override public boolean isFalseDoubleTap() { + mClassifyAsSingleTap = false; FalsingClassifier.Result result = mDoubleTapClassifier.classifyGesture(); + mPriorResults = Collections.singleton(result); if (result.isFalse()) { logInfo(String.format( (Locale) null, "{classifier=%s}", mDoubleTapClassifier.getClass().getName())); @@ -208,6 +258,12 @@ public class BrightLineFalsingManager implements FalsingManager { if (reason != null) { logInfo(reason); } + } else { + // A valid double tap prevents an invalid single tap from going into history. + if (mSingleTapHistoryCanceller != null) { + mSingleTapHistoryCanceller.run(); + mSingleTapHistoryCanceller = null; + } } return result.isFalse(); } diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java index 75fa6f19bce05..e08b43b3521f5 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingCollectorImpl.java @@ -249,7 +249,7 @@ class FalsingCollectorImpl implements FalsingCollector { public void onTouchEvent(MotionEvent ev) { // We delay processing down events to see if another component wants to process them. // If #avoidGesture is called after a MotionEvent.ACTION_DOWN, all following motion events - //will be ignored by the collector until another MotionEvent.ACTION_DOWN is passed in. + // will be ignored by the collector until another MotionEvent.ACTION_DOWN is passed in. // avoidGesture must be called immediately following the MotionEvent.ACTION_DOWN, before // any other events are processed, otherwise the whole gesture will be recorded. if (ev.getActionMasked() == MotionEvent.ACTION_DOWN) { diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java index deb9e6dc442ca..4bacc15984905 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java @@ -90,10 +90,8 @@ public class FalsingDataProvider { } if (motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { - if (!mRecentMotionEvents.isEmpty()) { - mExtendedMotionEvents.addFirst(mRecentMotionEvents); - mRecentMotionEvents = new TimeLimitedMotionEventBuffer(MOTION_EVENT_AGE_MS); - } + completePriorGesture(); + mRecentMotionEvents = new TimeLimitedMotionEventBuffer(MOTION_EVENT_AGE_MS); } mRecentMotionEvents.addAll(motionEvents); @@ -101,9 +99,25 @@ public class FalsingDataProvider { mMotionEventListeners.forEach(listener -> listener.onMotionEvent(motionEvent)); + // We explicitly do not complete a gesture on UP or CANCEL events. + // We wait for the next gesture to start before marking the prior gesture as complete. This + // has multiple benefits. First, it makes it trivial to track the "current" or "recent" + // gesture, as it will always be found in mRecentMotionEvents. Second, and most importantly, + // it ensures that the current gesture doesn't get added to this HistoryTracker before it + // is analyzed. + mDirty = true; } + private void completePriorGesture() { + if (!mRecentMotionEvents.isEmpty()) { + mGestuerCompleteListeners.forEach(listener -> listener.onGestureComplete( + mRecentMotionEvents.get(mRecentMotionEvents.size() - 1).getEventTime())); + + mExtendedMotionEvents.addFirst(mRecentMotionEvents); + } + } + /** Returns screen width in pixels. */ public int getWidthPixels() { return mWidthPixels; @@ -146,13 +160,6 @@ public class FalsingDataProvider { } } - /** - * Returns true if new data has been supplied since the last time this class has been accessed. - */ - public boolean isDirty() { - return mDirty; - } - /** Return the interaction type that is being compared against for falsing. */ public final int getInteractionType() { return mInteractionType; @@ -387,6 +394,6 @@ public class FalsingDataProvider { /** Callback to be alerted when the current gesture ends. */ public interface GestureCompleteListener { /** */ - void onGestureComplete(); + void onGestureComplete(long completionTimeMs); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java b/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java index 8c547b127da20..5709ce3035a2e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java @@ -16,12 +16,15 @@ package com.android.systemui.classifier; -import static com.android.systemui.util.mockito.KotlinMockitoHelpersKt.any; - import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.ArgumentMatchers.anyDouble; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -35,6 +38,7 @@ import com.android.internal.logging.testing.FakeMetricsLogger; import com.android.systemui.SysuiTestCase; import com.android.systemui.classifier.FalsingDataProvider.GestureCompleteListener; import com.android.systemui.dock.DockManagerFake; +import com.android.systemui.util.concurrency.FakeExecutor; import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; @@ -45,7 +49,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.util.ArrayList; -import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -53,6 +56,8 @@ import java.util.Set; @SmallTest @RunWith(AndroidTestingRunner.class) public class BrightLineClassifierTest extends SysuiTestCase { + private static final long DOUBLE_TAP_TIMEOUT_MS = 1000; + private BrightLineFalsingManager mBrightLineFalsingManager; @Mock private FalsingDataProvider mFalsingDataProvider; @@ -69,24 +74,35 @@ public class BrightLineClassifierTest extends SysuiTestCase { private FalsingClassifier mClassifierB; private final List mMotionEventList = new ArrayList<>(); @Mock - private HistoryTracker mHistoryTracker; - private FakeSystemClock mSystemClock = new FakeSystemClock(); + private HistoryTracker mHistoryTracker;; + private final FakeExecutor mFakeExecutor = new FakeExecutor(new FakeSystemClock()); private final FalsingClassifier.Result mFalsedResult = FalsingClassifier.Result.falsed(1, ""); private final FalsingClassifier.Result mPassedResult = FalsingClassifier.Result.passed(1); + private GestureCompleteListener mGestureCompleteListener; @Before public void setup() { MockitoAnnotations.initMocks(this); when(mClassifierA.classifyGesture(anyDouble(), anyDouble())).thenReturn(mPassedResult); when(mClassifierB.classifyGesture(anyDouble(), anyDouble())).thenReturn(mPassedResult); + when(mSingleTapClassfier.isTap(any(List.class))).thenReturn(mPassedResult); + when(mDoubleTapClassifier.classifyGesture()).thenReturn(mPassedResult); mClassifiers.add(mClassifierA); mClassifiers.add(mClassifierB); - when(mFalsingDataProvider.isDirty()).thenReturn(true); when(mFalsingDataProvider.getRecentMotionEvents()).thenReturn(mMotionEventList); mBrightLineFalsingManager = new BrightLineFalsingManager(mFalsingDataProvider, mDockManager, mMetricsLogger, mClassifiers, mSingleTapClassfier, mDoubleTapClassifier, - mHistoryTracker, mSystemClock, false); + mHistoryTracker, mFakeExecutor, DOUBLE_TAP_TIMEOUT_MS, false); + + + ArgumentCaptor gestureCompleteListenerCaptor = + ArgumentCaptor.forClass(GestureCompleteListener.class); + + verify(mFalsingDataProvider).addGestureCompleteListener( + gestureCompleteListenerCaptor.capture()); + + mGestureCompleteListener = gestureCompleteListenerCaptor.getValue(); } @Test @@ -179,15 +195,57 @@ public class BrightLineClassifierTest extends SysuiTestCase { @Test public void testHistory() { - ArgumentCaptor gestureCompleteListenerCaptor = - ArgumentCaptor.forClass(GestureCompleteListener.class); + mGestureCompleteListener.onGestureComplete(1000); - verify(mFalsingDataProvider).addGestureCompleteListener( - gestureCompleteListenerCaptor.capture()); + verify(mHistoryTracker).addResults(anyCollection(), eq(1000L)); + } - GestureCompleteListener gestureCompleteListener = gestureCompleteListenerCaptor.getValue(); - gestureCompleteListener.onGestureComplete(); + @Test + public void testHistory_singleTap() { + // When trying to classify single taps, we don't immediately add results to history. + mBrightLineFalsingManager.isFalseTap(false); + mGestureCompleteListener.onGestureComplete(1000); - verify(mHistoryTracker).addResults(any(Collection.class), eq(mSystemClock.uptimeMillis())); + verify(mHistoryTracker, never()).addResults(any(), anyLong()); + + mFakeExecutor.advanceClockToNext(); + mFakeExecutor.runAllReady(); + + verify(mHistoryTracker).addResults(anyCollection(), eq(1000L)); + } + + @Test + public void testHistory_multipleSingleTaps() { + // When trying to classify single taps, we don't immediately add results to history. + mBrightLineFalsingManager.isFalseTap(false); + mGestureCompleteListener.onGestureComplete(1000); + mBrightLineFalsingManager.isFalseTap(false); + mGestureCompleteListener.onGestureComplete(2000); + + verify(mHistoryTracker, never()).addResults(any(), anyLong()); + + mFakeExecutor.advanceClockToNext(); + mFakeExecutor.runNextReady(); + verify(mHistoryTracker).addResults(anyCollection(), eq(1000L)); + reset(mHistoryTracker); + mFakeExecutor.advanceClockToNext(); + mFakeExecutor.runNextReady(); + verify(mHistoryTracker).addResults(anyCollection(), eq(2000L)); + } + + @Test + public void testHistory_doubleTap() { + // When trying to classify single taps, we don't immediately add results to history. + mBrightLineFalsingManager.isFalseTap(false); + mGestureCompleteListener.onGestureComplete(1000); + // Before checking for double tap, we may check for single-tap on the second gesture. + mBrightLineFalsingManager.isFalseTap(false); + mBrightLineFalsingManager.isFalseDoubleTap(); + mGestureCompleteListener.onGestureComplete(2000); + + // Double tap is immediately added to history. Single tap is never added. + verify(mHistoryTracker).addResults(anyCollection(), eq(2000L)); + + assertThat(mFakeExecutor.numPending()).isEqualTo(0); } }