Linux accelerators cleanup

chrome/browser/gtk/browser_window_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_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"
 #include "chrome/browser/gtk/find_bar_gtk.h"
 #include "chrome/browser/gtk/nine_box.h"
 #include "chrome/browser/gtk/status_bubble_gtk.h"
 #include "chrome/browser/gtk/tab_contents_container_gtk.h"
 #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"
 
 namespace {
 
 static GdkPixbuf* LoadThemeImage(int resource_id) {
   // TODO(mmoss) refactor -- stolen from custom_button.cc
   if (0 == resource_id)
     return NULL;
 
@@ skipping 36 matching lines @@
 // Using gtk_window_get_position/size creates a race condition, so only use
 // this to get the initial bounds.  After window creation, we pick up the
 // normal bounds by connecting to the configure-event signal.
 gfx::Rect GetInitialWindowBounds(GtkWindow* window) {
   gint x, y, width, height;
   gtk_window_get_position(window, &x, &y);
   gtk_window_get_size(window, &width, &height);
   return gfx::Rect(x, y, width, height);
 }
 
+static const guint kFocusLocationKey = GDK_l;
+static const guint kFocusSearchKey = GDK_k;
+static const guint kOpenFileKey = GDK_o;

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

there are in a namespace (so shouldn't be static)

+
+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)
     :  browser_(browser),
        // TODO(port): make this a pref.
        custom_frame_(false) {
   Init();
   browser_->tabstrip_model()->AddObserver(this);
 }
 
@@ skipping 35 matching lines @@
                   -1, -1,
                   GDK_RGB_DITHER_NORMAL, 0, 0);
   gdk_pixbuf_unref(pixbuf);
 
   return FALSE;  // Allow subwidgets to paint.
 }
 
 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),
     LoadThemeImage(IDR_CONTENT_TOP_RIGHT_CORNER),
     LoadThemeImage(IDR_CONTENT_LEFT_SIDE),
     NULL,
     LoadThemeImage(IDR_CONTENT_RIGHT_SIDE),
     LoadThemeImage(IDR_CONTENT_BOTTOM_LEFT_CORNER),
     LoadThemeImage(IDR_CONTENT_BOTTOM_CENTER),
     LoadThemeImage(IDR_CONTENT_BOTTOM_RIGHT_CORNER)
   };
   content_area_ninebox_.reset(new NineBox(images));
 
   // This vbox is intended to surround the "content": toolbar+page.
   // When we add the tab strip, it should go in a vbox surrounding this one.
   vbox_ = gtk_vbox_new(FALSE, 0);
   gtk_widget_set_app_paintable(vbox_, TRUE);
   gtk_widget_set_double_buffered(vbox_, FALSE);
   g_signal_connect(G_OBJECT(vbox_), "expose-event",
                    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();
   find_bar_controller_.reset(new FindBarController(find_bar_gtk));
   find_bar_gtk->set_find_bar_controller(find_bar_controller_.get());
 
   contents_container_.reset(new TabContentsContainerGtk(
       find_bar_gtk->gtk_widget()));
 
   contents_container_->AddContainerToBox(vbox_);
@@ skipping 46 matching lines @@
 bool BrowserWindowGtk::IsActive() const {
   NOTIMPLEMENTED();
   return true;
 }
 
 void BrowserWindowGtk::FlashFrame() {
   // May not be respected by all window managers.
   gtk_window_set_urgency_hint(window_, TRUE);
 }
 
-void* BrowserWindowGtk::GetNativeHandle() {
+gfx::NativeWindow BrowserWindowGtk::GetNativeHandle() {
   return window_;
 }
 
 BrowserWindowTesting* BrowserWindowGtk::GetBrowserWindowTesting() {
   NOTIMPLEMENTED();
   return NULL;
 }
 
 StatusBubble* BrowserWindowGtk::GetStatusBubble() {
   return status_bubble_.get();
@@ skipping 170 matching lines @@
 }
 
 void BrowserWindowGtk::OnBoundsChanged(const gfx::Rect& bounds) {
   bounds_ = bounds;
 }
 
 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);
+  // The window now owns |accel_group|.

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

Same here, it's confusing.

+  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_) {
     gtk_container_set_border_width(GTK_CONTAINER(vbox_), 2);
     // TODO(port): all the crazy blue title bar, etc.
     NOTIMPLEMENTED();
   } else {
     gtk_container_set_border_width(GTK_CONTAINER(vbox_), 0);
   }
 }
 
 // static
 gboolean BrowserWindowGtk::OnWindowDestroyed(GtkWidget* window,
                                              BrowserWindowGtk* browser_win) {
   // BUG 8712. When we gtk_widget_destroy() in Close(), this will emit the
   // signal right away, and we will be here (while Close() is still in the
   // call stack).  In order to not reenter Close(), and to also follow the
   // expectations of BrowserList, we should run the BrowserWindowGtk destructor
   // not now, but after the run loop goes back to process messages.  Otherwise
   // we will remove ourself from BrowserList while it's being iterated.
   // Additionally, now that we know the window is gone, we need to make sure to
   // set window_ to NULL, otherwise we will try to close the window again when
   // we call Close() in the destructor.
   browser_win->window_ = NULL;
   MessageLoop::current()->DeleteSoon(FROM_HERE, browser_win);
   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;
+}