Linux accelerators cleanup

chrome/browser/gtk/browser_toolbar_gtk.cc

Index: chrome/browser/gtk/browser_toolbar_gtk.cc
===================================================================
--- chrome/browser/gtk/browser_toolbar_gtk.cc	(revision 11738)
+++ chrome/browser/gtk/browser_toolbar_gtk.cc	(working copy)
@@ -27,24 +27,6 @@
 // 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() {
 }
 
-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,11 @@
   // -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_);
+  // The window now owns |accel_group_|.

[Dean McNamee, 2009/03/16 19:19:02]

this makes it sound like add_accel took ownership.  Say like // Drop our
reference and let the window own.

+  g_object_unref(accel_group_);
+
   toolbar_tooltips_ = gtk_tooltips_new();
 
   back_.reset(BuildBackForwardButton(IDR_BACK, IDR_BACK_P, IDR_BACK_H,
@@ -93,6 +79,9 @@
                  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 +101,6 @@
   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);