Linux accelerators cleanup

chrome/browser/gtk/browser_toolbar_gtk.cc

 // Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/gtk/browser_toolbar_gtk.h"
 
 #include <gdk/gdkkeysyms.h>
 
 #include "base/logging.h"
 #include "base/base_paths_linux.h"
 #include "base/path_service.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/gtk/back_forward_menu_model_gtk.h"
 #include "chrome/browser/gtk/custom_button.h"
 #include "chrome/browser/gtk/location_bar_view_gtk.h"
 #include "chrome/browser/gtk/standard_menus.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/common/l10n_util.h"
 #include "chrome/common/resource_bundle.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 
 const int BrowserToolbarGtk::kToolbarHeight = 38;
 // For the back/forward dropdown menus, the time in milliseconds between
 // 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(),
                                            browser->toolbar_model())),
       model_(browser->toolbar_model()),
       browser_(browser),
       profile_(NULL),
       show_menu_factory_(this) {
   browser_->command_updater()->AddCommandObserver(IDC_BACK, this);
   browser_->command_updater()->AddCommandObserver(IDC_FORWARD, this);
   browser_->command_updater()->AddCommandObserver(IDC_RELOAD, this);
   browser_->command_updater()->AddCommandObserver(IDC_HOME, this);
   browser_->command_updater()->AddCommandObserver(IDC_STAR, this);
 
   back_menu_model_.reset(new BackForwardMenuModelGtk(
       browser, BackForwardMenuModel::BACKWARD_MENU_DELEGATE));
   forward_menu_model_.reset(new BackForwardMenuModelGtk(
       browser, BackForwardMenuModel::FORWARD_MENU_DELEGATE));
 }
 
 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);
   gtk_container_set_border_width(GTK_CONTAINER(toolbar_), 4);
   // Demand we're always at least kToolbarHeight tall.
   // -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,
               IDR_BACK_D,
               l10n_util::GetString(IDS_TOOLTIP_BACK)));
   AddAcceleratorToButton(back_, GDK_Left, GDK_MOD1_MASK);
   forward_.reset(BuildBackForwardButton(IDR_FORWARD, IDR_FORWARD_P,
                  IDR_FORWARD_H, IDR_FORWARD_D,
                  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,
       l10n_util::GetString(IDS_TOOLTIP_RELOAD)));
   AddAcceleratorToButton(reload_, GDK_r, GDK_CONTROL_MASK);
 
   // TODO(port): we need to dynamically react to changes in show_home_button_
   // and hide/show home appropriately.  But we don't have a UI for it yet.
   if (*show_home_button_)
     home_.reset(MakeHomeButton());
 
   gtk_box_pack_start(GTK_BOX(toolbar_), gtk_label_new("  "), FALSE, FALSE, 0);
 
   star_.reset(BuildToolbarButton(IDR_STAR, IDR_STAR_P, IDR_STAR_H, IDR_STAR_D,
       l10n_util::GetString(IDS_TOOLTIP_STAR)));
 
   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);
 
   page_menu_button_.reset(BuildToolbarMenuButton(IDR_MENU_PAGE,
       l10n_util::GetString(IDS_PAGEMENU_TOOLTIP)));
   page_menu_.reset(new MenuGtk(this, GetStandardPageMenu(), accel_group_));
 
   app_menu_button_.reset(BuildToolbarMenuButton(IDR_MENU_CHROME,
       l10n_util::GetStringF(IDS_APPMENU_TOOLTIP,
@@ skipping 236 matching lines @@
 }
 
 void BrowserToolbarGtk::RunAppMenu(GdkEvent* button_press_event) {
   app_menu_->Popup(app_menu_button_->widget(), button_press_event);
 }
 
 CustomDrawButton* BrowserToolbarGtk::MakeHomeButton() {
   return BuildToolbarButton(IDR_HOME, IDR_HOME_P, IDR_HOME_H, 0,
                             l10n_util::GetString(IDS_TOOLTIP_HOME));
 }