Merge 204753 "Enqueue touch events to the pending list prior to ..."

> Enqueue touch events to the pending list prior to sending to native.
> 
> The confirmTouchEvent callback is not always async, so we need to ensure
> the events are in the queue prior to sending to native to avoid an event/
> ack mismatch that will prevent all further gestures on a given tab.
> 
> BUG=246481
> 
> Review URL: https://chromiumcodereview.appspot.com/16352006

TBR=tedchoc@chromium.org

Review URL: https://codereview.chromium.org/16834002

git-svn-id: svn://svn.chromium.org/chrome/branches/1500/src@205888 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 94 insertions, 37 deletions

diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java
index 42483bd..67f6a07 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java
@@ -632,12 +632,6 @@ class ContentViewGestureHandler implements LongPressDelegate {
         mIgnoreSingleTap = value;
     }
 
-    private float calculateDragAngle(float dx, float dy) {
-        dx = Math.abs(dx);
-        dy = Math.abs(dy);
-        return (float) Math.atan2(dy, dx);
-    }
-
     private void setClickXAndY(int x, int y) {
         mSingleTapX = x;
         mSingleTapY = y;
@@ -755,21 +749,23 @@ class ContentViewGestureHandler implements LongPressDelegate {
                 return true;
             }
         }
-        int forward = EVENT_NOT_FORWARDED;
         if (mPendingMotionEvents.isEmpty()) {
-            forward = sendTouchEventToNative(event);
-        }
-        if (!mPendingMotionEvents.isEmpty() || forward != EVENT_NOT_FORWARDED) {
+            // Add the event to the pending queue prior to calling sendTouchEventToNative.
+            // When sending an event to native, the callback to confirmTouchEvent can be
+            // synchronous or asynchronous and confirmTouchEvent expects the event to be
+            // in the queue when it is called.
+            MotionEvent clone = MotionEvent.obtain(event);
+            mPendingMotionEvents.add(clone);
+
+            int forward = sendTouchEventToNative(clone);
+            if (forward == EVENT_NOT_FORWARDED) mPendingMotionEvents.remove(clone);
+            return forward != EVENT_NOT_FORWARDED;
+        } else {
             // Copy the event, as the original may get mutated after this method returns.
             MotionEvent clone = MotionEvent.obtain(event);
             mPendingMotionEvents.add(clone);
-            // If touch cancel was sent, remember the event.
-            if (forward == EVENT_CONVERTED_TO_CANCEL) {
-                mLastCancelledEvent = clone;
-            }
             return true;
         }
-        return false;
     }
 
     private int sendTouchEventToNative(MotionEvent event) {
@@ -778,19 +774,26 @@ class ContentViewGestureHandler implements LongPressDelegate {
         TouchPoint[] pts = new TouchPoint[event.getPointerCount()];
         int type = TouchPoint.createTouchPoints(event, pts);
 
-        if (type != TouchPoint.CONVERSION_ERROR) {
-            if (!mNativeScrolling && !mPinchInProgress) {
-                mTouchCancelEventSent = false;
-                if (mMotionEventDelegate.sendTouchEvent(event.getEventTime(), type, pts)) {
-                    mTouchEventTimeoutHandler.start(event.getEventTime(), pts);
-                    return EVENT_FORWARDED_TO_NATIVE;
-                }
-            } else if (!mTouchCancelEventSent) {
-                mTouchCancelEventSent = true;
-                if (mMotionEventDelegate.sendTouchEvent(event.getEventTime(),
-                        TouchPoint.TOUCH_EVENT_TYPE_CANCEL, pts)) {
-                    return EVENT_CONVERTED_TO_CANCEL;
-                }
+        if (type == TouchPoint.CONVERSION_ERROR) return EVENT_NOT_FORWARDED;
+
+        if (!mNativeScrolling && !mPinchInProgress) {
+            mTouchCancelEventSent = false;
+
+            if (mMotionEventDelegate.sendTouchEvent(event.getEventTime(), type, pts)) {
+                mTouchEventTimeoutHandler.start(event.getEventTime(), pts);
+                return EVENT_FORWARDED_TO_NATIVE;
+            }
+        } else if (!mTouchCancelEventSent) {
+            mTouchCancelEventSent = true;
+
+            MotionEvent previousCancelEvent = mLastCancelledEvent;
+            mLastCancelledEvent = event;
+
+            if (mMotionEventDelegate.sendTouchEvent(event.getEventTime(),
+                    TouchPoint.TOUCH_EVENT_TYPE_CANCEL, pts)) {
+                return EVENT_CONVERTED_TO_CANCEL;
+            } else {
+                mLastCancelledEvent = previousCancelEvent;
             }
         }
         return EVENT_NOT_FORWARDED;
@@ -842,7 +845,7 @@ class ContentViewGestureHandler implements LongPressDelegate {
 
     /**
      * Respond to a MotionEvent being returned from the native side.
-     * @param ackResult Whether the MotionEvent was consumed on the native side.
+     * @param ackResult The status acknowledgment code.
      */
     void confirmTouchEvent(int ackResult) {
         if (mTouchEventTimeoutHandler.confirmTouchEvent()) return;
@@ -903,8 +906,6 @@ class ContentViewGestureHandler implements LongPressDelegate {
             if (!mPendingMotionEvents.isEmpty()) {
                 trySendNextEventToNative(mPendingMotionEvents.peekFirst());
             }
-        } else if (forward == EVENT_CONVERTED_TO_CANCEL) {
-            mLastCancelledEvent = mPendingMotionEvents.peekFirst();
         }
     }
 
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 8627a07d..4bb96c6 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
@@ -380,10 +380,9 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase {
         mLongPressDetector = new LongPressDetector(
                 getInstrumentation().getTargetContext(), mGestureHandler);
 
-        MotionEvent event = motionEvent(MotionEvent.ACTION_DOWN, downTime, downTime);
-
         mGestureHandler.hasTouchEventHandlers(true);
 
+        MotionEvent event = motionEvent(MotionEvent.ACTION_DOWN, downTime, downTime);
         assertTrue(mGestureHandler.onTouchEvent(event));
         assertEquals(1, mGestureHandler.getNumberOfPendingMotionEventsForTesting());
         assertFalse("Should not have a pending LONG_PRESS", mLongPressDetector.hasPendingMessage());
@@ -827,14 +826,14 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase {
         MotionEvent event = motionEvent(MotionEvent.ACTION_DOWN, downTime, downTime);
         assertTrue(gestureHandler.onTouchEvent(event));
         gestureHandler.confirmTouchEvent(
-            ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+                ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
 
         event = MotionEvent.obtain(
                 downTime, downTime + 5, MotionEvent.ACTION_MOVE,
                 FAKE_COORD_X, FAKE_COORD_Y - deltaY / 2, 0);
         assertTrue(gestureHandler.onTouchEvent(event));
         gestureHandler.confirmTouchEvent(
-            ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+                ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
 
         // This event will be converted to touchcancel and put into the pending
         // queue.
@@ -844,7 +843,7 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase {
         assertTrue(gestureHandler.onTouchEvent(event));
         assertEquals(1, gestureHandler.getNumberOfPendingMotionEventsForTesting());
         assertTrue(gestureHandler.isEventCancelledForTesting(
-            gestureHandler.peekFirstInPendingMotionEventsForTesting()));
+                gestureHandler.peekFirstInPendingMotionEventsForTesting()));
 
         event = motionEvent(MotionEvent.ACTION_POINTER_DOWN, downTime + 15, downTime + 15);
         assertTrue(gestureHandler.onTouchEvent(event));
@@ -852,7 +851,7 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase {
 
         // Acking the touchcancel will drain all the events.
         gestureHandler.confirmTouchEvent(
-            ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+                ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
         assertEquals(0, gestureHandler.getNumberOfPendingMotionEventsForTesting());
     }
 
@@ -945,6 +944,63 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase {
     }
 
     /**
+     * Verifies that when hasTouchEventHandlers changes while in a gesture, that the pending
+     * queue does not grow continually.
+     */
+    @SmallTest
+    @Feature({"Gestures"})
+    public void testHasTouchEventHandlersChangesInGesture() {
+        final long downTime = SystemClock.uptimeMillis();
+        final long eventTime = SystemClock.uptimeMillis();
+
+        MockMotionEventDelegate eventDelegate = new MockMotionEventDelegate() {
+            @Override
+            public boolean sendTouchEvent(long timeMs, int action, TouchPoint[] pts) {
+                if (action == TouchPoint.TOUCH_EVENT_TYPE_CANCEL) {
+                    // Ensure the event to be cancelled is already in the pending queue.
+                    assertTrue(mGestureHandler.isEventCancelledForTesting(
+                            mGestureHandler.peekFirstInPendingMotionEventsForTesting()));
+
+                    mGestureHandler.confirmTouchEvent(
+                            ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
+                }
+                return super.sendTouchEvent(timeMs, action, pts);
+            }
+        };
+        mGestureHandler = new ContentViewGestureHandler(
+                getInstrumentation().getTargetContext(), eventDelegate,
+                new MockZoomManager(getInstrumentation().getTargetContext(), null),
+                ContentViewCore.INPUT_EVENTS_DELIVERED_AT_VSYNC);
+        mLongPressDetector = new LongPressDetector(
+                getInstrumentation().getTargetContext(), mGestureHandler);
+
+        MotionEvent event = motionEvent(MotionEvent.ACTION_DOWN, downTime, downTime);
+        assertTrue(mGestureHandler.onTouchEvent(event));
+        assertEquals(0, mGestureHandler.getNumberOfPendingMotionEventsForTesting());
+        assertFalse("Should not have a pending LONG_PRESS", mLongPressDetector.hasPendingMessage());
+
+        event = MotionEvent.obtain(
+                downTime, eventTime + 5, MotionEvent.ACTION_MOVE,
+                FAKE_COORD_X * 5, FAKE_COORD_Y * 5, 0);
+        assertTrue(mGestureHandler.onTouchEvent(event));
+        assertEquals(0, mGestureHandler.getNumberOfPendingMotionEventsForTesting());
+
+        mGestureHandler.hasTouchEventHandlers(true);
+
+        event = MotionEvent.obtain(
+                downTime, eventTime + 10, MotionEvent.ACTION_MOVE,
+                FAKE_COORD_X * 10, FAKE_COORD_Y * 10, 0);
+        assertTrue(mGestureHandler.onTouchEvent(event));
+        assertEquals(0, mGestureHandler.getNumberOfPendingMotionEventsForTesting());
+
+        event = MotionEvent.obtain(
+                downTime, eventTime + 15, MotionEvent.ACTION_MOVE,
+                FAKE_COORD_X * 15, FAKE_COORD_Y * 15, 0);
+        assertTrue(mGestureHandler.onTouchEvent(event));
+        assertEquals(0, mGestureHandler.getNumberOfPendingMotionEventsForTesting());
+    }
+
+    /**
      * Verify that LONG_TAP is triggered after LongPress followed by an UP.
      *
      * @throws Exception