Delay Input.dispatchKeyEvent response until after key event ack.

BUG=chromedriver:1506

Review-Url: https://codereview.chromium.org/2387353004
Cr-Commit-Position: refs/heads/master@{#437706}

content/browser/devtools/protocol/devtools_protocol_browsertest.cc

@@ -186,12 +186,15 @@ class DevToolsProtocolTest : public ContentBrowserTest,
     agent_host_->DispatchProtocolMessage(this, json_command);
     // Some messages are dispatched synchronously.
     // Only run loop if we are not finished yet.
-    if (in_dispatch_ && wait) {
+    if (in_dispatch_ && wait)
+      WaitForResponse();
+    in_dispatch_ = false;
+  }
+
+  void WaitForResponse() {
     waiting_for_command_result_id_ = last_sent_id_;
     base::RunLoop().Run();
   }
-    in_dispatch_ = false;
-  }
 
   bool HasValue(const std::string& path) {
     base::Value* value = 0;
@@ -409,14 +412,26 @@ class SyntheticKeyEventTest : public DevToolsProtocolTest {
                     int modifier,
                     int windowsKeyCode,
                     int nativeKeyCode,
-                    const std::string& key) {
+                    const std::string& key,
+                    bool wait) {
     std::unique_ptr<base::DictionaryValue> params(new base::DictionaryValue());
     params->SetString("type", type);
     params->SetInteger("modifiers", modifier);
     params->SetInteger("windowsVirtualKeyCode", windowsKeyCode);
     params->SetInteger("nativeVirtualKeyCode", nativeKeyCode);
     params->SetString("key", key);
-    SendCommand("Input.dispatchKeyEvent", std::move(params));
+    SendCommand("Input.dispatchKeyEvent", std::move(params), wait);
+  }
+};
+
+class SyntheticMouseEventTest : public DevToolsProtocolTest {
+ protected:
+  void SendMouseEvent(const std::string& type, int x, int y, bool wait) {
+    std::unique_ptr<base::DictionaryValue> params(new base::DictionaryValue());
+    params->SetString("type", type);
+    params->SetInteger("x", x);
+    params->SetInteger("y", y);
+    SendCommand("Input.dispatchMouseEvent", std::move(params), wait);
   }
 };
 
@@ -435,8 +450,8 @@ IN_PROC_BROWSER_TEST_F(SyntheticKeyEventTest, KeyEventSynthesizeKey) {
   DOMMessageQueue dom_message_queue;
 
   // Send enter (keycode 13).
-  SendKeyEvent("rawKeyDown", 0, 13, 13, "Enter");
-  SendKeyEvent("keyUp", 0, 13, 13, "Enter");
+  SendKeyEvent("rawKeyDown", 0, 13, 13, "Enter", true);
+  SendKeyEvent("keyUp", 0, 13, 13, "Enter", true);
 
   std::string key;
   ASSERT_TRUE(dom_message_queue.WaitForMessage(&key));
@@ -445,8 +460,8 @@ IN_PROC_BROWSER_TEST_F(SyntheticKeyEventTest, KeyEventSynthesizeKey) {
   EXPECT_EQ("\"Enter\"", key);
 
   // Send escape (keycode 27).
-  SendKeyEvent("rawKeyDown", 0, 27, 27, "Escape");
-  SendKeyEvent("keyUp", 0, 27, 27, "Escape");
+  SendKeyEvent("rawKeyDown", 0, 27, 27, "Escape", true);
+  SendKeyEvent("keyUp", 0, 27, 27, "Escape", true);
 
   ASSERT_TRUE(dom_message_queue.WaitForMessage(&key));
   EXPECT_EQ("\"Escape\"", key);
@@ -454,6 +469,59 @@ IN_PROC_BROWSER_TEST_F(SyntheticKeyEventTest, KeyEventSynthesizeKey) {
   EXPECT_EQ("\"Escape\"", key);
 }
 
+IN_PROC_BROWSER_TEST_F(SyntheticKeyEventTest, KeyboardEventAck) {
+  NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
+  Attach();
+  ASSERT_TRUE(content::ExecuteScript(
+      shell()->web_contents()->GetRenderViewHost(),
+      "document.body.addEventListener('keydown', () => console.log('x'));"));
+
+  scoped_refptr<InputMsgWatcher> filter = new InputMsgWatcher(
+      RenderWidgetHostImpl::From(
+          shell()->web_contents()->GetRenderViewHost()->GetWidget()),
+      blink::WebInputEvent::MouseMove);
+
+  SendCommand("Runtime.enable", nullptr);
+  SendKeyEvent("rawKeyDown", 0, 13, 13, "Enter", false);
+
+  // We expect that the console log message event arrives *before* the input
+  // event ack, and the subsequent command response for Input.dispatchKeyEvent.
+  WaitForNotification("Runtime.consoleAPICalled");
+  EXPECT_THAT(console_messages_, ElementsAre("x"));
+  EXPECT_FALSE(filter->HasReceivedAck());
+  EXPECT_EQ(1u, result_ids_.size());
+
+  WaitForResponse();
+  EXPECT_EQ(2u, result_ids_.size());
+}
+
+IN_PROC_BROWSER_TEST_F(SyntheticMouseEventTest, MouseEventAck) {
+  NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
+  Attach();
+  ASSERT_TRUE(content::ExecuteScript(
+      shell()->web_contents()->GetRenderViewHost(),
+      "document.body.addEventListener('mousemove', () => console.log('x'));"));
+
+  scoped_refptr<InputMsgWatcher> filter = new InputMsgWatcher(
+      RenderWidgetHostImpl::From(
+          shell()->web_contents()->GetRenderViewHost()->GetWidget()),
+      blink::WebInputEvent::MouseMove);
+
+  SendCommand("Runtime.enable", nullptr);
+  SendMouseEvent("mouseMoved", 15, 15, false);
+
+  // We expect that the console log message event arrives *before* the input
+  // event ack, and the subsequent command response for
+  // Input.dispatchMouseEvent.
+  WaitForNotification("Runtime.consoleAPICalled");
+  EXPECT_THAT(console_messages_, ElementsAre("x"));
+  EXPECT_FALSE(filter->HasReceivedAck());
+  EXPECT_EQ(1u, result_ids_.size());
+
+  WaitForResponse();
+  EXPECT_EQ(2u, result_ids_.size());
+}
+
 namespace {
 bool DecodePNG(std::string base64_data, SkBitmap* bitmap) {
   std::string png_data;

content/browser/devtools/protocol/devtools_protocol_handler_generator.py

@@ -663,6 +663,8 @@ async_commands_list = [
     "Network.getCookies",
     "Network.deleteCookie",
     "Network.setCookie",
+    "Input.dispatchKeyEvent",
+    "Input.dispatchMouseEvent",
     "Input.synthesizePinchGesture",
     "Input.synthesizeScrollGesture",
     "Input.synthesizeTapGesture"]

content/browser/devtools/protocol/input_handler.cc

@@ -130,6 +130,7 @@ bool SetMouseEventType(blink::WebMouseEvent* event, const std::string& type) {
 
 InputHandler::InputHandler()
     : host_(NULL),
+      input_queued_(false),
       page_scale_factor_(1.0),
       weak_factory_(this) {
 }
@@ -137,8 +138,30 @@ InputHandler::InputHandler()
 InputHandler::~InputHandler() {
 }
 
+void InputHandler::OnInputEvent(const blink::WebInputEvent& event) {
+  input_queued_ = true;
+}
+
+void InputHandler::OnInputEventAck(const blink::WebInputEvent& event) {
+  if (blink::WebInputEvent::isKeyboardEventType(event.type) &&
+      !pending_key_command_ids_.empty()) {
+    SendDispatchKeyEventResponse(pending_key_command_ids_.front());
+    pending_key_command_ids_.pop_front();
+  } else if (blink::WebInputEvent::isMouseEventType(event.type) &&
+             !pending_mouse_command_ids_.empty()) {
+    SendDispatchMouseEventResponse(pending_mouse_command_ids_.front());
+    pending_mouse_command_ids_.pop_front();
+  }
+}
+
 void InputHandler::SetRenderWidgetHost(RenderWidgetHostImpl* host) {
+  ClearPendingKeyCommands();
+  ClearPendingMouseCommands();
+  if (host_)
+    host_->RemoveInputEventObserver(this);
   host_ = host;
+  if (host)
+    host->AddInputEventObserver(this);
 }
 
 void InputHandler::SetClient(std::unique_ptr<Client> client) {
@@ -151,7 +174,15 @@ void InputHandler::OnSwapCompositorFrame(
   scrollable_viewport_size_ = frame_metadata.scrollable_viewport_size;
 }
 
+void InputHandler::Detached() {
+  ClearPendingKeyCommands();
+  ClearPendingMouseCommands();
+  if (host_)
+    host_->RemoveInputEventObserver(this);
+}
+
 Response InputHandler::DispatchKeyEvent(
+    DevToolsCommandId command_id,
     const std::string& type,
     const int* modifiers,
     const double* timestamp,
@@ -213,11 +244,17 @@ Response InputHandler::DispatchKeyEvent(
     return Response::ServerError("Could not connect to view");
 
   host_->Focus();
+  input_queued_ = false;
   host_->ForwardKeyboardEvent(event);
+  if (input_queued_)
+    pending_key_command_ids_.push_back(command_id);
+  else
+    SendDispatchKeyEventResponse(command_id);
   return Response::OK();
 }
 
 Response InputHandler::DispatchMouseEvent(
+    DevToolsCommandId command_id,
     const std::string& type,
     int x,
     int y,
@@ -249,7 +286,12 @@ Response InputHandler::DispatchMouseEvent(
     return Response::ServerError("Could not connect to view");
 
   host_->Focus();
+  input_queued_ = false;
   host_->ForwardMouseEvent(event);
+  if (input_queued_)
+    pending_mouse_command_ids_.push_back(command_id);
+  else
+    SendDispatchMouseEventResponse(command_id);
   return Response::OK();
 }
 
@@ -479,6 +521,17 @@ Response InputHandler::DispatchTouchEvent(
   return Response::FallThrough();
 }
 
+void InputHandler::SendDispatchKeyEventResponse(DevToolsCommandId command_id) {
+  client_->SendDispatchKeyEventResponse(
+      command_id, DispatchKeyEventResponse::Create());
+}
+
+void InputHandler::SendDispatchMouseEventResponse(
+    DevToolsCommandId command_id) {
+  client_->SendDispatchMouseEventResponse(
+      command_id, DispatchMouseEventResponse::Create());
+}
+
 void InputHandler::SendSynthesizePinchGestureResponse(
     DevToolsCommandId command_id,
     SyntheticGesture::Result result) {
@@ -521,6 +574,18 @@ void InputHandler::SendSynthesizeTapGestureResponse(
   }
 }
 
+void InputHandler::ClearPendingKeyCommands() {
+  for (const DevToolsCommandId& command_id : pending_key_command_ids_)
+    SendDispatchKeyEventResponse(command_id);
+  pending_key_command_ids_.clear();
+}
+
+void InputHandler::ClearPendingMouseCommands() {
+  for (const DevToolsCommandId& command_id : pending_mouse_command_ids_)
+    SendDispatchMouseEventResponse(command_id);
+  pending_mouse_command_ids_.clear();
+}
+
 }  // namespace input
 }  // namespace devtools
 }  // namespace content

content/browser/devtools/protocol/input_handler.h

@@ -10,6 +10,7 @@
 #include "content/browser/devtools/protocol/devtools_protocol_dispatcher.h"
 #include "content/browser/renderer_host/input/synthetic_gesture.h"
 #include "content/common/input/synthetic_smooth_scroll_gesture_params.h"
+#include "content/public/browser/render_widget_host.h"
 #include "ui/gfx/geometry/size_f.h"
 
 namespace cc {
@@ -23,18 +24,20 @@ class RenderWidgetHostImpl;
 namespace devtools {
 namespace input {
 
-class InputHandler {
+class InputHandler : public RenderWidgetHost::InputEventObserver {
  public:
   typedef DevToolsProtocolClient::Response Response;
 
   InputHandler();
-  virtual ~InputHandler();
+  ~InputHandler() override;
 
   void SetRenderWidgetHost(RenderWidgetHostImpl* host);
   void SetClient(std::unique_ptr<Client> client);
   void OnSwapCompositorFrame(const cc::CompositorFrameMetadata& frame_metadata);
+  void Detached();
 
-  Response DispatchKeyEvent(const std::string& type,
+  Response DispatchKeyEvent(DevToolsCommandId command_id,
+                            const std::string& type,
                             const int* modifiers,
                             const double* timestamp,
                             const std::string* text,
@@ -48,7 +51,8 @@ class InputHandler {
                             const bool* is_keypad,
                             const bool* is_system_key);
 
-  Response DispatchMouseEvent(const std::string& type,
+  Response DispatchMouseEvent(DevToolsCommandId command_id,
+                              const std::string& type,
                               int x,
                               int y,
                               const int* modifiers,
@@ -101,6 +105,13 @@ class InputHandler {
       const double* timestamp);
 
  private:
+  // InputEventObserver
+  void OnInputEvent(const blink::WebInputEvent& event) override;
+  void OnInputEventAck(const blink::WebInputEvent& event) override;
+
+  void SendDispatchKeyEventResponse(DevToolsCommandId command_id);
+  void SendDispatchMouseEventResponse(DevToolsCommandId command_id);
+
   void SendSynthesizePinchGestureResponse(DevToolsCommandId command_id,
                                           SyntheticGesture::Result result);
 
@@ -125,8 +136,16 @@ class InputHandler {
                         DevToolsCommandId command_id,
                         SyntheticGesture::Result result);
 
+  void ClearPendingKeyCommands();
+  void ClearPendingMouseCommands();
+
   RenderWidgetHostImpl* host_;
   std::unique_ptr<Client> client_;
+  // DevToolsCommandIds for calls to Input.dispatchKey/MouseEvent that have been
+  // sent to the renderer, but that we haven't yet received an ack for.
+  bool input_queued_;
+  std::deque<DevToolsCommandId> pending_key_command_ids_;
+  std::deque<DevToolsCommandId> pending_mouse_command_ids_;
   float page_scale_factor_;
   gfx::SizeF scrollable_viewport_size_;
   base::WeakPtrFactory<InputHandler> weak_factory_;

content/browser/devtools/render_frame_devtools_agent_host.cc

@@ -584,6 +584,7 @@ void RenderFrameDevToolsAgentHost::OnClientDetached() {
     emulation_handler_->Detached();
   if (page_handler_)
     page_handler_->Detached();
+  input_handler_->Detached();
   service_worker_handler_->Detached();
   target_handler_->Detached();
   frame_trace_recorder_.reset();

content/browser/renderer_host/render_widget_host_impl.cc

@@ -2153,6 +2153,8 @@ void RenderWidgetHostImpl::OnKeyboardEventAck(
       const NativeWebKeyboardEventWithLatencyInfo& event,
       InputEventAckState ack_result) {
   latency_tracker_.OnInputEventAck(event.event, &event.latency, ack_result);
+  for (auto& input_event_observer : input_event_observers_)
+    input_event_observer.OnInputEventAck(event.event);
 
   const bool processed = (INPUT_EVENT_ACK_STATE_CONSUMED == ack_result);
 
@@ -2173,6 +2175,8 @@ void RenderWidgetHostImpl::OnMouseEventAck(
     InputEventAckState ack_result) {
   latency_tracker_.OnInputEventAck(mouse_event.event, &mouse_event.latency,
                                    ack_result);
+  for (auto& input_event_observer : input_event_observers_)
+    input_event_observer.OnInputEventAck(mouse_event.event);
 }
 
 void RenderWidgetHostImpl::OnWheelEventAck(
@@ -2180,6 +2184,8 @@ void RenderWidgetHostImpl::OnWheelEventAck(
     InputEventAckState ack_result) {
   latency_tracker_.OnInputEventAck(wheel_event.event, &wheel_event.latency,
                                    ack_result);
+  for (auto& input_event_observer : input_event_observers_)
+    input_event_observer.OnInputEventAck(wheel_event.event);
 
   if (!is_hidden() && view_) {
     if (ack_result != INPUT_EVENT_ACK_STATE_CONSUMED &&
@@ -2194,6 +2200,8 @@ void RenderWidgetHostImpl::OnGestureEventAck(
     const GestureEventWithLatencyInfo& event,
     InputEventAckState ack_result) {
   latency_tracker_.OnInputEventAck(event.event, &event.latency, ack_result);
+  for (auto& input_event_observer : input_event_observers_)
+    input_event_observer.OnInputEventAck(event.event);
 
   if (view_)
     view_->GestureEventAck(event.event, ack_result);
@@ -2203,6 +2211,8 @@ void RenderWidgetHostImpl::OnTouchEventAck(
     const TouchEventWithLatencyInfo& event,
     InputEventAckState ack_result) {
   latency_tracker_.OnInputEventAck(event.event, &event.latency, ack_result);
+  for (auto& input_event_observer : input_event_observers_)
+    input_event_observer.OnInputEventAck(event.event);
 
   if (touch_emulator_ &&
       touch_emulator_->HandleTouchEventAck(event.event, ack_result)) {

content/public/browser/render_widget_host.h

@@ -243,12 +243,13 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
   virtual void AddMouseEventCallback(const MouseEventCallback& callback) = 0;
   virtual void RemoveMouseEventCallback(const MouseEventCallback& callback) = 0;
 
-  // Observer for WebInputEvents (but not input event acks).
+  // Observer for WebInputEvents.
   class InputEventObserver {
    public:
     virtual ~InputEventObserver() {}
 
-    virtual void OnInputEvent(const blink::WebInputEvent&) = 0;
+    virtual void OnInputEvent(const blink::WebInputEvent&) {};
+    virtual void OnInputEventAck(const blink::WebInputEvent&) {};
   };
 
   // Add/remove an input event observer.

content/public/test/browser_test_utils.cc

@@ -1527,9 +1527,13 @@ bool InputMsgWatcher::OnMessageReceived(const IPC::Message& message) {
   return false;
 }
 
+bool InputMsgWatcher::HasReceivedAck() const {
+  return ack_result_ != INPUT_EVENT_ACK_STATE_UNKNOWN;
+}
+
 uint32_t InputMsgWatcher::WaitForAck() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (ack_result_ != INPUT_EVENT_ACK_STATE_UNKNOWN)
+  if (HasReceivedAck())
     return ack_result_;
   base::RunLoop run_loop;
   base::AutoReset<base::Closure> reset_quit(&quit_, run_loop.QuitClosure());

content/public/test/browser_test_utils.h

@@ -552,6 +552,8 @@ class InputMsgWatcher : public BrowserMessageFilter {
   InputMsgWatcher(RenderWidgetHost* render_widget_host,
                   blink::WebInputEvent::Type type);
 
+  bool HasReceivedAck() const;
+
   // Wait until ack message occurs, returning the ack result from
   // the message.
   uint32_t WaitForAck();