Coalesce async touch move events until the ack back from render

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 398dfb79eddc35666572087380e74238a543775f..9e9e22504ec3c5ab87c4197efe35ecb8b11fd42e 100644
--- a/content/browser/renderer_host/input/touch_event_queue_unittest.cc
+++ b/content/browser/renderer_host/input/touch_event_queue_unittest.cc
@@ -118,6 +118,10 @@ class TouchEventQueueTest : public testing::Test,
     queue_->ProcessTouchAck(ack_result, ui::LatencyInfo());
   }
 
+  void SendUncancelableTouchMoveEventAck() {
+    queue_->ProcessUncancelableTouchMoveAck();
+  }
+
   void SendGestureEventAck(WebInputEvent::Type type,
                            InputEventAckState ack_result) {
     blink::WebGestureEvent gesture_event;
@@ -271,6 +275,10 @@ class TouchEventQueueTest : public testing::Test,
     base::MessageLoop::current()->Run();
   }
 
+  int uncancelable_touch_moves_pending_ack_count() const {
+    return queue_->UncancelableTouchMovesPendingAckCount();
+  }
+
  private:
   void SendTouchEvent() {
     SendTouchEvent(touch_event_);
@@ -1634,6 +1642,7 @@ TEST_F(TouchEventQueueTest, AsyncTouchThrottledAfterScroll) {
   followup_scroll.type = WebInputEvent::GestureScrollUpdate;
   SetFollowupEvent(followup_scroll);
   SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  SendUncancelableTouchMoveEventAck();
   EXPECT_FALSE(HasPendingAsyncTouchMove());
   EXPECT_EQ(1U, GetAndResetSentEventCount());
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
@@ -1831,6 +1840,7 @@ TEST_F(TouchEventQueueTest, AsyncTouchWithAckTimeout) {
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
 
   AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);
+  SendUncancelableTouchMoveEventAck();
   MoveTouchPoint(0, 5, 5);
   EXPECT_FALSE(IsTimeoutRunning());
   EXPECT_FALSE(HasPendingAsyncTouchMove());
@@ -1971,6 +1981,292 @@ TEST_F(TouchEventQueueTest, AsyncTouchWithHardTouchStartReset) {
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
 }
 
+// Ensure that even when the interval expires, we still need to wait for the
+// ack sent back from render to send the next async touchmove once the scroll
+// starts.
+TEST_F(TouchEventQueueTest, SendNextThrottledAsyncTouchMoveAfterAck) {
+  // Process a TouchStart
+  PressTouchPoint(0, 1);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Initiate async touchmove dispatch after the start of a scroll sequence.
+  MoveTouchPoint(0, 0, 5);
+  WebGestureEvent followup_scroll;
+  followup_scroll.type = WebInputEvent::GestureScrollBegin;
+  SetFollowupEvent(followup_scroll);
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  MoveTouchPoint(0, 0, 10);
+  followup_scroll.type = WebInputEvent::GestureScrollUpdate;
+  SetFollowupEvent(followup_scroll);
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Now queue a second touchmove and verify it's not (yet) dispatched, because

[tdresser, 2015/03/31 14:44:07]

nit: a second -> another 

It's not actually the second touchmove.

[lanwei, 2015/04/01 13:14:21]

Done.

+  // we've now started scrolling, and we throttle the touchmoves.
+  MoveTouchPoint(0, 0, 20);
+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  // Even though we do not dispatch this touchmove, we still send a fake ack for
+  // this event.
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Coalesce the subsequent touchmove with pending_async_touch_move_ but don't
+  // dispatch it yet.

[tdresser, 2015/03/31 14:44:07]

I think we can make this test more focused, and move some of the logic from this
test into another test.

Testing that coalescing works should probably go in a separate test.

This test is supposed to verify that we wait for an async ack before dispatching
the pending async touch move.

Everything other than testing that should be removed if possible. I'd set the
timestamp of the first async touchmove to after the throttle limit, so the logic
is something like:
- touchstart
- touchmove (scrollbegin)
- touchmove (scrollupdate)
- async touchmove (dispatched)
- async touchmove (passed throttle timeout)
- ack first async touchmove

I think you can get rid of two events, which will make the test easier to read.

[lanwei, 2015/04/01 13:14:21]

Done.

+  MoveTouchPoint(0, 0, 30);
+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0, uncancelable_touch_moves_pending_ack_count());
+
+  // We set the next touch event time to be after the throttled interval.
+  AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);

[tdresser, 2015/03/31 14:44:07]

Oops, I completely misinterpreted what this method does. The expectations which
I got you to add below are completely unnecessary. Sorry!

I was thinking that this method advanced the global clock, not the timestamp of
future touch events.

+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+
+  // Dispatch the pending_async_touch_move_ when sufficient time has passed.
+  MoveTouchPoint(0, 0, 40);
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_FALSE(sent_event().cancelable);
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_EQ(1, uncancelable_touch_moves_pending_ack_count());
+
+  // We receive the fake ack for the above touchmove and clear the queue.
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Do not dispatch the event until throttledTouchmoves intervals expires and
+  // receive an ack from render.
+  AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);
+  MoveTouchPoint(0, 0, 50);
+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(1, uncancelable_touch_moves_pending_ack_count());
+
+  // Send pending_async_touch_move_ when we receive an ack back from render.
+  SendUncancelableTouchMoveEventAck();
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_EQ(1, uncancelable_touch_moves_pending_ack_count());
+}
+
+// Ensure that even when we receive the ack from render, we still need to wait
+// for the interval expires to send the next async touchmove once the scroll
+// starts.
+TEST_F(TouchEventQueueTest, SendNextAsyncTouchMoveAfterAckAndTimeExpire) {
+  // Process a TouchStart
+  PressTouchPoint(0, 1);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Initiate async touchmove dispatch after the start of a scroll sequence.
+  MoveTouchPoint(0, 0, 5);
+  WebGestureEvent followup_scroll;
+  followup_scroll.type = WebInputEvent::GestureScrollBegin;
+  SetFollowupEvent(followup_scroll);
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  MoveTouchPoint(0, 0, 10);
+  followup_scroll.type = WebInputEvent::GestureScrollUpdate;
+  SetFollowupEvent(followup_scroll);
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Now queue a second touchmove and verify it's not (yet) dispatched, because
+  // we've now started scrolling, and we throttle the touchmoves.
+  MoveTouchPoint(0, 0, 20);

[tdresser, 2015/03/31 14:44:07]

Same as above: I think you can get rid of this touch move and the next.

+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Coalesce the subsequent touchmove with pending_async_touch_move_ but don't
+  // dispatch it yet.
+  MoveTouchPoint(0, 0, 30);
+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0, uncancelable_touch_moves_pending_ack_count());
+
+  AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);
+  MoveTouchPoint(0, 0, 40);
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_FALSE(sent_event().cancelable);
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_EQ(1, uncancelable_touch_moves_pending_ack_count());
+
+  // We receive the fake ack for touchmove and clear the queue.
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // We receive an ack back from render but the time interval is not expired,
+  // so we do not dispatch the event.
+  SendUncancelableTouchMoveEventAck();
+  EXPECT_EQ(0, uncancelable_touch_moves_pending_ack_count());
+
+  MoveTouchPoint(0, 0, 50);
+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);
+  MoveTouchPoint(0, 0, 50);
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(WebInputEvent::TouchMove, sent_event().type);
+  EXPECT_FALSE(sent_event().cancelable);
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_EQ(1, uncancelable_touch_moves_pending_ack_count());
+}
+
+TEST_F(TouchEventQueueTest, AsyncTouchFlushedByNonTouchMove) {
+  // Process a TouchStart
+  PressTouchPoint(0, 1);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Initiate async touchmove dispatch after the start of a scroll sequence.
+  MoveTouchPoint(0, 0, 5);
+  WebGestureEvent followup_scroll;
+  followup_scroll.type = WebInputEvent::GestureScrollBegin;
+  SetFollowupEvent(followup_scroll);
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  MoveTouchPoint(0, 0, 10);
+  followup_scroll.type = WebInputEvent::GestureScrollUpdate;
+  SetFollowupEvent(followup_scroll);
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Now queue a second touchmove and verify it's not (yet) dispatched, because
+  // we've now started scrolling, and we throttle the touchmoves.
+  MoveTouchPoint(0, 0, 20);

[tdresser, 2015/03/31 14:44:07]

We should be able to get rid of this touchmove.

+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0, uncancelable_touch_moves_pending_ack_count());
+
+  AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);
+  MoveTouchPoint(0, 0, 40);
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_FALSE(sent_event().cancelable);
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_EQ(1, uncancelable_touch_moves_pending_ack_count());
+
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  for (int i = 0; i < 3; ++i) {
+    MoveTouchPoint(0, 10 + 10 * i, 10 + 10 * i);
+    EXPECT_TRUE(HasPendingAsyncTouchMove());
+    EXPECT_EQ(0U, queued_event_count());
+    EXPECT_EQ(0U, GetAndResetSentEventCount());
+    EXPECT_EQ(1U, GetAndResetAckedEventCount());
+    EXPECT_EQ(i + 1, uncancelable_touch_moves_pending_ack_count());
+
+    // Send touchstart will flush pending_async_touch_move_, and increase the
+    // count.
+    PressTouchPoint(30, 30);
+    EXPECT_FALSE(HasPendingAsyncTouchMove());
+    EXPECT_EQ(WebInputEvent::TouchMove, sent_event().type);
+    EXPECT_FALSE(sent_event().cancelable);
+    EXPECT_EQ(10 + 10 * i, sent_event().touches[0].position.x);
+    EXPECT_EQ(10 + 10 * i, sent_event().touches[0].position.y);
+    EXPECT_EQ(1U, GetAndResetSentEventCount());
+    EXPECT_EQ(i + 2, uncancelable_touch_moves_pending_ack_count());
+
+    // Touchstart is cancelable during async touch dispatch.
+    SendTouchEventAck(INPUT_EVENT_ACK_STATE_IGNORED);
+    EXPECT_EQ(WebInputEvent::TouchStart, sent_event().type);
+    EXPECT_TRUE(sent_event().cancelable);
+    EXPECT_EQ(1U, GetAndResetSentEventCount());
+    EXPECT_EQ(0U, GetAndResetAckedEventCount());
+    EXPECT_EQ(i + 2, uncancelable_touch_moves_pending_ack_count());
+
+    SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+    EXPECT_EQ(0U, queued_event_count());
+    EXPECT_FALSE(HasPendingAsyncTouchMove());
+    EXPECT_EQ(0U, GetAndResetSentEventCount());
+    EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  }
+
+  EXPECT_EQ(4, uncancelable_touch_moves_pending_ack_count());
+  // When we receive an ack from render we decrease the count.
+  SendUncancelableTouchMoveEventAck();
+  EXPECT_EQ(3, uncancelable_touch_moves_pending_ack_count());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+
+  // Do not dispatch the next uncancelable touchmove when we have not recevied
+  // all the acks back from render.
+  AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);
+  MoveTouchPoint(0, 20, 30);
+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(3, uncancelable_touch_moves_pending_ack_count());
+
+  // Once we receive the ack from render, we do not dispatch the
+  // pending_async_touchmove_ until the count is 0.
+  SendUncancelableTouchMoveEventAck();
+  EXPECT_EQ(2, uncancelable_touch_moves_pending_ack_count());
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_TRUE(HasPendingAsyncTouchMove());
+
+  SendUncancelableTouchMoveEventAck();
+
+  // When we receive this ack from render, and the count is 0, so we can
+  // dispatch the pending_async_touchmove_.
+  SendUncancelableTouchMoveEventAck();
+  EXPECT_EQ(1, uncancelable_touch_moves_pending_ack_count());
+  EXPECT_FALSE(HasPendingAsyncTouchMove());
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(WebInputEvent::TouchMove, sent_event().type);
+  EXPECT_FALSE(sent_event().cancelable);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+}
+
 TEST_F(TouchEventQueueTest, TouchAbsorptionWithConsumedFirstMove) {
   // Queue a TouchStart.
   PressTouchPoint(0, 1);
@@ -2059,6 +2355,7 @@ TEST_F(TouchEventQueueTest, TouchStartCancelableDuringScroll) {
   EXPECT_TRUE(sent_event().cancelable);
   AdvanceTouchTime(kMinSecondsBetweenThrottledTouchmoves + 0.1);
   MoveTouchPoint(0, 30, 5);
+  SendUncancelableTouchMoveEventAck();
   ASSERT_EQ(1U, GetAndResetSentEventCount());
   SendTouchEventAck(INPUT_EVENT_ACK_STATE_IGNORED);
   EXPECT_FALSE(sent_event().cancelable);