Revert of Delay Input.dispatchKeyEvent response until after key event ack....

Revert of Delay Input.dispatchKeyEvent response until after key event ack. (patchset #16 id:300001 of https://codereview.chromium.org/2387353004/ )

Reason for revert:
Tentative revert for:

Failed steps failed chrome_public_test_apk on android failed content_browsertests on android

https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29?numbuilds=200

as of this CL (and 2 others which are mere renames + don't touch content so suspecting this one from the 3: https://chromium.googlesource.com/chromium/src/+log/b671a5234f41f1784bd261a7c1eb6010434cd2c2..92aad3c0c15d18263af3e5ce0dad525c0b063297?pretty=fuller&n=10000)

No logs from failures on Android bot unfortunately :-(... Don't understand why it would have passed CQ and not waterfall but still can't see what else in range could be at fault, sorry..

Original issue's description:
> Delay Input.dispatchKeyEvent response until after key event ack.
>
> BUG=chromedriver:1506
>
> Review-Url: https://codereview.chromium.org/2387353004

TBR=tdresser@chromium.org,dtapuska@chromium.org,dgozman@chromium.org,pfeldman@chromium.org,samuong@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=673218, chromedriver:1506

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

content/browser/devtools/protocol/devtools_protocol_browsertest.cc

@@ -186,16 +186,13 @@ 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)
-      WaitForResponse();
+    if (in_dispatch_ && wait) {
+      waiting_for_command_result_id_ = last_sent_id_;
+      base::RunLoop().Run();
+    }
     in_dispatch_ = false;
   }
 
-  void WaitForResponse() {
-    waiting_for_command_result_id_ = last_sent_id_;
-    base::RunLoop().Run();
-  }
-
   bool HasValue(const std::string& path) {
     base::Value* value = 0;
     return result_->Get(path, &value);
@@ -412,26 +409,14 @@ class SyntheticKeyEventTest : public DevToolsProtocolTest {
                     int modifier,
                     int windowsKeyCode,
                     int nativeKeyCode,
-                    const std::string& key,
-                    bool wait) {
+                    const std::string& key) {
     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), 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);
+    SendCommand("Input.dispatchKeyEvent", std::move(params));
   }
 };
 
@@ -450,8 +435,8 @@ IN_PROC_BROWSER_TEST_F(SyntheticKeyEventTest, KeyEventSynthesizeKey) {
   DOMMessageQueue dom_message_queue;
 
   // Send enter (keycode 13).
-  SendKeyEvent("rawKeyDown", 0, 13, 13, "Enter", true);
-  SendKeyEvent("keyUp", 0, 13, 13, "Enter", true);
+  SendKeyEvent("rawKeyDown", 0, 13, 13, "Enter");
+  SendKeyEvent("keyUp", 0, 13, 13, "Enter");
 
   std::string key;
   ASSERT_TRUE(dom_message_queue.WaitForMessage(&key));
@@ -460,8 +445,8 @@ IN_PROC_BROWSER_TEST_F(SyntheticKeyEventTest, KeyEventSynthesizeKey) {
   EXPECT_EQ("\"Enter\"", key);
 
   // Send escape (keycode 27).
-  SendKeyEvent("rawKeyDown", 0, 27, 27, "Escape", true);
-  SendKeyEvent("keyUp", 0, 27, 27, "Escape", true);
+  SendKeyEvent("rawKeyDown", 0, 27, 27, "Escape");
+  SendKeyEvent("keyUp", 0, 27, 27, "Escape");
 
   ASSERT_TRUE(dom_message_queue.WaitForMessage(&key));
   EXPECT_EQ("\"Escape\"", key);
@@ -469,59 +454,6 @@ 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,8 +663,6 @@ 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,7 +130,6 @@ bool SetMouseEventType(blink::WebMouseEvent* event, const std::string& type) {
 
 InputHandler::InputHandler()
     : host_(NULL),
-      input_queued_(false),
       page_scale_factor_(1.0),
       weak_factory_(this) {
 }
@@ -138,30 +137,8 @@ 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) {
@@ -174,15 +151,7 @@ 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,
@@ -244,17 +213,11 @@ 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,
@@ -286,12 +249,7 @@ 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();
 }
 
@@ -521,17 +479,6 @@ 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) {
@@ -574,18 +521,6 @@ 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,7 +10,6 @@
 #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 {
@@ -24,20 +23,18 @@ class RenderWidgetHostImpl;
 namespace devtools {
 namespace input {
 
-class InputHandler : public RenderWidgetHost::InputEventObserver {
+class InputHandler {
  public:
   typedef DevToolsProtocolClient::Response Response;
 
   InputHandler();
-  ~InputHandler() override;
+  virtual ~InputHandler();
 
   void SetRenderWidgetHost(RenderWidgetHostImpl* host);
   void SetClient(std::unique_ptr<Client> client);
   void OnSwapCompositorFrame(const cc::CompositorFrameMetadata& frame_metadata);
-  void Detached();
 
-  Response DispatchKeyEvent(DevToolsCommandId command_id,
-                            const std::string& type,
+  Response DispatchKeyEvent(const std::string& type,
                             const int* modifiers,
                             const double* timestamp,
                             const std::string* text,
@@ -51,8 +48,7 @@ class InputHandler : public RenderWidgetHost::InputEventObserver {
                             const bool* is_keypad,
                             const bool* is_system_key);
 
-  Response DispatchMouseEvent(DevToolsCommandId command_id,
-                              const std::string& type,
+  Response DispatchMouseEvent(const std::string& type,
                               int x,
                               int y,
                               const int* modifiers,
@@ -105,13 +101,6 @@ class InputHandler : public RenderWidgetHost::InputEventObserver {
       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);
 
@@ -136,16 +125,8 @@ class InputHandler : public RenderWidgetHost::InputEventObserver {
                         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,7 +584,6 @@ 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,8 +2153,6 @@ 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);
 
@@ -2175,8 +2173,6 @@ 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(
@@ -2184,8 +2180,6 @@ 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 &&
@@ -2200,8 +2194,6 @@ 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);
@@ -2211,8 +2203,6 @@ 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,13 +243,12 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
   virtual void AddMouseEventCallback(const MouseEventCallback& callback) = 0;
   virtual void RemoveMouseEventCallback(const MouseEventCallback& callback) = 0;
 
-  // Observer for WebInputEvents.
+  // Observer for WebInputEvents (but not input event acks).
   class InputEventObserver {
    public:
     virtual ~InputEventObserver() {}
 
-    virtual void OnInputEvent(const blink::WebInputEvent&) {};
-    virtual void OnInputEventAck(const blink::WebInputEvent&) {};
+    virtual void OnInputEvent(const blink::WebInputEvent&) = 0;
   };
 
   // Add/remove an input event observer.

content/public/test/browser_test_utils.cc

@@ -1531,13 +1531,9 @@ 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 (HasReceivedAck())
+  if (ack_result_ != INPUT_EVENT_ACK_STATE_UNKNOWN)
     return ack_result_;
   base::RunLoop run_loop;
   base::AutoReset<base::Closure> reset_quit(&quit_, run_loop.QuitClosure());

content/public/test/browser_test_utils.h

@@ -556,8 +556,6 @@ 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();