Coalesce async touch move events until the ack back from render

content/browser/renderer_host/input/input_router_impl_unittest.cc

Index: content/browser/renderer_host/input/input_router_impl_unittest.cc
diff --git a/content/browser/renderer_host/input/input_router_impl_unittest.cc b/content/browser/renderer_host/input/input_router_impl_unittest.cc
index 83aab3819eea090c42fd97bfd83c813688d2bd5e..681d66e611e5b36538ccbca7752a9a2c29808c5d 100644
--- a/content/browser/renderer_host/input/input_router_impl_unittest.cc
+++ b/content/browser/renderer_host/input/input_router_impl_unittest.cc
@@ -155,6 +155,8 @@ class InputRouterImplTest : public testing::Test {
                                             config_));
     client_->set_input_router(input_router());
     ack_handler_->set_input_router(input_router());
+    sent_events_ids_.clear();
+    all_sent_events_ids_.clear();
   }
 
   void TearDown() override {
@@ -250,6 +252,7 @@ class InputRouterImplTest : public testing::Test {
   void SendTouchEvent() {
     input_router_->SendTouchEvent(TouchEventWithLatencyInfo(touch_event_));
     touch_event_.ResetPoints();
+    SetLastEventID();
   }
 
   int PressTouchPoint(int x, int y) {
@@ -270,10 +273,14 @@ class InputRouterImplTest : public testing::Test {
 
   void SendInputEventACK(blink::WebInputEvent::Type type,
                          InputEventAckState ack_result) {
-    InputHostMsg_HandleInputEvent_ACK_Params ack;
-    ack.type = type;
-    ack.state = ack_result;
+    InputEventAck ack(type, ack_result);
+    if (WebInputEvent::isTouchEventType(type)) {
+      DCHECK(!sent_events_ids_.empty());
+      ack.unique_touch_event_id = sent_events_ids_.front();
+      sent_events_ids_.pop_front();
+    }
     input_router_->OnMessageReceived(InputHostMsg_HandleInputEvent_ACK(0, ack));
+    SetLastEventID();
   }
 
   InputRouterImpl* input_router() const {
@@ -316,6 +323,22 @@ class InputRouterImplTest : public testing::Test {
     return count;
   }
 
+  bool TouchEventCancelable() { return touch_event_.cancelable; }
+
+  void SetLastEventID() {
+    size_t count = process_->sink().message_count();

[jdduke (slow), 2015/05/12 16:07:11]

Hmm, this probably works fine, but I wonder if it's worth being more explicit
when we ack touches. What if we changed our touch forwarding to something like:

uint32 SendTouchEvent();

and then each caller of SendTouchEvent would just store the ID and then include
that in the ack? It's a bit more work for each test, but I feel like there may
be less surprises. Then we'd have a method like:

SendTouchEventAck(type, uint32 id, result);

Looks like there are 4 or 5 tests where you might need to do this, but I think
it's probably worth doing now.

Alternatively, we could cheat and modify the touch id to always be some
constant, say, kTestTouchId, before we send it, but that probably violates the
spirit of the test :).

[lanwei, 2015/05/13 21:01:53]

I think you are right, even though we have to add the additional event_id every
time when we ack events, but it is much clearer, which event it acks.

+    for (size_t i = 0; i < count; ++i) {
+      const WebInputEvent* input_event =
+          GetInputEventFromMessage(*process_->sink().GetMessageAt(i));
+      uint32 id = WebInputEventTraits::GetUniqueTouchEventId(*input_event);
+      if (id != 0 &&
+          all_sent_events_ids_.find(id) == all_sent_events_ids_.end()) {
+        sent_events_ids_.push_back(id);
+        all_sent_events_ids_.insert(id);
+      }
+    }
+  }
+
   static void RunTasksAndWait(base::TimeDelta delay) {
     base::MessageLoop::current()->PostDelayedTask(
         FROM_HERE, base::MessageLoop::QuitClosure(), delay);
@@ -333,6 +356,8 @@ class InputRouterImplTest : public testing::Test {
   SyntheticWebTouchEvent touch_event_;
 
   scoped_ptr<TestBrowserContext> browser_context_;
+  std::deque<uint32> sent_events_ids_;
+  std::set<uint32> all_sent_events_ids_;
 };
 
 TEST_F(InputRouterImplTest, CoalescesRangeSelection) {
@@ -957,11 +982,11 @@ TEST_F(InputRouterImplTest, UnhandledWheelEvent) {
 TEST_F(InputRouterImplTest, TouchTypesIgnoringAck) {
   OnHasTouchEventHandlers(true);
   // Only acks for TouchCancel should always be ignored.
-  ASSERT_FALSE(WebInputEventTraits::IgnoresAckDisposition(
+  ASSERT_TRUE(WebInputEventTraits::WillReceiveAckFromRenderer(
       GetEventWithType(WebInputEvent::TouchStart)));
-  ASSERT_FALSE(WebInputEventTraits::IgnoresAckDisposition(
+  ASSERT_TRUE(WebInputEventTraits::WillReceiveAckFromRenderer(
       GetEventWithType(WebInputEvent::TouchMove)));
-  ASSERT_FALSE(WebInputEventTraits::IgnoresAckDisposition(
+  ASSERT_TRUE(WebInputEventTraits::WillReceiveAckFromRenderer(
       GetEventWithType(WebInputEvent::TouchEnd)));
 
   // Precede the TouchCancel with an appropriate TouchStart;
@@ -1019,7 +1044,8 @@ TEST_F(InputRouterImplTest, GestureTypesIgnoringAck) {
       WebInputEvent::GestureScrollEnd};
   for (size_t i = 0; i < arraysize(eventTypes); ++i) {
     WebInputEvent::Type type = eventTypes[i];
-    if (!WebInputEventTraits::IgnoresAckDisposition(GetEventWithType(type))) {
+    if (WebInputEventTraits::WillReceiveAckFromRenderer(
+            GetEventWithType(type))) {
       SimulateGestureEvent(type, blink::WebGestureDeviceTouchscreen);
       EXPECT_EQ(1U, GetSentMessageCountAndResetSink());
       EXPECT_EQ(0U, ack_handler_->GetAndResetAckCount());
@@ -1049,8 +1075,9 @@ TEST_F(InputRouterImplTest, MouseTypesIgnoringAck) {
   for (int i = start_type; i <= end_type; ++i) {
     WebInputEvent::Type type = static_cast<WebInputEvent::Type>(i);
     int expected_in_flight_event_count =
-        WebInputEventTraits::IgnoresAckDisposition(GetEventWithType(type)) ? 0
-                                                                           : 1;
+        !WebInputEventTraits::WillReceiveAckFromRenderer(GetEventWithType(type))
+            ? 0
+            : 1;
 
     // Note: Mouse event acks are never forwarded to the ack handler, so the key
     // result here is that ignored ack types don't affect the in-flight count.
@@ -1068,7 +1095,7 @@ TEST_F(InputRouterImplTest, MouseTypesIgnoringAck) {
 }
 
 // Guard against breaking changes to the list of ignored event ack types in
-// |WebInputEventTraits::IgnoresAckDisposition|.
+// |WebInputEventTraits::WillReceiveAckFromRenderer|.
 TEST_F(InputRouterImplTest, RequiredEventAckTypes) {
   const WebInputEvent::Type kRequiredEventAckTypes[] = {
     WebInputEvent::MouseMove,
@@ -1086,7 +1113,7 @@ TEST_F(InputRouterImplTest, RequiredEventAckTypes) {
   };
   for (size_t i = 0; i < arraysize(kRequiredEventAckTypes); ++i) {
     const WebInputEvent::Type required_ack_type = kRequiredEventAckTypes[i];
-    EXPECT_FALSE(WebInputEventTraits::IgnoresAckDisposition(
+    ASSERT_TRUE(WebInputEventTraits::WillReceiveAckFromRenderer(
         GetEventWithType(required_ack_type)));
   }
 }
@@ -1504,7 +1531,7 @@ TEST_F(InputRouterImplTest, DoubleTapGestureDependsOnFirstTap) {
                        blink::WebGestureDeviceTouchscreen);
   EXPECT_EQ(1U, GetSentMessageCountAndResetSink());
   // This test will become invalid if GestureTap stops requiring an ack.
-  ASSERT_FALSE(WebInputEventTraits::IgnoresAckDisposition(
+  ASSERT_TRUE(WebInputEventTraits::WillReceiveAckFromRenderer(
       GetEventWithType(WebInputEvent::GestureTap)));
   EXPECT_EQ(2, client_->in_flight_event_count());
   SendInputEventACK(WebInputEvent::GestureTap,
@@ -1532,7 +1559,7 @@ TEST_F(InputRouterImplTest, DoubleTapGestureDependsOnFirstTap) {
   SimulateGestureEvent(WebInputEvent::GestureDoubleTap,
                        blink::WebGestureDeviceTouchscreen);
   // This test will become invalid if GestureDoubleTap stops requiring an ack.
-  ASSERT_FALSE(WebInputEventTraits::IgnoresAckDisposition(
+  ASSERT_TRUE(WebInputEventTraits::WillReceiveAckFromRenderer(
       GetEventWithType(WebInputEvent::GestureDoubleTap)));
   EXPECT_EQ(1, client_->in_flight_event_count());
   SendInputEventACK(WebInputEvent::GestureTap, INPUT_EVENT_ACK_STATE_CONSUMED);
@@ -1730,9 +1757,8 @@ TEST_F(InputRouterImplTest, OverscrollDispatch) {
   wheel_overscroll.current_fling_velocity = gfx::Vector2dF(1, 0);
 
   SimulateWheelEvent(3, 0, 0, false);
-  InputHostMsg_HandleInputEvent_ACK_Params ack;
-  ack.type = WebInputEvent::MouseWheel;
-  ack.state = INPUT_EVENT_ACK_STATE_NOT_CONSUMED;
+  InputEventAck ack(WebInputEvent::MouseWheel,
+                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
   ack.overscroll.reset(new DidOverscrollParams(wheel_overscroll));
   input_router_->OnMessageReceived(InputHostMsg_HandleInputEvent_ACK(0, ack));