From 7da60de0908b0024c076d27b5661d1094b4526ee Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Thu, 4 Mar 2021 16:23:05 -0500 Subject: [PATCH] Fix unbounded MotionEvent growth in Falsing We were keeping an unbounded list of MotionEvents inside of the FalsingManager. Generally, this list should have shrunk itself periodically, but in practice it only shrank itself when someone double taps a notification. Meanwhile, we weren't even using the data in the list! With this change, we only keep the current and prior list of MotionEvents - the ones we actually use. The list of historical events is removed, so the leak goes with it. Fixes: 177329773 Test: atest SystemUITests && manual Change-Id: If8bfeea944850d972434915018b5bbfb3acb80e4 --- .../classifier/DoubleTapClassifier.java | 4 +-- .../classifier/FalsingClassifier.java | 5 ++- .../classifier/FalsingDataProvider.java | 33 ++++++------------- .../TimeLimitedMotionEventBuffer.java | 12 ------- .../systemui/classifier/ClassifierTest.java | 4 +-- .../classifier/DoubleTapClassifierTest.java | 12 +++---- .../classifier/FalsingDataProviderTest.java | 4 +-- .../TimeLimitedMotionEventBufferTest.java | 16 --------- 8 files changed, 19 insertions(+), 71 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/classifier/DoubleTapClassifier.java b/packages/SystemUI/src/com/android/systemui/classifier/DoubleTapClassifier.java index 64576a97ddb22..f7fe14a8e8412 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/DoubleTapClassifier.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/DoubleTapClassifier.java @@ -22,7 +22,6 @@ import static com.android.systemui.classifier.FalsingModule.DOUBLE_TAP_TOUCH_SLO import android.view.MotionEvent; import java.util.List; -import java.util.Queue; import javax.inject.Inject; import javax.inject.Named; @@ -49,8 +48,7 @@ public class DoubleTapClassifier extends FalsingClassifier { @Override Result calculateFalsingResult(double historyPenalty, double historyConfidence) { List secondTapEvents = getRecentMotionEvents(); - Queue> historicalEvents = getHistoricalEvents(); - List firstTapEvents = historicalEvents.peek(); + List firstTapEvents = getPriorMotionEvents(); StringBuilder reason = new StringBuilder(); diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingClassifier.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingClassifier.java index dbfeacfa91c46..f40045b0f08eb 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingClassifier.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingClassifier.java @@ -21,7 +21,6 @@ import android.view.MotionEvent; import com.android.systemui.util.sensors.ProximitySensor; import java.util.List; -import java.util.Queue; /** * Base class for rules that determine False touches. @@ -40,8 +39,8 @@ public abstract class FalsingClassifier { return mDataProvider.getRecentMotionEvents(); } - Queue> getHistoricalEvents() { - return mDataProvider.getHistoricalMotionEvents(); + List getPriorMotionEvents() { + return mDataProvider.getPriorMotionEvents(); } MotionEvent getFirstMotionEvent() { diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java index 4bacc15984905..336f13f117d3a 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingDataProvider.java @@ -23,13 +23,9 @@ import android.view.MotionEvent.PointerProperties; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.statusbar.policy.BatteryController; -import com.android.systemui.util.time.SystemClock; import java.util.ArrayList; -import java.util.Deque; -import java.util.LinkedList; import java.util.List; -import java.util.Queue; import javax.inject.Inject; @@ -40,24 +36,23 @@ import javax.inject.Inject; public class FalsingDataProvider { private static final long MOTION_EVENT_AGE_MS = 1000; - private static final long EXTENDED_MOTION_EVENT_AGE_MS = 30 * 1000; private static final float THREE_HUNDRED_SIXTY_DEG = (float) (2 * Math.PI); private final int mWidthPixels; private final int mHeightPixels; private final BatteryController mBatteryController; - private final SystemClock mSystemClock; private final float mXdpi; private final float mYdpi; private final List mSessionListeners = new ArrayList<>(); private final List mMotionEventListeners = new ArrayList<>(); - private final List mGestuerCompleteListeners = new ArrayList<>(); + private final List mGestureCompleteListeners = new ArrayList<>(); private @Classifier.InteractionType int mInteractionType; - private final Deque mExtendedMotionEvents = new LinkedList<>(); private TimeLimitedMotionEventBuffer mRecentMotionEvents = new TimeLimitedMotionEventBuffer(MOTION_EVENT_AGE_MS); + private List mPriorMotionEvents; + private boolean mDirty = true; private float mAngle = 0; @@ -66,14 +61,12 @@ public class FalsingDataProvider { private boolean mJustUnlockedWithFace; @Inject - public FalsingDataProvider(DisplayMetrics displayMetrics, BatteryController batteryController, - SystemClock systemClock) { + public FalsingDataProvider(DisplayMetrics displayMetrics, BatteryController batteryController) { mXdpi = displayMetrics.xdpi; mYdpi = displayMetrics.ydpi; mWidthPixels = displayMetrics.widthPixels; mHeightPixels = displayMetrics.heightPixels; mBatteryController = batteryController; - mSystemClock = systemClock; FalsingClassifier.logInfo("xdpi, ydpi: " + getXdpi() + ", " + getYdpi()); FalsingClassifier.logInfo("width, height: " + getWidthPixels() + ", " + getHeightPixels()); @@ -111,10 +104,10 @@ public class FalsingDataProvider { private void completePriorGesture() { if (!mRecentMotionEvents.isEmpty()) { - mGestuerCompleteListeners.forEach(listener -> listener.onGestureComplete( + mGestureCompleteListeners.forEach(listener -> listener.onGestureComplete( mRecentMotionEvents.get(mRecentMotionEvents.size() - 1).getEventTime())); - mExtendedMotionEvents.addFirst(mRecentMotionEvents); + mPriorMotionEvents = mRecentMotionEvents; } } @@ -140,14 +133,8 @@ public class FalsingDataProvider { return mRecentMotionEvents; } - /** Returns recent gestures, exclusive of the most recent gesture. Newer gestures come first. */ - public Queue> getHistoricalMotionEvents() { - long nowMs = mSystemClock.uptimeMillis(); - - mExtendedMotionEvents.removeIf( - motionEvents -> motionEvents.isFullyExpired(nowMs - EXTENDED_MOTION_EVENT_AGE_MS)); - - return mExtendedMotionEvents; + public List getPriorMotionEvents() { + return mPriorMotionEvents; } /** @@ -344,12 +331,12 @@ public class FalsingDataProvider { /** Register a {@link GestureCompleteListener}. */ public void addGestureCompleteListener(GestureCompleteListener listener) { - mGestuerCompleteListeners.add(listener); + mGestureCompleteListeners.add(listener); } /** Unregister a {@link GestureCompleteListener}. */ public void removeGestureCompleteListener(GestureCompleteListener listener) { - mGestuerCompleteListeners.remove(listener); + mGestureCompleteListeners.remove(listener); } void onSessionStarted() { diff --git a/packages/SystemUI/src/com/android/systemui/classifier/TimeLimitedMotionEventBuffer.java b/packages/SystemUI/src/com/android/systemui/classifier/TimeLimitedMotionEventBuffer.java index 7969b4e83ac09..e5da389365933 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/TimeLimitedMotionEventBuffer.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/TimeLimitedMotionEventBuffer.java @@ -42,18 +42,6 @@ public class TimeLimitedMotionEventBuffer implements List { mMotionEvents = new LinkedList<>(); } - /** - * Returns true if the most recent event in the buffer is past the expiration time. - * - * This method does not mutate the underlying data. This method does imply that, if the supplied - * expiration time is old enough and a new {@link MotionEvent} gets added to the buffer, all - * prior events would be removed. - */ - public boolean isFullyExpired(long expirationMs) { - return mMotionEvents.isEmpty() - || mMotionEvents.getLast().getEventTime() <= expirationMs; - } - private void ejectOldEvents() { if (mMotionEvents.isEmpty()) { return; diff --git a/packages/SystemUI/tests/src/com/android/systemui/classifier/ClassifierTest.java b/packages/SystemUI/tests/src/com/android/systemui/classifier/ClassifierTest.java index 472ed7a22fe31..ca0a4aa2b04d5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/classifier/ClassifierTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/ClassifierTest.java @@ -21,7 +21,6 @@ import static com.android.systemui.classifier.Classifier.UNLOCK; import android.util.DisplayMetrics; import android.view.MotionEvent; -import com.android.systemui.util.time.FakeSystemClock; import com.android.systemui.utils.leaks.FakeBatteryController; import com.android.systemui.utils.leaks.LeakCheckedTest; @@ -45,8 +44,7 @@ public class ClassifierTest extends LeakCheckedTest { displayMetrics.widthPixels = 1000; displayMetrics.heightPixels = 1000; mFakeBatteryController = new FakeBatteryController(getLeakCheck()); - mDataProvider = new FalsingDataProvider(displayMetrics, mFakeBatteryController, - new FakeSystemClock()); + mDataProvider = new FalsingDataProvider(displayMetrics, mFakeBatteryController); mDataProvider.setInteractionType(UNLOCK); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/classifier/DoubleTapClassifierTest.java b/packages/SystemUI/tests/src/com/android/systemui/classifier/DoubleTapClassifierTest.java index 17c2700c5bdac..4e1742497d909 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/classifier/DoubleTapClassifierTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/DoubleTapClassifierTest.java @@ -19,7 +19,6 @@ package com.android.systemui.classifier; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; import android.testing.AndroidTestingRunner; @@ -35,8 +34,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.util.ArrayList; -import java.util.Deque; -import java.util.LinkedList; import java.util.List; @SmallTest @@ -47,7 +44,7 @@ public class DoubleTapClassifierTest extends ClassifierTest { private static final long DOUBLE_TAP_TIMEOUT_MS = 100; private List mMotionEvents = new ArrayList<>(); - private final Deque> mHistoricalMotionEvents = new LinkedList<>(); + private List mPriorMotionEvents = new ArrayList<>(); @Mock private FalsingDataProvider mDataProvider; @@ -64,7 +61,7 @@ public class DoubleTapClassifierTest extends ClassifierTest { MockitoAnnotations.initMocks(this); mClassifier = new DoubleTapClassifier(mDataProvider, mSingleTapClassifier, TOUCH_SLOP, DOUBLE_TAP_TIMEOUT_MS); - doReturn(mHistoricalMotionEvents).when(mDataProvider).getHistoricalMotionEvents(); + when(mDataProvider.getPriorMotionEvents()).thenReturn(mPriorMotionEvents); } @After @@ -177,9 +174,8 @@ public class DoubleTapClassifierTest extends ClassifierTest { } private void archiveMotionEvents() { - mHistoricalMotionEvents.addFirst(mMotionEvents); - doReturn(mHistoricalMotionEvents).when(mDataProvider).getHistoricalMotionEvents(); + mPriorMotionEvents = mMotionEvents; + when(mDataProvider.getPriorMotionEvents()).thenReturn(mPriorMotionEvents); mMotionEvents = new ArrayList<>(); - } } 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 be0cc97a2f0fd..ebdda67a90d0f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingDataProviderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingDataProviderTest.java @@ -26,7 +26,6 @@ import android.view.MotionEvent; import androidx.test.filters.SmallTest; -import com.android.systemui.util.time.FakeSystemClock; import com.android.systemui.utils.leaks.FakeBatteryController; import org.junit.After; @@ -52,8 +51,7 @@ public class FalsingDataProviderTest extends ClassifierTest { displayMetrics.ydpi = 100; displayMetrics.widthPixels = 1000; displayMetrics.heightPixels = 1000; - mDataProvider = new FalsingDataProvider(displayMetrics, mFakeBatteryController, - new FakeSystemClock()); + mDataProvider = new FalsingDataProvider(displayMetrics, mFakeBatteryController); } @After diff --git a/packages/SystemUI/tests/src/com/android/systemui/classifier/TimeLimitedMotionEventBufferTest.java b/packages/SystemUI/tests/src/com/android/systemui/classifier/TimeLimitedMotionEventBufferTest.java index 6e312594a2e44..901196f8bbba9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/classifier/TimeLimitedMotionEventBufferTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/TimeLimitedMotionEventBufferTest.java @@ -89,20 +89,4 @@ public class TimeLimitedMotionEventBufferTest extends SysuiTestCase { assertThat(mBuffer.get(0), is(eventC)); assertThat(mBuffer.get(1), is(eventD)); } - - @Test - public void testFullyExpired() { - MotionEvent eventA = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 0, 0, 0); - MotionEvent eventB = MotionEvent.obtain(0, 1, MotionEvent.ACTION_MOVE, 0, 0, 0); - MotionEvent eventC = MotionEvent.obtain(0, 2, MotionEvent.ACTION_MOVE, 0, 0, 0); - MotionEvent eventD = MotionEvent.obtain(0, 3, MotionEvent.ACTION_UP, 0, 0, 0); - - mBuffer.add(eventA); - mBuffer.add(eventB); - mBuffer.add(eventC); - mBuffer.add(eventD); - - assertThat(mBuffer.isFullyExpired(2), is(false)); - assertThat(mBuffer.isFullyExpired(6), is(true)); - } }