diff options
| author | 2021-03-04 16:23:05 -0500 | |
|---|---|---|
| committer | 2021-03-04 16:43:56 -0500 | |
| commit | 7da60de0908b0024c076d27b5661d1094b4526ee (patch) | |
| tree | cb765c9c86315175de69b34b1e6a73f53ec29348 | |
| parent | 632686728c0893b3fe8afb16d2aab08405a8918d (diff) | |
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
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 64576a97ddb2..f7fe14a8e841 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<MotionEvent> secondTapEvents = getRecentMotionEvents(); - Queue<? extends List<MotionEvent>> historicalEvents = getHistoricalEvents(); - List<MotionEvent> firstTapEvents = historicalEvents.peek(); + List<MotionEvent> 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 dbfeacfa91c4..f40045b0f08e 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<? extends List<MotionEvent>> getHistoricalEvents() { - return mDataProvider.getHistoricalMotionEvents(); + List<MotionEvent> 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 4bacc1598490..336f13f117d3 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<SessionListener> mSessionListeners = new ArrayList<>(); private final List<MotionEventListener> mMotionEventListeners = new ArrayList<>(); - private final List<GestureCompleteListener> mGestuerCompleteListeners = new ArrayList<>(); + private final List<GestureCompleteListener> mGestureCompleteListeners = new ArrayList<>(); private @Classifier.InteractionType int mInteractionType; - private final Deque<TimeLimitedMotionEventBuffer> mExtendedMotionEvents = new LinkedList<>(); private TimeLimitedMotionEventBuffer mRecentMotionEvents = new TimeLimitedMotionEventBuffer(MOTION_EVENT_AGE_MS); + private List<MotionEvent> 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<? extends List<MotionEvent>> getHistoricalMotionEvents() { - long nowMs = mSystemClock.uptimeMillis(); - - mExtendedMotionEvents.removeIf( - motionEvents -> motionEvents.isFullyExpired(nowMs - EXTENDED_MOTION_EVENT_AGE_MS)); - - return mExtendedMotionEvents; + public List<MotionEvent> 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 7969b4e83ac0..e5da38936593 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<MotionEvent> { 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 472ed7a22fe3..ca0a4aa2b04d 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 17c2700c5bda..4e1742497d90 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<MotionEvent> mMotionEvents = new ArrayList<>(); - private final Deque<List<MotionEvent>> mHistoricalMotionEvents = new LinkedList<>(); + private List<MotionEvent> 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 be0cc97a2f0f..ebdda67a90d0 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 6e312594a2e4..901196f8bbba 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)); - } } |