Fix BaseSwipeDetector#setState() called inside another setState()
Clients of BaseSwipeDetector are required to call finishedScrolling(),
which calls setState(IDLE). An obvious place to call this is in
onDragEnd(), which itself is called from a setState(SETTLING). If the
client does this, then the SETTLING state actually clobbers the IDLE
state, leading to undefined behavior. The reason we don't see this in
practice is because we usually call finishedScrolling() after an
animation from onDragEnd() instead of calling it immediately.
To fix this, we add a simple queue such that any calls to setState()
while one is in progress have to wait and are executed in turn. This
ensures we get all the proper state callbacks and end in the correct
one.
Also fix an incorrect call in AbstractStateChangeTouchController which
was masked by this bug. We were calling setState(IDLE) in onDragStart(),
which only worked because the original setState(DRAGGING) incorrectly
clobbered this. Now we only setState(IDLE) (via finishedScrolling())
when we fully clear the state, i.e. when the interaction is finished.
Test: added testInterleavedSetState
Bug: 141939911
Change-Id: Iae630ee7101921b57a85d40646468cf19f59b674
diff --git a/src/com/android/launcher3/touch/AbstractStateChangeTouchController.java b/src/com/android/launcher3/touch/AbstractStateChangeTouchController.java
index f40f976..662838b 100644
--- a/src/com/android/launcher3/touch/AbstractStateChangeTouchController.java
+++ b/src/com/android/launcher3/touch/AbstractStateChangeTouchController.java
@@ -508,7 +508,7 @@
mAtomicComponentsController.getAnimationPlayer().end();
mAtomicComponentsController = null;
}
- cancelAnimationControllers();
+ clearState();
boolean shouldGoToTargetState = true;
if (mPendingAnimation != null) {
boolean reachedTarget = mToState == targetState;
@@ -546,13 +546,13 @@
mAtomicAnim = null;
}
mScheduleResumeAtomicComponent = false;
+ mDetector.finishedScrolling();
+ mDetector.setDetectableScrollConditions(0, false);
}
private void cancelAnimationControllers() {
mCurrentAnimation = null;
cancelAtomicComponentsController();
- mDetector.finishedScrolling();
- mDetector.setDetectableScrollConditions(0, false);
}
private void cancelAtomicComponentsController() {
diff --git a/src/com/android/launcher3/touch/BaseSwipeDetector.java b/src/com/android/launcher3/touch/BaseSwipeDetector.java
index 12ca5ee..30283da 100644
--- a/src/com/android/launcher3/touch/BaseSwipeDetector.java
+++ b/src/com/android/launcher3/touch/BaseSwipeDetector.java
@@ -24,6 +24,10 @@
import android.view.ViewConfiguration;
import androidx.annotation.NonNull;
+import androidx.annotation.VisibleForTesting;
+
+import java.util.LinkedList;
+import java.util.Queue;
/**
* Scroll/drag/swipe gesture detector.
@@ -49,13 +53,15 @@
protected final boolean mIsRtl;
protected final float mTouchSlop;
protected final float mMaxVelocity;
+ private final Queue<Runnable> mSetStateQueue = new LinkedList<>();
private int mActivePointerId = INVALID_POINTER_ID;
private VelocityTracker mVelocityTracker;
private PointF mLastDisplacement = new PointF();
private PointF mDisplacement = new PointF();
protected PointF mSubtractDisplacement = new PointF();
- private ScrollState mState = ScrollState.IDLE;
+ @VisibleForTesting ScrollState mState = ScrollState.IDLE;
+ private boolean mIsSettingState;
protected boolean mIgnoreSlopWhenSettling;
@@ -195,6 +201,12 @@
// SETTLING -> (View settled) -> IDLE
private void setState(ScrollState newState) {
+ if (mIsSettingState) {
+ mSetStateQueue.add(() -> setState(newState));
+ return;
+ }
+ mIsSettingState = true;
+
if (DBG) {
Log.d(TAG, "setState:" + mState + "->" + newState);
}
@@ -212,6 +224,10 @@
}
mState = newState;
+ mIsSettingState = false;
+ if (!mSetStateQueue.isEmpty()) {
+ mSetStateQueue.remove().run();
+ }
}
private void initializeDragging() {
diff --git a/tests/src/com/android/launcher3/touch/SingleAxisSwipeDetectorTest.java b/tests/src/com/android/launcher3/touch/SingleAxisSwipeDetectorTest.java
index 5174e4d..6d463b5 100644
--- a/tests/src/com/android/launcher3/touch/SingleAxisSwipeDetectorTest.java
+++ b/tests/src/com/android/launcher3/touch/SingleAxisSwipeDetectorTest.java
@@ -21,9 +21,11 @@
import static com.android.launcher3.touch.SingleAxisSwipeDetector.HORIZONTAL;
import static com.android.launcher3.touch.SingleAxisSwipeDetector.VERTICAL;
+import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyFloat;
import static org.mockito.Matchers.anyObject;
+import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
@@ -168,4 +170,21 @@
// TODO: actually calculate the following parameters and do exact value checks.
verify(mMockListener).onDragEnd(anyFloat());
}
+
+ @Test
+ public void testInterleavedSetState() {
+ doAnswer(invocationOnMock -> {
+ // Sets state to IDLE. (Normally onDragEnd() will have state SETTLING.)
+ mDetector.finishedScrolling();
+ return null;
+ }).when(mMockListener).onDragEnd(anyFloat());
+
+ mGenerator.put(0, 100, 100);
+ mGenerator.move(0, 100, 100 + mTouchSlop);
+ mGenerator.move(0, 100, 100 + mTouchSlop * 2);
+ mGenerator.lift(0);
+ verify(mMockListener).onDragEnd(anyFloat());
+ assertTrue("SwipeDetector should be IDLE but was " + mDetector.mState,
+ mDetector.isIdleState());
+ }
}