Don't send tab switching/killing/creating keyboard accelerators to pages. This avoids tabs maliciously preventing closing using ctrl+f4/ctrl+w/alt+f4, and also hung/slow renderers from making tab cycling sluggish.

BUG=5496
TEST=added ui test
Review URL: http://codereview.chromium.org/224023

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

7 files changed, 35 insertions, 12 deletions

diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index 2b66289..c0cb864 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -1425,19 +1425,18 @@ void RenderViewHost::OnUserMetricsRecordAction(const std::wstring& action) {
   UserMetrics::RecordComputedAction(action.c_str(), process()->profile());
 }
 
+bool RenderViewHost::ShouldSendToRenderer(const NativeWebKeyboardEvent& event) {
+  RenderViewHostDelegate::View* view = delegate_->GetViewDelegate();
+  if (!view)
+    return true;
+  return !view->IsReservedAccelerator(event);
+}
+
 void RenderViewHost::UnhandledKeyboardEvent(
     const NativeWebKeyboardEvent& event) {
   RenderViewHostDelegate::View* view = delegate_->GetViewDelegate();
-  if (view) {
-    // TODO(brettw) why do we have to filter these types of events here. Can't
-    // the renderer just send us the ones we care abount, or maybe the view
-    // should be able to decide which ones it wants or not?
-    if ((event.type == WebInputEvent::RawKeyDown) ||
-        (event.type == WebInputEvent::KeyDown) ||
-        (event.type == WebInputEvent::Char)) {
-      view->HandleKeyboardEvent(event);
-    }
-  }
+  if (view)
+    view->HandleKeyboardEvent(event);
 }
 
 void RenderViewHost::OnUserGesture() {
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index 328a566..72ee9eb 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -436,6 +436,7 @@ class RenderViewHost : public RenderWidgetHost,
 
  protected:
   // RenderWidgetHost protected overrides.
+  virtual bool ShouldSendToRenderer(const NativeWebKeyboardEvent& event);
   virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event);
   virtual void OnUserGesture();
   virtual void NotifyRendererUnresponsive();
diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h
index 9c121a1..d31d94c 100644
--- a/chrome/browser/renderer_host/render_view_host_delegate.h
+++ b/chrome/browser/renderer_host/render_view_host_delegate.h
@@ -125,6 +125,10 @@ class RenderViewHostDelegate {
     // true, it means the focus was retrieved by doing a Shift-Tab.
     virtual void TakeFocus(bool reverse) = 0;
 
+    // Returns whether the event is a reserved keyboard shortcut that should not
+    // be sent to the renderer.
+    virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event) = 0;
+
     // Callback to inform the browser that the renderer did not process the
     // specified events. This gives an opportunity to the browser to process the
     // event (used for keyboard shortcuts).
diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc
index d9971ec..01ee015 100644
--- a/chrome/browser/renderer_host/render_widget_host.cc
+++ b/chrome/browser/renderer_host/render_widget_host.cc
@@ -400,6 +400,13 @@ void RenderWidgetHost::ForwardKeyboardEvent(
     if (!process_->HasConnection())
       return;
 
+    // Tab switching/closing accelerators aren't sent to the renderer to avoid a
+    // hung/malicious renderer from interfering.
+    if (!ShouldSendToRenderer(key_event)) {
+      UnhandledKeyboardEvent(key_event);
+      return;
+    }
+
     // Put all WebKeyboardEvent objects in a queue since we can't trust the
     // renderer and we need to give something to the UnhandledInputEvent
     // handler.
@@ -754,7 +761,7 @@ void RenderWidgetHost::OnMsgInputEventAck(const IPC::Message& message) {
       if (!message.ReadBool(&iter, &processed))
         process()->ReceivedBadMessage(message.type());
 
-      KeyQueue::value_type front_item = key_queue_.front();
+      NativeWebKeyboardEvent front_item = key_queue_.front();
       key_queue_.pop();
 
       if (!processed) {
diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h
index a80d1fd..2438e16 100644
--- a/chrome/browser/renderer_host/render_widget_host.h
+++ b/chrome/browser/renderer_host/render_widget_host.h
@@ -357,6 +357,12 @@ class RenderWidgetHost : public IPC::Channel::Listener,
   // This is used for various IPC messages, including plugins.
   gfx::NativeViewId GetNativeViewId();
 
+  // Called when an InputEvent is received to check if the event should be sent
+  // to the renderer or not.
+  virtual bool ShouldSendToRenderer(const NativeWebKeyboardEvent& event) {
+    return true;
+  }
+
   // Called when we an InputEvent was not processed by the renderer. This is
   // overridden by RenderView to send upwards to its delegate.
   virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) {}
diff --git a/chrome/browser/renderer_host/render_widget_host_view_gtk.cc b/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
index 0440820..d382aff 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
@@ -519,6 +519,9 @@ void RenderWidgetHostViewGtk::Destroy() {
   // See http://www.crbug.com/11847 for details.
   gtk_widget_destroy(view_.get());
 
+  // The RenderWidgetHost's destruction led here, so don't call it.
+  host_ = NULL;
+
   MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
@@ -643,6 +646,9 @@ void RenderWidgetHostViewGtk::DestroyPluginContainer(
 
 void RenderWidgetHostViewGtk::ForwardKeyboardEvent(
     const NativeWebKeyboardEvent& event) {
+  if (!host_)
+    return;
+
   EditCommands edit_commands;
   if (key_bindings_handler_->Match(event, &edit_commands)) {
     host_->ForwardEditCommandsForNextKeyEvent(edit_commands);
diff --git a/chrome/browser/renderer_host/render_widget_host_view_gtk.h b/chrome/browser/renderer_host/render_widget_host_view_gtk.h
index 2f7e6b3..933629e 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_gtk.h
+++ b/chrome/browser/renderer_host/render_widget_host_view_gtk.h
@@ -90,7 +90,7 @@ class RenderWidgetHostViewGtk : public RenderWidgetHostView {
   void ShowCurrentCursor();
 
   // The model object.
-  RenderWidgetHost* const host_;
+  RenderWidgetHost* host_;
 
   // The native UI widget.
   OwnedWidgetGtk view_;