[Android] Properly handle cancelled scroll events

content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java

Index: content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java
diff --git a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java
index a05863fab915ebdce55cbfd55c00b9e47fede5c0..4aeb74f1a0a88fea8ec8139b0d379b9434448f6f 100644
--- a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java
+++ b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java
@@ -596,6 +596,77 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase {
     }
 
     /**
+     * Verify that for a normal scroll the following events are sent:
+     * - GESTURE_SCROLL_START
+     * - GESTURE_SCROLL_BY
+     * - GESTURE_SCROLL_END
+     * @throws Exception
+     */
+    @SmallTest
+    @Feature({"Gestures"})
+    public void testScrollEventSequence() throws Exception {

[Ted C, 2013/10/14 20:05:38]

should specify this as action_up in the name to be consistent with cancelled
below

[cjhopman, 2013/10/14 20:37:09]

Done.

+        doScrollEventSequence(MotionEvent.ACTION_UP);
+    }
+
+    /**
+     * Verify that for a cancelled scroll the following events are sent:
+     * - GESTURE_SCROLL_START
+     * - GESTURE_SCROLL_BY
+     * - GESTURE_SCROLL_END
+     * @throws Exception
+     */
+    @SmallTest
+    @Feature({"Gestures"})
+    public void testScrollEventCancelledSequence() throws Exception {
+        doScrollEventSequence(MotionEvent.ACTION_CANCEL);
+    }
+

[Ted C, 2013/10/14 20:05:38]

remove extra blank line

[cjhopman, 2013/10/14 20:37:09]

Done.

+
+    private void doScrollEventSequence(int endActionType) throws Exception {

[Ted C, 2013/10/14 20:05:38]

The name of this method seems somewhat unclear to me.  I think something like:

assertScrollEndSentForEndingActionType

Just so it doesn't look like some generic scroll method.

[cjhopman, 2013/10/14 20:37:09]

How's this?

+        final long downTime = SystemClock.uptimeMillis();
+        final long eventTime = SystemClock.uptimeMillis();
+        final int scrollToX = FAKE_COORD_X + 100;
+        final int scrollToY = FAKE_COORD_Y + 100;
+
+        GestureRecordingMotionEventDelegate mockDelegate =
+                new GestureRecordingMotionEventDelegate();
+        mGestureHandler = new ContentViewGestureHandler(
+                getInstrumentation().getTargetContext(), mockDelegate, mMockZoomManager,
+                ContentViewCore.INPUT_EVENTS_DELIVERED_AT_VSYNC);
+
+        MotionEvent event = motionEvent(MotionEvent.ACTION_DOWN, downTime, downTime);
+
+        assertTrue(mGestureHandler.onTouchEvent(event));
+
+        event = MotionEvent.obtain(
+                downTime, eventTime + 100, MotionEvent.ACTION_MOVE, scrollToX, scrollToY, 0);
+        assertTrue(mGestureHandler.onTouchEvent(event));
+        assertTrue(mGestureHandler.isNativeScrolling());
+        assertTrue("A scrollStart event should have been sent",
+                mockDelegate.mGestureTypeList.contains(
+                        ContentViewGestureHandler.GESTURE_SCROLL_START));
+        assertEquals("We should have started scrolling",
+                ContentViewGestureHandler.GESTURE_SCROLL_BY,
+                mockDelegate.mMostRecentGestureEvent.mType);
+        assertEquals("Only scrollBegin and scrollBy should have been sent",
+                2, mockDelegate.mGestureTypeList.size());
+
+        event = MotionEvent.obtain(
+                downTime, eventTime + 100, endActionType, scrollToX, scrollToY, 0);

[Ted C, 2013/10/14 20:05:38]

Although it probably doesn't matter, but should you be incrementing the event
time by something larger than what you do for ACTION_MOVE above?

[cjhopman, 2013/10/14 20:37:09]

It's pretty common that the framework sends a final ACTION_MOVE before the
ACTION_UP with the same time, so this should be fine.

+        assertFalse(mGestureHandler.onTouchEvent(event));
+        assertFalse(mGestureHandler.isNativeScrolling());
+        assertTrue("A scrollEnd event should have been sent",
+                mockDelegate.mGestureTypeList.contains(
+                        ContentViewGestureHandler.GESTURE_SCROLL_END));
+        assertEquals("We should have stopped scrolling",
+                ContentViewGestureHandler.GESTURE_SCROLL_END,
+                mockDelegate.mMostRecentGestureEvent.mType);
+        assertEquals("Only scrollBegin and scrollBy and scrollEnd should have been sent",
+                3, mockDelegate.mGestureTypeList.size());
+    }
+

[Ted C, 2013/10/14 20:05:38]

remove extra blank line

[cjhopman, 2013/10/14 20:37:09]

Done.

+
+    /**
      * Verify that for a normal fling (fling after scroll) the following events are sent:
      * - GESTURE_SCROLL_BEGIN
      * - GESTURE_FLING_START