Linux accelerators cleanup:

- Give renderer a chance to handle accelerators before browser does.
- Handle browser accelerators that aren't attached to any particular UI element in BrowserWindowGtk rather than in BrowserToolbarGtk
- Use Browser::ExecuteCommand() to handle accelerator activation
- Switch a random void* to gfx::NativeWindow
- Enable three browser commands on linux : Focus Location, Focus Search, Open file

This fully enables ctrl-l, ctrl-k, and ctrl-o. This fixes copy-pasta in the omnibox. This fixes the problem Dean described with <http://www.quirksmode.org/js/keys.html>.

bug=8659

Review URL: http://codereview.chromium.org/42190

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

4 files changed, 91 insertions, 42 deletions

diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index ff770c6..95fbbbc 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -27,24 +27,6 @@ const int BrowserToolbarGtk::kToolbarHeight = 38;
 // when the user clicks and the popup menu appears.
 static const int kMenuTimerDelay = 500;
 
-gboolean OnAccelerator(GtkAccelGroup* accel_group,
-                       GObject* acceleratable,
-                       guint keyval,
-                       GdkModifierType modifier,
-                       gpointer userdata) {
-  BrowserToolbarGtk* self = reinterpret_cast<BrowserToolbarGtk*>(userdata);
-  switch (keyval) {
-    case GDK_l:
-      self->GetLocationBar()->FocusLocation();
-      return TRUE;
-    case GDK_k:
-      self->GetLocationBar()->FocusSearch();
-      return TRUE;
-    default:
-      return FALSE;
-  }
-}
-
 BrowserToolbarGtk::BrowserToolbarGtk(Browser* browser)
     : toolbar_(NULL),
       location_bar_(new LocationBarViewGtk(browser->command_updater(),
@@ -68,12 +50,11 @@ BrowserToolbarGtk::BrowserToolbarGtk(Browser* browser)
 BrowserToolbarGtk::~BrowserToolbarGtk() {
 }
 
-void BrowserToolbarGtk::Init(Profile* profile, GtkAccelGroup* accel_group) {
+void BrowserToolbarGtk::Init(Profile* profile,
+                             GtkWindow* top_level_window) {
   // Make sure to tell the location bar the profile before calling its Init.
   SetProfile(profile);
 
-  accel_group_ = accel_group;
-
   show_home_button_.Init(prefs::kShowHomeButton, profile->GetPrefs(), this);
 
   toolbar_ = gtk_hbox_new(FALSE, 0);
@@ -82,6 +63,12 @@ void BrowserToolbarGtk::Init(Profile* profile, GtkAccelGroup* accel_group) {
   // -1 for width means "let GTK do its normal sizing".
   gtk_widget_set_size_request(toolbar_, -1, kToolbarHeight);
 
+  accel_group_ = gtk_accel_group_new();
+  gtk_window_add_accel_group(top_level_window, accel_group_);
+  // Drop the initial ref on |accel_group_| so that |top_level_window| will own
+  // it.
+  g_object_unref(accel_group_);
+
   toolbar_tooltips_ = gtk_tooltips_new();
 
   back_.reset(BuildBackForwardButton(IDR_BACK, IDR_BACK_P, IDR_BACK_H,
@@ -93,6 +80,9 @@ void BrowserToolbarGtk::Init(Profile* profile, GtkAccelGroup* accel_group) {
                  l10n_util::GetString(IDS_TOOLTIP_FORWARD)));
   AddAcceleratorToButton(forward_, GDK_Right, GDK_MOD1_MASK);
 
+  // TODO(estade): These blank labels are kind of ghetto. Padding should be
+  // handled differently (via spacing parameters or padding widgets that use
+  // gtk_widget_set_size_request).
   gtk_box_pack_start(GTK_BOX(toolbar_), gtk_label_new(" "), FALSE, FALSE, 0);
 
   reload_.reset(BuildToolbarButton(IDR_RELOAD, IDR_RELOAD_P, IDR_RELOAD_H, 0,
@@ -112,15 +102,6 @@ void BrowserToolbarGtk::Init(Profile* profile, GtkAccelGroup* accel_group) {
   location_bar_->Init();
   gtk_box_pack_start(GTK_BOX(toolbar_), location_bar_->widget(), TRUE, TRUE, 0);
 
-  // Map ctrl-l for setting focus to the location entry.
-  gtk_accel_group_connect(
-      accel_group_, GDK_l, GDK_CONTROL_MASK, GtkAccelFlags(0),
-      g_cclosure_new(G_CALLBACK(OnAccelerator), this, NULL));
-  // Map ctrl-k for setting focus to a search in the location entry.
-  gtk_accel_group_connect(
-      accel_group_, GDK_k, GDK_CONTROL_MASK, GtkAccelFlags(0),
-      g_cclosure_new(G_CALLBACK(OnAccelerator), this, NULL));
-
   go_.reset(BuildToolbarButton(IDR_GO, IDR_GO_P, IDR_GO_H, 0, L""));
 
   gtk_box_pack_start(GTK_BOX(toolbar_), gtk_label_new(" "), FALSE, FALSE, 0);
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.h b/chrome/browser/gtk/browser_toolbar_gtk.h
index 200fe28..1fce6ec 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_toolbar_gtk.h
@@ -36,8 +36,9 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
   explicit BrowserToolbarGtk(Browser* browser);
   virtual ~BrowserToolbarGtk();
 
-  // Create the contents of the toolbar
-  void Init(Profile* profile, GtkAccelGroup* accel_group);
+  // Create the contents of the toolbar. |top_level_window| is the GtkWindow
+  // to which we attach our accelerators.
+  void Init(Profile* profile, GtkWindow* top_level_window);
 
   // Adds this GTK toolbar into a sizing box.
   void AddToolbarToBox(GtkWidget* box);
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index 56745bd..e7aa460 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -4,10 +4,13 @@
 
 #include "chrome/browser/gtk/browser_window_gtk.h"
 
+#include <gdk/gdkkeysyms.h>
+
 #include "base/base_paths_linux.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
+#include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/find_bar_controller.h"
 #include "chrome/browser/gtk/browser_toolbar_gtk.h"
@@ -18,7 +21,6 @@
 #include "chrome/browser/location_bar.h"
 #include "chrome/browser/renderer_host/render_widget_host_view_gtk.h"
 #include "chrome/browser/tab_contents/web_contents.h"
-
 #include "chrome/common/resource_bundle.h"
 #include "grit/theme_resources.h"
 
@@ -75,6 +77,34 @@ gfx::Rect GetInitialWindowBounds(GtkWindow* window) {
   return gfx::Rect(x, y, width, height);
 }
 
+const guint kFocusLocationKey = GDK_l;
+const guint kFocusSearchKey = GDK_k;
+const guint kOpenFileKey = GDK_o;
+
+static int GetCommandFromKeyval(guint accel_key) {
+  switch (accel_key) {
+    case kFocusLocationKey:
+      return IDC_FOCUS_LOCATION;
+    case kFocusSearchKey:
+      return IDC_FOCUS_SEARCH;
+    case kOpenFileKey:
+      return IDC_OPEN_FILE;
+    default:
+      NOTREACHED();
+      return 0;
+  }
+}
+
+// Usually accelerators are checked before propagating the key event, but in our
+// case we want to reverse the order of things to allow webkit to handle key
+// events like ctrl-l. If the window's children can handle the key event, this
+// will return TRUE and the signal won't be propagated further. If the window's
+// children cannot handle the key event then this will return FALSE and the
+// default GtkWindow key press handler will be invoked.
+gboolean OnKeyPress(GtkWindow* window, GdkEventKey* event, gpointer userdata) {
+  return gtk_window_propagate_key_event(window, event);
+}
+
 }  // namespace
 
 BrowserWindowGtk::BrowserWindowGtk(Browser* browser)
@@ -130,17 +160,17 @@ gboolean BrowserWindowGtk::OnContentAreaExpose(GtkWidget* widget,
 void BrowserWindowGtk::Init() {
   window_ = GTK_WINDOW(gtk_window_new(GTK_WINDOW_TOPLEVEL));
   gtk_window_set_default_size(window_, 640, 480);
-  g_signal_connect(G_OBJECT(window_), "destroy",
+  g_signal_connect(window_, "destroy",
                    G_CALLBACK(OnWindowDestroyed), this);
-  g_signal_connect(G_OBJECT(window_), "configure-event",
+  g_signal_connect(window_, "configure-event",
                    G_CALLBACK(MainWindowConfigured), this);
-  g_signal_connect(G_OBJECT(window_), "window-state-event",
+  g_signal_connect(window_, "window-state-event",
                    G_CALLBACK(MainWindowStateChanged), this);
+  g_signal_connect(window_, "key-press-event",
+                   G_CALLBACK(OnKeyPress), NULL);
+  ConnectAccelerators();
   bounds_ = GetInitialWindowBounds(window_);
 
-  GtkAccelGroup* accel_group = gtk_accel_group_new();
-  gtk_window_add_accel_group(window_, accel_group);
-
   GdkPixbuf* images[9] = {
     LoadThemeImage(IDR_CONTENT_TOP_LEFT_CORNER),
     LoadThemeImage(IDR_CONTENT_TOP_CENTER),
@@ -163,7 +193,7 @@ void BrowserWindowGtk::Init() {
                    G_CALLBACK(&OnContentAreaExpose), this);
 
   toolbar_.reset(new BrowserToolbarGtk(browser_.get()));
-  toolbar_->Init(browser_->profile(), accel_group);
+  toolbar_->Init(browser_->profile(), window_);
   toolbar_->AddToolbarToBox(vbox_);
 
   FindBarGtk* find_bar_gtk = new FindBarGtk();
@@ -230,7 +260,7 @@ void BrowserWindowGtk::FlashFrame() {
   gtk_window_set_urgency_hint(window_, TRUE);
 }
 
-void* BrowserWindowGtk::GetNativeHandle() {
+gfx::NativeWindow BrowserWindowGtk::GetNativeHandle() {
   return window_;
 }
 
@@ -424,6 +454,23 @@ void BrowserWindowGtk::OnStateChanged(GdkWindowState state) {
   state_ = state;
 }
 
+void BrowserWindowGtk::ConnectAccelerators() {
+  GtkAccelGroup* accel_group = gtk_accel_group_new();
+  gtk_window_add_accel_group(window_, accel_group);
+  // Drop the initial ref on |accel_group| so |window_| will own it.
+  g_object_unref(accel_group);
+
+  gtk_accel_group_connect(
+      accel_group, kFocusLocationKey, GDK_CONTROL_MASK, GtkAccelFlags(0),
+      g_cclosure_new(G_CALLBACK(OnAccelerator), this, NULL));
+  gtk_accel_group_connect(
+      accel_group, kFocusSearchKey, GDK_CONTROL_MASK, GtkAccelFlags(0),
+      g_cclosure_new(G_CALLBACK(OnAccelerator), this, NULL));
+  gtk_accel_group_connect(
+      accel_group, kOpenFileKey, GDK_CONTROL_MASK, GtkAccelFlags(0),
+      g_cclosure_new(G_CALLBACK(OnAccelerator), this, NULL));
+}
+
 void BrowserWindowGtk::SetCustomFrame(bool custom_frame) {
   custom_frame_ = custom_frame;
   if (custom_frame_) {
@@ -452,3 +499,12 @@ gboolean BrowserWindowGtk::OnWindowDestroyed(GtkWidget* window,
   return FALSE;  // Don't stop this message.
 }
 
+// static
+gboolean BrowserWindowGtk::OnAccelerator(GtkAccelGroup* accel_group,
+                                         GObject* acceleratable,
+                                         guint keyval,
+                                         GdkModifierType modifier,
+                                         BrowserWindowGtk* browser_window) {
+  browser_window->browser_->ExecuteCommand(GetCommandFromKeyval(keyval));
+  return TRUE;
+}
diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h
index abf2610..89eaa6e 100644
--- a/chrome/browser/gtk/browser_window_gtk.h
+++ b/chrome/browser/gtk/browser_window_gtk.h
@@ -37,7 +37,7 @@ class BrowserWindowGtk : public BrowserWindow,
   virtual void Activate();
   virtual bool IsActive() const;
   virtual void FlashFrame();
-  virtual void* GetNativeHandle();
+  virtual gfx::NativeWindow GetNativeHandle();
   virtual BrowserWindowTesting* GetBrowserWindowTesting();
   virtual StatusBubble* GetStatusBubble();
   virtual void SelectedTabToolbarSizeChanged(bool is_animating);
@@ -90,6 +90,10 @@ class BrowserWindowGtk : public BrowserWindow,
   scoped_ptr<Browser> browser_;
 
  private:
+  // Connect accelerators that aren't connected to menu items (like ctrl-o,
+  // ctrl-l, etc.).
+  void ConnectAccelerators();
+
   // Change whether we're showing the custom blue frame.
   // Must be called once at startup.
   // Triggers relayout of the content.
@@ -103,6 +107,13 @@ class BrowserWindowGtk : public BrowserWindow,
 
   static gboolean OnWindowDestroyed(GtkWidget* window,
                                     BrowserWindowGtk* browser_win);
+
+  static gboolean OnAccelerator(GtkAccelGroup* accel_group,
+                                GObject* acceleratable,
+                                guint keyval,
+                                GdkModifierType modifier,
+                                BrowserWindowGtk* browser_window);
+
   gfx::Rect bounds_;
   GdkWindowState state_;