Refactor compositor event handling path to be callback-based

ui/events/blink/input_handler_proxy.cc

Index: ui/events/blink/input_handler_proxy.cc
diff --git a/ui/events/blink/input_handler_proxy.cc b/ui/events/blink/input_handler_proxy.cc
index 60176895533d6997681d64cb3e36e581ef400f2b..4cc87649ccac7dabaa697e8ee3e9e787cadc175b 100644
--- a/ui/events/blink/input_handler_proxy.cc
+++ b/ui/events/blink/input_handler_proxy.cc
@@ -61,6 +61,8 @@ const double kMinBoostTouchScrollSpeedSquare = 150 * 150.;
 // slightly increased value to accomodate small IPC message delays.
 const double kFlingBoostTimeoutDelaySeconds = 0.05;
 
+const bool kBundleAckWithTriggeringEvent = true;

[tdresser, 2016/09/01 16:58:45]

The point of this bool is just to make this easy to revert, right? It seems like
it's adding some complexity, and I'm not sure it's worth it.

[chongz, 2016/09/01 20:13:13]

Removed.

I was just trying to avoid passing raw 'true'/'false'. I thought I read it from
code style but I might be wrong.

[tdresser, 2016/09/01 21:08:10]

Yup, that is general good practice. I'd tend to do it by making the parameter a
two valued enum type though.

+
 gfx::Vector2dF ToClientScrollIncrement(const WebFloatSize& increment) {
   return gfx::Vector2dF(-increment.width, -increment.height);
 }
@@ -246,7 +248,8 @@ InputHandlerProxy::InputHandlerProxy(cc::InputHandler* input_handler,
       has_fling_animation_started_(false),
       smooth_scroll_enabled_(false),
       uma_latency_reporting_enabled_(base::TimeTicks::IsHighResolution()),
-      touch_start_result_(kEventDispositionUndefined) {
+      touch_start_result_(kEventDispositionUndefined),
+      current_overscroll_params_(nullptr) {
   DCHECK(client);
   input_handler_->BindToClient(this);
   cc::ScrollElasticityHelper* scroll_elasticity_helper =
@@ -265,25 +268,29 @@ void InputHandlerProxy::WillShutdown() {
   client_->WillShutdown();
 }
 
-InputHandlerProxy::EventDisposition
-InputHandlerProxy::HandleInputEventWithLatencyInfo(
-    const WebInputEvent& event,
-    ui::LatencyInfo* latency_info) {
+void InputHandlerProxy::HandleInputEventWithLatencyInfo(
+    ScopedWebInputEvent event,
+    const LatencyInfo& latency_info,
+    const EventDispositionCallback& callback) {
   DCHECK(input_handler_);
 
   if (uma_latency_reporting_enabled_)
-    ReportInputEventLatencyUma(event, *latency_info);
+    ReportInputEventLatencyUma(*event, latency_info);
 
-  TRACE_EVENT_WITH_FLOW1("input,benchmark",
-                         "LatencyInfo.Flow",
-                         TRACE_ID_DONT_MANGLE(latency_info->trace_id()),
+  TRACE_EVENT_WITH_FLOW1("input,benchmark", "LatencyInfo.Flow",
+                         TRACE_ID_DONT_MANGLE(latency_info.trace_id()),
                          TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT,
                          "step", "HandleInputEventImpl");
 
+  ui::LatencyInfo monitored_latency_info = latency_info;
   std::unique_ptr<cc::SwapPromiseMonitor> latency_info_swap_promise_monitor =
-      input_handler_->CreateLatencyInfoSwapPromiseMonitor(latency_info);
-  InputHandlerProxy::EventDisposition disposition = HandleInputEvent(event);
-  return disposition;
+      input_handler_->CreateLatencyInfoSwapPromiseMonitor(
+          &monitored_latency_info);
+
+  current_overscroll_params_.reset();
+  InputHandlerProxy::EventDisposition disposition = HandleInputEvent(*event);
+  callback.Run(disposition, std::move(event), monitored_latency_info,
+               std::move(current_overscroll_params_));
 }
 
 InputHandlerProxy::EventDisposition InputHandlerProxy::HandleInputEvent(
@@ -526,7 +533,7 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::ScrollByMouseWheel(
 
         scroll_result = input_handler_->ScrollBy(&scroll_state_update);
         HandleOverscroll(gfx::Point(wheel_event.x, wheel_event.y),
-                         scroll_result);
+                         scroll_result, !kBundleAckWithTriggeringEvent);
 
         cc::ScrollStateData scroll_state_end_data;
         scroll_state_end_data.is_ending = true;
@@ -638,7 +645,7 @@ InputHandlerProxy::HandleGestureScrollUpdate(
   }
   cc::InputHandlerScrollResult scroll_result =
       input_handler_->ScrollBy(&scroll_state);
-  HandleOverscroll(scroll_point, scroll_result);
+  HandleOverscroll(scroll_point, scroll_result, kBundleAckWithTriggeringEvent);
 
   if (scroll_elasticity_controller_)
     HandleScrollElasticityOverscroll(gesture_event, scroll_result);
@@ -1103,7 +1110,8 @@ void InputHandlerProxy::SynchronouslyZoomBy(float magnify_delta,
 
 void InputHandlerProxy::HandleOverscroll(
     const gfx::Point& causal_event_viewport_point,
-    const cc::InputHandlerScrollResult& scroll_result) {
+    const cc::InputHandlerScrollResult& scroll_result,
+    bool bundle_ack_with_triggering_event) {

[tdresser, 2016/09/01 16:58:45]

Can we tweak this naming? bundle_overscroll_params_with_ack perhaps? 

[chongz, 2016/09/01 20:13:13]

Done.

   DCHECK(client_);
   if (!scroll_result.did_overscroll_root)
     return;
@@ -1125,6 +1133,20 @@ void InputHandlerProxy::HandleOverscroll(
         kFlingOverscrollThreshold;
   }
 
+  if (bundle_ack_with_triggering_event) {
+    // Bundle overscroll message with triggering event response, saving an IPC.
+    current_overscroll_params_.reset(new DidOverscrollParams());
+    current_overscroll_params_->accumulated_overscroll =
+        scroll_result.accumulated_root_overscroll;
+    current_overscroll_params_->latest_overscroll_delta =
+        scroll_result.unused_scroll_delta;
+    current_overscroll_params_->current_fling_velocity =
+        ToClientScrollIncrement(current_fling_velocity_);
+    current_overscroll_params_->causal_event_viewport_point =
+        gfx::PointF(causal_event_viewport_point);
+    return;
+  }
+
   client_->DidOverscroll(scroll_result.accumulated_root_overscroll,
                          scroll_result.unused_scroll_delta,
                          ToClientScrollIncrement(current_fling_velocity_),
@@ -1297,7 +1319,8 @@ bool InputHandlerProxy::scrollBy(const WebFloatSize& increment,
       cc::ScrollState scroll_state(scroll_state_data);
       cc::InputHandlerScrollResult scroll_result =
           input_handler_->ScrollBy(&scroll_state);
-      HandleOverscroll(fling_parameters_.point, scroll_result);
+      HandleOverscroll(fling_parameters_.point, scroll_result,
+                       !kBundleAckWithTriggeringEvent);
       did_scroll = scroll_result.did_scroll;
     } break;
     case blink::WebGestureDeviceUninitialized: