Coalesce async touch move events until the ack back from render

content/browser/renderer_host/input/input_router_impl.cc

 // Copyright 2013 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.
 
 #include "content/browser/renderer_host/input/input_router_impl.h"
 
 #include <math.h>
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
@@ skipping 338 matching lines @@
                                       const ui::LatencyInfo& latency_info,
                                       bool is_keyboard_shortcut) {
   output_stream_validator_.Validate(input_event);
 
   if (OfferToClient(input_event, latency_info))
     return;
 
   OfferToRenderer(input_event, latency_info, is_keyboard_shortcut);
 
   // Touch events should always indicate in the event whether they are
-  // cancelable (respect ACK disposition) or not.
-  bool ignores_ack = WebInputEventTraits::IgnoresAckDisposition(input_event);
-  if (WebInputEvent::isTouchEventType(input_event.type)) {
+  // cancelable (respect ACK disposition) or not except touchmove.
+  bool needs_synthetic_ack =
+      WebInputEventTraits::WillReceiveAckFromRenderer(input_event);

[jdduke (slow), 2015/05/07 21:11:15]

I think you're missing a "!" here.

[lanwei, 2015/05/08 19:31:25]

Not really, because we change both the function name and variable name, they
mean the same thing right now, and I revert the condition below.

+
+  if (WebInputEvent::isTouchEventType(input_event.type) &&
+      input_event.type != WebInputEvent::TouchMove) {
     const WebTouchEvent& touch = static_cast<const WebTouchEvent&>(input_event);
-    DCHECK_NE(ignores_ack, !!touch.cancelable);
+    DCHECK_EQ(needs_synthetic_ack, touch.cancelable);
   }
 
-  // If we don't care about the ack disposition, send the ack immediately.
-  if (ignores_ack) {
-    ProcessInputEventAck(input_event.type,
-                         INPUT_EVENT_ACK_STATE_IGNORED,
-                         latency_info,
-                         IGNORING_DISPOSITION);
+  // If we don't need synthetic ack from render, send the ack immediately.
+  if (!needs_synthetic_ack) {
+    InputEventAck ack(input_event.type, INPUT_EVENT_ACK_STATE_IGNORED,

[jdduke (slow), 2015/05/07 21:11:15]

Looking at this now, I can see why you wondered about why we need the new
struct. I guess it doesn't actually save us any code.

I had originally anticipated that we would send this ack throughout the pipeline
(i.e., use it instead of InputEventAckState when acking to the router's client,
and the touch queue, etc...), but maybe that refactoring should be done
separately.

+                      latency_info,
+                      WebInputEventTraits::GetUniqueTouchEventId(input_event));
+    ProcessInputEventAck(ack, IGNORING_DISPOSITION);
   }
 }
 
 bool InputRouterImpl::OfferToClient(const WebInputEvent& input_event,
                                     const ui::LatencyInfo& latency_info) {
   bool consumed = false;
 
   InputEventAckState filter_ack =
       client_->FilterInputEvent(input_event, latency_info);
   switch (filter_ack) {
     case INPUT_EVENT_ACK_STATE_CONSUMED:
-    case INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS:
+    case INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS: {
       // Send the ACK and early exit.
       next_mouse_move_.reset();
-      ProcessInputEventAck(input_event.type, filter_ack, latency_info, CLIENT);
+      InputEventAck ack(
+          input_event.type, filter_ack, latency_info,
+          WebInputEventTraits::GetUniqueTouchEventId(input_event));
+      ProcessInputEventAck(ack, CLIENT);
       // WARNING: |this| may be deleted at this point.
       consumed = true;
       break;
+    }
     case INPUT_EVENT_ACK_STATE_UNKNOWN:
       // Simply drop the event.
       consumed = true;
       break;
     default:
       break;
   }
 
   return consumed;
 }
 
 bool InputRouterImpl::OfferToRenderer(const WebInputEvent& input_event,
                                       const ui::LatencyInfo& latency_info,
                                       bool is_keyboard_shortcut) {
   if (Send(new InputMsg_HandleInputEvent(
           routing_id(), &input_event, latency_info, is_keyboard_shortcut))) {
     // Ack messages for ignored ack event types should never be sent by the
     // renderer. Consequently, such event types should not affect event time
     // or in-flight event count metrics.
-    if (!WebInputEventTraits::IgnoresAckDisposition(input_event)) {
+    if (WebInputEventTraits::WillReceiveAckFromRenderer(input_event)) {
       input_event_start_time_ = TimeTicks::Now();
       client_->IncrementInFlightEventCount();
     }
     return true;
   }
   return false;
 }
 
-void InputRouterImpl::OnInputEventAck(
-    const InputHostMsg_HandleInputEvent_ACK_Params& ack) {
+void InputRouterImpl::OnInputEventAck(const InputEventAck& ack) {
   client_->DecrementInFlightEventCount();
-
   // Log the time delta for processing an input event.
   TimeDelta delta = TimeTicks::Now() - input_event_start_time_;
   UMA_HISTOGRAM_TIMES("MPArch.IIR_InputEventDelta", delta);
 
   if (ack.overscroll) {
     DCHECK(ack.type == WebInputEvent::MouseWheel ||
            ack.type == WebInputEvent::GestureScrollUpdate);
     OnDidOverscroll(*ack.overscroll);
   }
 
-  ProcessInputEventAck(ack.type, ack.state, ack.latency, RENDERER);
-  // WARNING: |this| may be deleted at this point.
-
-  // This is used only for testing, and the other end does not use the
-  // source object.  On linux, specifying
-  // Source<RenderWidgetHost> results in a very strange
-  // runtime error in the epilogue of the enclosing
-  // (ProcessInputEventAck) method, but not on other platforms; using
-  // 'void' instead is just as safe (since NotificationSource
-  // is not actually typesafe) and avoids this error.
-  int type = static_cast<int>(ack.type);
-  NotificationService::current()->Notify(
-      NOTIFICATION_RENDER_WIDGET_HOST_DID_RECEIVE_INPUT_EVENT_ACK,
-      Source<void>(this),
-      Details<int>(&type));
+  ProcessInputEventAck(ack, RENDERER);
 }
 
 void InputRouterImpl::OnDidOverscroll(const DidOverscrollParams& params) {
   client_->DidOverscroll(params);
 }
 
 void InputRouterImpl::OnMsgMoveCaretAck() {
   move_caret_pending_ = false;
   if (next_move_caret_)
     SendMoveCaret(next_move_caret_.Pass());
@@ skipping 38 matching lines @@
   // TOUCH_ACTION_NONE should disable the touch ack timeout.
   UpdateTouchAckTimeoutEnabled();
 }
 
 void InputRouterImpl::OnDidStopFlinging() {
   // TODO(jdduke): Track fling status to allow flush notifications after a fling
   // has terminated, crbug.com/483037.
   client_->DidStopFlinging();
 }
 
-void InputRouterImpl::ProcessInputEventAck(
-    WebInputEvent::Type event_type,

[jdduke (slow), 2015/05/07 21:11:15]

For now let's keep this signature (adding a touch id argument) so we avoid
creating the temporary ack structures that I guess aren't really necessary as
we're not propagating them to the client. Maybe at some point we can switch, but
for now let's avoid the overhead.

[lanwei, 2015/05/08 19:31:25]

Done.

-    InputEventAckState ack_result,
-    const ui::LatencyInfo& latency_info,
-    AckSource ack_source) {
+void InputRouterImpl::ProcessInputEventAck(const InputEventAck& ack,
+                                           AckSource ack_source) {
+  blink::WebInputEvent::Type event_type = ack.type;
+  InputEventAckState ack_result = ack.state;
   TRACE_EVENT2("input", "InputRouterImpl::ProcessInputEventAck",
                "type", WebInputEventTraits::GetName(event_type),
                "ack", GetEventAckName(ack_result));
 
   // Note: The keyboard ack must be treated carefully, as it may result in
   // synchronous destruction of |this|. Handling immediately guards against
   // future references to |this|, as with |auto_reset_current_ack_source| below.
   if (WebInputEvent::isKeyboardEventType(event_type)) {
     ProcessKeyboardAck(event_type, ack_result);
     // WARNING: |this| may be deleted at this point.
     return;
   }
 
   base::AutoReset<AckSource> auto_reset_current_ack_source(
       &current_ack_source_, ack_source);
 
   if (WebInputEvent::isMouseEventType(event_type)) {
     ProcessMouseAck(event_type, ack_result);
   } else if (event_type == WebInputEvent::MouseWheel) {
-    ProcessWheelAck(ack_result, latency_info);
+    ProcessWheelAck(ack_result, ack.latency);
   } else if (WebInputEvent::isTouchEventType(event_type)) {
-    ProcessTouchAck(ack_result, latency_info);
+    ProcessTouchAck(ack_result, ack.latency, ack.unique_touch_event_id);
   } else if (WebInputEvent::isGestureEventType(event_type)) {
-    ProcessGestureAck(event_type, ack_result, latency_info);
+    ProcessGestureAck(event_type, ack_result, ack.latency);
   } else if (event_type != WebInputEvent::Undefined) {
     ack_handler_->OnUnexpectedEventAck(InputAckHandler::BAD_ACK_MESSAGE);
   }
 
   SignalFlushedIfNecessary();
 }
 
 void InputRouterImpl::ProcessKeyboardAck(blink::WebInputEvent::Type type,
                                          InputEventAckState ack_result) {
   if (key_queue_.empty()) {
@@ skipping 56 matching lines @@
   }
 }
 
 void InputRouterImpl::ProcessGestureAck(WebInputEvent::Type type,
                                         InputEventAckState ack_result,
                                         const ui::LatencyInfo& latency) {
   // |gesture_event_queue_| will forward to OnGestureEventAck when appropriate.
   gesture_event_queue_.ProcessGestureAck(ack_result, type, latency);
 }
 
-void InputRouterImpl::ProcessTouchAck(
-    InputEventAckState ack_result,
-    const ui::LatencyInfo& latency) {
+void InputRouterImpl::ProcessTouchAck(InputEventAckState ack_result,
+                                      const ui::LatencyInfo& latency,
+                                      const uint32 unique_touch_event_id) {

[jdduke (slow), 2015/05/07 21:11:15]

Hmm, passing by const value is rare, let's just pass a regular uint32.

[lanwei, 2015/05/08 19:31:25]

Done.

   // |touch_event_queue_| will forward to OnTouchEventAck when appropriate.
-  touch_event_queue_.ProcessTouchAck(ack_result, latency);
+  touch_event_queue_.ProcessTouchAck(ack_result, latency,
+                                     unique_touch_event_id);
 }
 
 void InputRouterImpl::UpdateTouchAckTimeoutEnabled() {
   // Mobile sites tend to be well-behaved with respect to touch handling, so
   // they have less need for the touch timeout fallback.
   const bool fixed_page_scale = (current_view_flags_ & FIXED_PAGE_SCALE) != 0;
   const bool mobile_viewport = (current_view_flags_ & MOBILE_VIEWPORT) != 0;
 
   // TOUCH_ACTION_NONE will prevent scrolling, in which case the timeout serves
   // little purpose. It's also a strong signal that touch handling is critical
@@ skipping 12 matching lines @@
     return;
 
   if (HasPendingEvents())
     return;
 
   flush_requested_ = false;
   client_->DidFlush();
 }
 
 }  // namespace content