[Android] Properly handle cancelled scroll events

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.content.browser;
 
 import android.content.Context;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.SystemClock;
@@ skipping 578 matching lines @@
                 downTime, eventTime + 10, MotionEvent.ACTION_UP,
                 FAKE_COORD_X * 10, FAKE_COORD_Y * 10, 0);
         assertTrue(mGestureHandler.onTouchEvent(event));
 
         // Synchronous, no need to wait.
         assertTrue("Should have a fling", mMockListener.mLastFling1 != null);
         assertTrue("Should not have a long press", mMockListener.mLastLongPress == null);
     }
 
     /**
+     * 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
      * and GESTURE_FLING_CANCEL is sent on the next touch.
      * @throws Exception
      */
     @SmallTest
     @Feature({"Gestures"})
     public void testFlingEventSequence() throws Exception {
         final long downTime = SystemClock.uptimeMillis();
@@ skipping 1094 matching lines @@
 
         // If events are delivered at vsync, multiple SCROLL_BY and PINCH_BY events should still
         // trigger only a single vsync from any given call to confirmTouchEvent().
         if (inputEventsDeliveredAtVSync) {
             assertEquals(1, mMockMotionEventDelegate.mTotalSentLastGestureForVSyncCount);
         } else {
             assertEquals(0, mMockMotionEventDelegate.mTotalSentLastGestureForVSyncCount);
         }
    }
 }