Refactor compositor event handling path to be callback-based

content/renderer/mus/compositor_mus_connection.cc

Index: content/renderer/mus/compositor_mus_connection.cc
diff --git a/content/renderer/mus/compositor_mus_connection.cc b/content/renderer/mus/compositor_mus_connection.cc
index 6c3d2783abee1bdb204bcc1711be6b002c5bab08..2b3c5fdd6de5b30c01ff5f171f25a88767b9426d 100644
--- a/content/renderer/mus/compositor_mus_connection.cc
+++ b/content/renderer/mus/compositor_mus_connection.cc
@@ -78,7 +78,7 @@ void CompositorMusConnection::OnConnectionLostOnMainThread() {
 }
 
 void CompositorMusConnection::OnWindowInputEventOnMainThread(
-    std::unique_ptr<blink::WebInputEvent> web_event,
+    ui::ScopedWebInputEvent web_event,
     const base::Callback<void(EventResult)>& ack) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   RenderWidgetMusConnection* connection =
@@ -125,17 +125,36 @@ void CompositorMusConnection::OnWindowInputEvent(
     const ui::Event& event,
     std::unique_ptr<base::Callback<void(EventResult)>>* ack_callback) {
   DCHECK(compositor_task_runner_->BelongsToCurrentThread());
-  std::unique_ptr<blink::WebInputEvent> web_event(
-      mojo::TypeConverter<std::unique_ptr<blink::WebInputEvent>,
-                          ui::Event>::Convert(event));
+
+  // Take ownership of the callback, indicating that we will handle it.
+  std::unique_ptr<base::Callback<void(EventResult)>> callback =
+      std::move(*ack_callback);
+  ui::ScopedWebInputEvent web_event = ui::WebInputEventTraits::Clone(

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

Why do we need to clone the event now?

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

Sorry I misused Clone. Fixed.

+      *mojo::TypeConverter<std::unique_ptr<blink::WebInputEvent>,
+                           ui::Event>::Convert(event));
   // TODO(sad): We probably need to plumb LatencyInfo through Mus.
   ui::LatencyInfo info;
-  InputEventAckState ack_state = input_handler_manager_->HandleInputEvent(
-      routing_id_, web_event.get(), &info);
+  input_handler_manager_->HandleInputEvent(
+      routing_id_, std::move(web_event), info,
+      base::Bind(
+          &CompositorMusConnection::DidHandleWindowInputEventAndOverscroll,
+          this, base::Passed(&callback)));
+}
+
+void CompositorMusConnection::DidHandleWindowInputEventAndOverscroll(
+    std::unique_ptr<base::Callback<void(EventResult)>> ack_callback,
+    InputEventAckState ack_state,
+    ui::ScopedWebInputEvent web_event,
+    const ui::LatencyInfo& latency_info,
+    std::unique_ptr<ui::DidOverscrollParams>) {
   // TODO(jonross): We probably need to ack the event based on the consumed
   // state.
-  if (ack_state != INPUT_EVENT_ACK_STATE_NOT_CONSUMED)
+  if (ack_state != INPUT_EVENT_ACK_STATE_NOT_CONSUMED) {
+    // We took the ownership of the callback, so we need to send the ack, and
+    // mark the event as not consumed to preserve existing behavior.
+    ack_callback->Run(EventResult::UNHANDLED);
     return;
+  }
   base::Callback<void(EventResult)> ack =
       base::Bind(&::DoNothingWithEventResult);
   const bool send_ack =
@@ -147,13 +166,18 @@ void CompositorMusConnection::OnWindowInputEvent(
     // OnWindowInputEventAckOnMainThread.
     ack =
         base::Bind(&CompositorMusConnection::OnWindowInputEventAckOnMainThread,
-                   this, *ack_callback->get());
-    ack_callback->reset();
+                   this, *ack_callback);
+  } else {
+    // We took the ownership of the callback, so we need to send the ack, and
+    // mark the event as not consumed to preserve existing behavior.
+    ack_callback->Run(EventResult::UNHANDLED);
   }
+  ack_callback.reset();
+
   main_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&CompositorMusConnection::OnWindowInputEventOnMainThread, this,
-                 base::Passed(std::move(web_event)), ack));
+                 base::Passed(&web_event), ack));

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

It isn't clear to me why this changed from std::move.

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

Changed back to |std::move|.

There is no actual difference. I'm just applying the syntactic sugar introduced
in "bind_helpers.h:433" and to keep consistence with my new code:
  141           this, base::Passed(&callback)));

See "bind_helpers.h:433":
https://cs.chromium.org/chromium/src/base/bind_helpers.h?q=base::passed&sq=pa...

 }
 
 }  // namespace content