Prevent partial touch sequences from reaching the renderer

content/browser/renderer_host/input/touch_event_queue_unittest.cc

Index: content/browser/renderer_host/input/touch_event_queue_unittest.cc
diff --git a/content/browser/renderer_host/input/touch_event_queue_unittest.cc b/content/browser/renderer_host/input/touch_event_queue_unittest.cc
index 33a26ca01ccf8ac283893da26315b8889a8e77f7..fdb994378804bbf1735c44aef1847affd5cacc18 100644
--- a/content/browser/renderer_host/input/touch_event_queue_unittest.cc
+++ b/content/browser/renderer_host/input/touch_event_queue_unittest.cc
@@ -151,9 +151,10 @@ class TouchEventQueueTest : public testing::Test,
   }
 
   bool WillForwardTouchEvents() {

[tdresser, 2014/01/17 14:33:40]

Something's wrong here...

[jdduke (slow), 2014/01/17 18:29:07]

lol, yeah this whole method needs to die... We should be testing visible state,
not internal bookkeeping.  I'll make the necessary changes.

-    return queue_->has_handlers_ &&
-           !queue_->scroll_in_progress_ &&
-           !queue_->HasTimeoutEvent();
+    return queue_->touch_handling_state_ !=
+               TouchEventQueue::NO_TOUCH_HANDLER_FOR_GESTURE &&
+           queue_->touch_handling_state_ !=
+               TouchEventQueue::NO_TOUCH_HANDLER_FOR_GESTURE;
   }
 
   bool IsTimeoutRunning() {
@@ -264,6 +265,32 @@ TEST_F(TouchEventQueueTest, QueueFlushedWhenHandlersRemoved) {
   EXPECT_EQ(31U, GetAndResetAckedEventCount());
 }
 
+// Tests that addition of a touch handler during a touch sequence will not cause
+// the remaining sequence to be forwarded.
+TEST_F(TouchEventQueueTest, ActiveSequenceNotForwardedWhenHandlersAdded) {
+  OnHasTouchEventHandlers(false);
+
+  // Send a touch-press event while there is no handler.
+  PressTouchPoint(1, 1);
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, queued_event_count());
+
+  OnHasTouchEventHandlers(true);
+
+  // The remaining touch sequence should not be forwarded.
+  MoveTouchPoint(0, 5, 5);
+  ReleaseTouchPoint(0);
+  EXPECT_EQ(2U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, queued_event_count());
+
+  // A new touch sequence should resume forwarding.
+  PressTouchPoint(1, 1);
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+}
+
 // Tests that removal of a touch handler during a touch sequence will prevent
 // the remaining sequence from being forwarded, even if another touch handler is
 // registered during the same touch sequence.
@@ -723,7 +750,7 @@ TEST_F(TouchEventQueueTest, ImmediateAckWithFollowupEvents) {
 TEST_F(TouchEventQueueTest, NoTouchBasic) {
   // Disable TouchEvent forwarding.
   OnHasTouchEventHandlers(false);
-  MoveTouchPoint(0, 30, 5);
+  PressTouchPoint(30, 5);
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
 
@@ -737,11 +764,6 @@ TEST_F(TouchEventQueueTest, NoTouchBasic) {
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
 
-  // TouchStart should not be sent to renderer.
-  PressTouchPoint(5, 5);
-  EXPECT_EQ(0U, GetAndResetSentEventCount());
-  EXPECT_EQ(1U, GetAndResetAckedEventCount());
-
   // Enable TouchEvent forwarding.
   OnHasTouchEventHandlers(true);
 
@@ -815,11 +837,12 @@ TEST_F(TouchEventQueueTest, NoTouchOnScroll) {
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, acked_event_state());
 
-  // GestureScrollEnd will resume the sending of TouchEvents to renderer.
-  SendGestureEvent(blink::WebInputEvent::GestureScrollEnd);
-  EXPECT_TRUE(WillForwardTouchEvents());
+  ReleaseTouchPoint(0);
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, acked_event_state());
 
-  // Now TouchEvents should be forwarded normally.
+  // Touch events from a new gesture sequence should be forwarded normally.
   PressTouchPoint(80, 10);
   EXPECT_EQ(1U, GetAndResetSentEventCount());
   SendTouchEventACK(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
@@ -952,7 +975,11 @@ TEST_F(TouchEventQueueTest, TouchTimeoutBasic) {
   EXPECT_EQ(1U, GetAndResetSentEventCount());
 
   // Touch events should not be forwarded until we receive the cancel acks.
-  PressTouchPoint(0, 1);
+  MoveTouchPoint(0, 1, 1);
+  ASSERT_EQ(0U, GetAndResetSentEventCount());
+  ASSERT_EQ(1U, GetAndResetAckedEventCount());
+
+  ReleaseTouchPoint(0);
   ASSERT_EQ(0U, GetAndResetSentEventCount());
   ASSERT_EQ(1U, GetAndResetAckedEventCount());
 
@@ -961,12 +988,12 @@ TEST_F(TouchEventQueueTest, TouchTimeoutBasic) {
   SendTouchEventACK(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(0U, GetAndResetAckedEventCount());
-  EXPECT_TRUE(WillForwardTouchEvents());
 
   // Subsequent events should be handled normally.
   PressTouchPoint(0, 1);
   EXPECT_EQ(1U, GetAndResetSentEventCount());
   EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_TRUE(WillForwardTouchEvents());
 }
 
 // Tests that the timeout is never started if the renderer consumes
@@ -1072,19 +1099,20 @@ TEST_F(TouchEventQueueTest, TouchTimeoutWithFollowupGesture) {
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(0U, GetAndResetAckedEventCount());
 
-  // Try to forward a touch event.
+  // Try to forward touch events for the current sequence.
   GetAndResetSentEventCount();
   GetAndResetAckedEventCount();
-  PressTouchPoint(0, 1);
+  MoveTouchPoint(0, 1, 1);
+  ReleaseTouchPoint(0);
   EXPECT_FALSE(IsTimeoutRunning());
   EXPECT_EQ(0U, GetAndResetSentEventCount());
-  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(2U, GetAndResetAckedEventCount());
 
   // Now end the scroll sequence, resuming touch handling.
   SendGestureEvent(blink::WebInputEvent::GestureScrollEnd);
-  EXPECT_TRUE(WillForwardTouchEvents());
   PressTouchPoint(0, 1);
   EXPECT_TRUE(IsTimeoutRunning());
+  EXPECT_TRUE(WillForwardTouchEvents());
   EXPECT_EQ(1U, GetAndResetSentEventCount());
   EXPECT_EQ(0U, GetAndResetAckedEventCount());
 }
@@ -1123,7 +1151,7 @@ TEST_F(TouchEventQueueTest, TouchTimeoutWithFollowupGestureAndDelayedAck) {
   // Try to forward a touch event.
   GetAndResetSentEventCount();
   GetAndResetAckedEventCount();
-  PressTouchPoint(0, 1);
+  MoveTouchPoint(0, 1, 1);
   EXPECT_FALSE(IsTimeoutRunning());
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
@@ -1131,10 +1159,11 @@ TEST_F(TouchEventQueueTest, TouchTimeoutWithFollowupGestureAndDelayedAck) {
   // Now end the scroll sequence.  Events will not be forwarded until the two
   // outstanding touch acks are received.
   SendGestureEvent(blink::WebInputEvent::GestureScrollEnd);
-  PressTouchPoint(0, 1);
+  MoveTouchPoint(0, 2, 2);
+  ReleaseTouchPoint(0);
   EXPECT_FALSE(IsTimeoutRunning());
   EXPECT_EQ(0U, GetAndResetSentEventCount());
-  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(2U, GetAndResetAckedEventCount());
 
   // Ack the original event, triggering a cancel.
   SendTouchEventACK(INPUT_EVENT_ACK_STATE_CONSUMED);
@@ -1178,20 +1207,21 @@ TEST_F(TouchEventQueueTest, NoCancelOnTouchTimeoutWithoutConsumer) {
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
 
   // Touch events should not be forwarded until we receive the original ack.
-  PressTouchPoint(0, 1);
+  MoveTouchPoint(0, 1, 1);
+  ReleaseTouchPoint(0);
   ASSERT_EQ(0U, GetAndResetSentEventCount());
-  ASSERT_EQ(1U, GetAndResetAckedEventCount());
+  ASSERT_EQ(2U, GetAndResetAckedEventCount());
 
   // Ack'ing the original event should not trigger a cancel event, as the
   // TouchStart had no consumer.  However, it should re-enable touch forwarding.
   SendTouchEventACK(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
   EXPECT_FALSE(IsTimeoutRunning());
-  EXPECT_TRUE(WillForwardTouchEvents());
   EXPECT_EQ(0U, GetAndResetAckedEventCount());
   EXPECT_EQ(0U, GetAndResetSentEventCount());
 
   // Subsequent events should be handled normally.
   PressTouchPoint(0, 1);
+  EXPECT_TRUE(WillForwardTouchEvents());
   EXPECT_EQ(1U, GetAndResetSentEventCount());
   EXPECT_EQ(0U, GetAndResetAckedEventCount());
 }