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
This commit is contained in:
@@ -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();
|
||||
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<>();
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user