Don't send WebInputEvents from the renderer to the browser.

The browser process now keeps a queue of the last keyboard
events that it sent to the renderer in a queue and pops them
on ACK. If a key is unhandled, we use the copy in the browser
process; we don't even send the key back in the ACK anymore.
Review URL: http://codereview.chromium.org/27244

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@10563 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 100 insertions, 13 deletions

diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index 35fd1fd..894e02a 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -1200,7 +1200,7 @@ void RenderViewHost::OnUserMetricsRecordAction(const std::wstring& action) {
   UserMetrics::RecordComputedAction(action.c_str(), process()->profile());
 }
 
-void RenderViewHost::UnhandledInputEvent(const WebInputEvent& event) {
+void RenderViewHost::UnhandledKeyboardEvent(const WebKeyboardEvent& event) {
   RenderViewHostDelegate::View* view = delegate_->GetViewDelegate();
   if (view) {
     // TODO(brettw) why do we have to filter these types of events here. Can't
@@ -1208,8 +1208,7 @@ void RenderViewHost::UnhandledInputEvent(const WebInputEvent& event) {
     // should be able to decide which ones it wants or not?
     if ((event.type == WebInputEvent::KEY_DOWN) ||
         (event.type == WebInputEvent::CHAR)) {
-      view->HandleKeyboardEvent(
-          static_cast<const WebKeyboardEvent&>(event));
+      view->HandleKeyboardEvent(event);
     }
   }
 }
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index 812851c..7a0a8148 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -423,7 +423,7 @@ class RenderViewHost : public RenderWidgetHost {
 
  protected:
   // RenderWidgetHost protected overrides.
-  virtual void UnhandledInputEvent(const WebInputEvent& event);
+  virtual void UnhandledKeyboardEvent(const WebKeyboardEvent& event);
   virtual void OnEnterOrSpace();
   virtual void NotifyRendererUnresponsive();
   virtual void NotifyRendererResponsive();
diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc
index 7e0c465..f75d2e6 100644
--- a/chrome/browser/renderer_host/render_widget_host.cc
+++ b/chrome/browser/renderer_host/render_widget_host.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/renderer_host/render_widget_host.h"
 
 #include "base/gfx/native_widget_types.h"
+#include "base/histogram.h"
 #include "base/message_loop.h"
 #include "base/keyboard_codes.h"
 #include "chrome/browser/renderer_host/backing_store.h"
@@ -305,6 +306,14 @@ void RenderWidgetHost::ForwardInputEvent(const WebInputEvent& input_event,
   if (!process_->channel())
     return;
 
+  if (WebInputEvent::IsKeyboardEventType(input_event.type)) {
+    // Put all WebKeyboardEvent objects in a queue since we can't trust the
+    // renderer and we need to give something to the UnhandledInputEvent
+    // handler.
+    key_queue_.push(static_cast<const WebKeyboardEvent&>(input_event));
+    HISTOGRAM_COUNTS_100("Renderer.KeyboardQueueSize", key_queue_.size());
+  }
+
   IPC::Message* message = new ViewMsg_HandleInputEvent(routing_id_);
   message->WriteData(
       reinterpret_cast<const char*>(&input_event), event_size);
@@ -554,12 +563,24 @@ void RenderWidgetHost::OnMsgInputEventAck(const IPC::Message& message) {
     }
   }
 
-  const char* data = NULL;
-  int length = 0;
-  if (message.ReadData(&iter, &data, &length)) {
-    const WebInputEvent* input_event =
-        reinterpret_cast<const WebInputEvent*>(data);
-    UnhandledInputEvent(*input_event);
+  if (WebInputEvent::IsKeyboardEventType(type)) {
+    if (key_queue_.size() == 0) {
+      LOG(ERROR) << "Got a KeyEvent back from the renderer but we "
+                 << "don't seem to have sent it to the renderer!";
+    } else if (key_queue_.front().type != type) {
+      LOG(ERROR) << "We seem to have a different key type sent from "
+                 << "the renderer. Ignoring event.";
+    } else {
+      bool processed = false;
+      r = message.ReadBool(&iter, &processed);
+      DCHECK(r);
+
+      if (!processed) {
+        UnhandledKeyboardEvent(key_queue_.front());
+      }
+
+      key_queue_.pop();
+    }
   }
 }
 
diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h
index 31a467a..647c893 100644
--- a/chrome/browser/renderer_host/render_widget_host.h
+++ b/chrome/browser/renderer_host/render_widget_host.h
@@ -5,12 +5,14 @@
 #ifndef CHROME_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_H_
 #define CHROME_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_H_
 
+#include <queue>
 #include <vector>
 
 #include "base/gfx/size.h"
 #include "base/timer.h"
 #include "chrome/common/ipc_channel.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
+#include "webkit/glue/webinputevent.h"
 
 namespace gfx {
 class Rect;
@@ -221,7 +223,7 @@ class RenderWidgetHost : public IPC::Channel::Listener {
 
   // Called when we an InputEvent was not processed by the renderer. This is
   // overridden by RenderView to send upwards to its delegate.
-  virtual void UnhandledInputEvent(const WebInputEvent& event) {}
+  virtual void UnhandledKeyboardEvent(const WebKeyboardEvent& event) {}
 
   // Notification that the user pressed the enter key or the spacebar. The
   // render view host overrides this to forward the information to its delegate
@@ -350,6 +352,11 @@ class RenderWidgetHost : public IPC::Channel::Listener {
   // operation to finish.
   base::TimeTicks repaint_start_time_;
 
+  // A queue of keyboard events. We can't trust data from the renderer so we
+  // stuff key events into a queue and pop them out on ACK, feeding our copy
+  // back to whatever unhandled handler instead of the returned version.
+  std::queue<WebKeyboardEvent> key_queue_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHost);
 };
 
diff --git a/chrome/browser/renderer_host/render_widget_host_unittest.cc b/chrome/browser/renderer_host/render_widget_host_unittest.cc
index c2dba12..7a720c3 100644
--- a/chrome/browser/renderer_host/render_widget_host_unittest.cc
+++ b/chrome/browser/renderer_host/render_widget_host_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/basictypes.h"
+#include "base/keyboard_codes.h"
 #include "base/scoped_ptr.h"
 #include "base/shared_memory.h"
 #include "build/build_config.h"
@@ -119,6 +120,30 @@ class TestView : public TestRenderWidgetHostView {
   DISALLOW_COPY_AND_ASSIGN(TestView);
 };
 
+// MockRenderWidgetHostTest ----------------------------------------------------
+
+class MockRenderWidgetHost : public RenderWidgetHost {
+ public:
+  MockRenderWidgetHost(RenderProcessHost* process, int routing_id)
+      : RenderWidgetHost(process, routing_id),
+        unhandled_keyboard_event_called_(false) {
+  }
+
+  // Tests that make sure we ignore keyboard event acknowledgments to events we
+  // didn't send work by making sure we didn't call UnhandledKeyboardEvent().
+  bool unhandled_keyboard_event_called() const {
+    return unhandled_keyboard_event_called_;
+  }
+
+ protected:
+  virtual void UnhandledKeyboardEvent(const WebKeyboardEvent& event) {
+    unhandled_keyboard_event_called_ = true;
+  }
+
+ private:
+  bool unhandled_keyboard_event_called_;
+};
+
 // RenderWidgetHostTest --------------------------------------------------------
 
 class RenderWidgetHostTest : public testing::Test {
@@ -133,7 +158,7 @@ class RenderWidgetHostTest : public testing::Test {
   void SetUp() {
     profile_.reset(new TestingProfile());
     process_ = new RenderWidgetHostProcess(profile_.get());
-    host_.reset(new RenderWidgetHost(process_, 1));
+    host_.reset(new MockRenderWidgetHost(process_, 1));
     view_.reset(new TestView);
     host_->set_view(view_.get());
   }
@@ -151,7 +176,7 @@ class RenderWidgetHostTest : public testing::Test {
 
   scoped_ptr<TestingProfile> profile_;
   RenderWidgetHostProcess* process_;  // Deleted automatically by the widget.
-  scoped_ptr<RenderWidgetHost> host_;
+  scoped_ptr<MockRenderWidgetHost> host_;
   scoped_ptr<TestView> view_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostTest);
@@ -304,3 +329,38 @@ TEST_F(RenderWidgetHostTest, HiddenPaint) {
   ViewMsg_WasRestored::Read(restored, &needs_repaint);
   EXPECT_TRUE(needs_repaint);
 }
+
+TEST_F(RenderWidgetHostTest, HandleKeyEventsWeSent) {
+  WebKeyboardEvent key_event;
+  key_event.type = WebInputEvent::KEY_DOWN;
+  key_event.modifiers = WebInputEvent::CTRL_KEY;
+  key_event.key_code = base::VKEY_L;  // non-null made up value.
+
+  host_->ForwardKeyboardEvent(key_event);
+
+  // Make sure we sent the input event to the renderer.
+  EXPECT_TRUE(process_->sink().GetUniqueMessageMatching(
+                  ViewMsg_HandleInputEvent::ID));
+  process_->sink().ClearMessages();
+
+  // Send the simulated response from the renderer back.
+  scoped_ptr<IPC::Message> response(
+      new ViewHostMsg_HandleInputEvent_ACK(0));
+  response->WriteInt(key_event.type);
+  response->WriteBool(false);
+  host_->OnMessageReceived(*response);
+
+  EXPECT_TRUE(host_->unhandled_keyboard_event_called());
+}
+
+TEST_F(RenderWidgetHostTest, IgnoreKeyEventsWeDidntSend) {
+  // Send a simulated, unrequested key response. We should ignore this.
+  scoped_ptr<IPC::Message> response(
+      new ViewHostMsg_HandleInputEvent_ACK(0));
+  response->WriteInt(WebInputEvent::KEY_DOWN);
+  response->WriteBool(false);
+  host_->OnMessageReceived(*response);
+
+  EXPECT_FALSE(host_->unhandled_keyboard_event_called());
+}
+