Refactor compositor event handling path to be callback-based

content/renderer/mus/compositor_mus_connection.cc

 // Copyright 2015 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/renderer/mus/compositor_mus_connection.h"
 
 #include "base/single_thread_task_runner.h"
 #include "content/renderer/input/input_handler_manager.h"
 #include "content/renderer/mus/render_widget_mus_connection.h"
 #include "mojo/converters/blink/blink_input_events_type_converters.h"
@@ skipping 60 matching lines @@
 void CompositorMusConnection::OnConnectionLostOnMainThread() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   RenderWidgetMusConnection* connection =
       RenderWidgetMusConnection::Get(routing_id_);
   if (!connection)
     return;
   connection->OnConnectionLost();
 }
 
 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 =
       RenderWidgetMusConnection::Get(routing_id_);
   if (!connection) {
     ack.Run(EventResult::UNHANDLED);
     return;
   }
   connection->OnWindowInputEvent(std::move(web_event), ack);
 }
@@ skipping 26 matching lines @@
     const ui::PointerEvent& event,
     ui::Window* target) {
   // Compositor does not use StartPointerWatcher().
 }
 
 void CompositorMusConnection::OnWindowInputEvent(
     ui::Window* window,
     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 =
       ui::WebInputEventTraits::ShouldBlockEventStream(*web_event);
   if (send_ack) {
     // Ultimately, this ACK needs to go back to the Mus client lib which is not
     // thread-safe and lives on the compositor thread. For ACKs that are passed
     // to the main thread we pass them back to the compositor thread via
     // 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