6 files changed, 58 insertions, 50 deletions

diff --git a/chrome/browser/gtk/constrained_window_gtk.cc b/chrome/browser/gtk/constrained_window_gtk.cc
index 8ec5195..321aa16 100644
--- a/chrome/browser/gtk/constrained_window_gtk.cc
+++ b/chrome/browser/gtk/constrained_window_gtk.cc
@@ -15,7 +15,8 @@ ConstrainedWindowGtk::ConstrainedWindowGtk(
     TabContents* owner, ConstrainedWindowGtkDelegate* delegate)
     : owner_(owner),
       delegate_(delegate),
-      visible_(false) {
+      visible_(false),
+      accel_group_(gtk_accel_group_new()) {
   DCHECK(owner);
   DCHECK(delegate);
   GtkWidget* dialog = delegate->GetWidgetRoot();
@@ -33,14 +34,20 @@ ConstrainedWindowGtk::ConstrainedWindowGtk(
   gtk_container_add(GTK_CONTAINER(frame), alignment);
   gtk_container_add(GTK_CONTAINER(ebox), frame);
   border_.Own(ebox);
-
-  gtk_widget_add_events(widget(), GDK_KEY_PRESS_MASK);
-  g_signal_connect(widget(), "key-press-event", G_CALLBACK(OnKeyPressThunk),
-                   this);
+  ConnectAccelerators();
 }
 
 ConstrainedWindowGtk::~ConstrainedWindowGtk() {
   border_.Destroy();
+
+  gtk_accel_group_disconnect_key(accel_group_, GDK_Escape,
+                                 static_cast<GdkModifierType>(0));
+  if (ContainingView() && ContainingView()->GetTopLevelNativeWindow()) {
+    gtk_window_remove_accel_group(
+        GTK_WINDOW(ContainingView()->GetTopLevelNativeWindow()),
+        accel_group_);
+  }
+  g_object_unref(accel_group_);
 }
 
 void ConstrainedWindowGtk::ShowConstrainedWindow() {
@@ -59,23 +66,36 @@ void ConstrainedWindowGtk::CloseConstrainedWindow() {
   delegate_->DeleteDelegate();
   owner_->WillClose(this);
 
-  // Let the stack unwind so any relevant event handler can release its ref
-  // on widget().
-  MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+  delete this;
 }
 
 TabContentsViewGtk* ConstrainedWindowGtk::ContainingView() {
   return static_cast<TabContentsViewGtk*>(owner_->view());
 }
 
-gboolean ConstrainedWindowGtk::OnKeyPress(GtkWidget* sender,
-                                          GdkEventKey* key) {
-  if (key->keyval == GDK_Escape) {
-    CloseConstrainedWindow();
-    return TRUE;
-  }
+void ConstrainedWindowGtk::ConnectAccelerators() {
+  gtk_accel_group_connect(accel_group_,
+                          GDK_Escape, static_cast<GdkModifierType>(0),
+                          static_cast<GtkAccelFlags>(0),
+                          g_cclosure_new(G_CALLBACK(OnEscapeThunk),
+                                         this, NULL));
+  gtk_window_add_accel_group(
+      GTK_WINDOW(ContainingView()->GetTopLevelNativeWindow()),
+      accel_group_);
+}
+
 
-  return FALSE;
+gboolean ConstrainedWindowGtk::OnEscape(GtkAccelGroup* group,
+                                        GObject* acceleratable,
+                                        guint keyval,
+                                        GdkModifierType modifier) {
+  // Handle this accelerator only if this is on the currently selected tab.
+  Browser* browser = BrowserList::GetLastActive();
+  if (!browser || browser->GetSelectedTabContents() != owner_)
+    return FALSE;
+
+  CloseConstrainedWindow();
+  return TRUE;
 }
 
 // static
@@ -84,3 +104,4 @@ ConstrainedWindow* ConstrainedWindow::CreateConstrainedDialog(
     ConstrainedWindowGtkDelegate* delegate) {
   return new ConstrainedWindowGtk(parent, delegate);
 }
+
diff --git a/chrome/browser/gtk/constrained_window_gtk.h b/chrome/browser/gtk/constrained_window_gtk.h
index 4c0a443..2312cb3 100644
--- a/chrome/browser/gtk/constrained_window_gtk.h
+++ b/chrome/browser/gtk/constrained_window_gtk.h
@@ -15,6 +15,7 @@
 
 class TabContents;
 class TabContentsViewGtk;
+typedef struct _GtkWidget GtkWidget;
 
 class ConstrainedWindowGtkDelegate {
  public:
@@ -55,9 +56,12 @@ class ConstrainedWindowGtk : public ConstrainedWindow {
   ConstrainedWindowGtk(TabContents* owner,
                        ConstrainedWindowGtkDelegate* delegate);
 
-  // Handler for Escape.
-  CHROMEGTK_CALLBACK_1(ConstrainedWindowGtk, gboolean, OnKeyPress,
-                       GdkEventKey*);
+  // Connects the ESC accelerator to the window.
+  void ConnectAccelerators();
+
+  // Handles an ESC accelerator being pressed.
+  CHROMEG_CALLBACK_3(ConstrainedWindowGtk, gboolean, OnEscape, GtkAccelGroup*,
+                     GObject*, guint, GdkModifierType);
 
   // The TabContents that owns and constrains this ConstrainedWindow.
   TabContents* owner_;
@@ -71,6 +75,8 @@ class ConstrainedWindowGtk : public ConstrainedWindow {
   // Stores if |ShowConstrainedWindow()| has been called.
   bool visible_;
 
+  GtkAccelGroup* accel_group_;
+
   DISALLOW_COPY_AND_ASSIGN(ConstrainedWindowGtk);
 };
 
diff --git a/chrome/browser/gtk/focus_store_gtk.cc b/chrome/browser/gtk/focus_store_gtk.cc
index 50ebfa7..dc099fc 100644
--- a/chrome/browser/gtk/focus_store_gtk.cc
+++ b/chrome/browser/gtk/focus_store_gtk.cc
@@ -19,22 +19,19 @@ FocusStoreGtk::~FocusStoreGtk() {
 }
 
 void FocusStoreGtk::Store(GtkWidget* widget) {
-  GtkWidget* focus_widget = NULL;
-  if (widget) {
-    GtkWindow* window = platform_util::GetTopLevel(widget);
-    if (window)
-      focus_widget = window->focus_widget;
+  DisconnectDestroyHandler();
+  if (!widget) {
+    widget_ = NULL;
+    return;
   }
 
-  SetWidget(focus_widget);
-}
-
-void FocusStoreGtk::SetWidget(GtkWidget* widget) {
-  DisconnectDestroyHandler();
+  GtkWindow* window = platform_util::GetTopLevel(widget);
+  if (!window) {
+    widget_ = NULL;
+    return;
+  }
 
-  // We don't add a ref. The signal handler below effectively gives us a weak
-  // reference.
-  widget_ = widget;
+  widget_ = window->focus_widget;
   if (widget_) {
     // When invoked, |gtk_widget_destroyed| will set |widget_| to NULL.
     destroy_handler_id_ = g_signal_connect(widget_, "destroy",
@@ -44,8 +41,6 @@ void FocusStoreGtk::SetWidget(GtkWidget* widget) {
 }
 
 void FocusStoreGtk::DisconnectDestroyHandler() {
-  if (widget_) {
+  if (widget_)
     g_signal_handler_disconnect(widget_, destroy_handler_id_);
-    widget_ = NULL;
-  }
 }
diff --git a/chrome/browser/gtk/focus_store_gtk.h b/chrome/browser/gtk/focus_store_gtk.h
index 2159f1e..ce3b68b 100644
--- a/chrome/browser/gtk/focus_store_gtk.h
+++ b/chrome/browser/gtk/focus_store_gtk.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -19,12 +19,9 @@ class FocusStoreGtk {
   GtkWidget* widget() const { return widget_; }
 
   // Save the widget that is currently focused for |widget|'s toplevel (NOT
-  // |widget|).
+  // |widget|). Call with NULL to clear |widget_|.
   void Store(GtkWidget* widget);
 
-  // Save |widget| as the focus widget. Call with NULL to clear |widget_|.
-  void SetWidget(GtkWidget* widget);
-
  private:
   // Disconnect the previous destroy handler (if any).
   void DisconnectDestroyHandler();
diff --git a/chrome/browser/gtk/gtk_util.cc b/chrome/browser/gtk/gtk_util.cc
index 541d2da..5a5a815 100644
--- a/chrome/browser/gtk/gtk_util.cc
+++ b/chrome/browser/gtk/gtk_util.cc
@@ -1059,11 +1059,4 @@ string16 GetStockPreferencesMenuLabel() {
   return preferences;
 }
 
-bool IsWidgetAncestryVisible(GtkWidget* widget) {
-  GtkWidget* parent = widget;
-  while (parent && GTK_WIDGET_VISIBLE(parent))
-    parent = parent->parent;
-  return !parent;
-}
-
 }  // namespace gtk_util
diff --git a/chrome/browser/gtk/gtk_util.h b/chrome/browser/gtk/gtk_util.h
index 95200d4..20d3c6a 100644
--- a/chrome/browser/gtk/gtk_util.h
+++ b/chrome/browser/gtk/gtk_util.h
@@ -315,10 +315,6 @@ gfx::Rect GetDialogBounds(GtkWidget* dialog);
 // empty string if no stock item found.
 string16 GetStockPreferencesMenuLabel();
 
-// Checks whether a widget is actually visible, i.e. whether it and all its
-// ancestors up to its toplevel are visible.
-bool IsWidgetAncestryVisible(GtkWidget* widget);
-
 }  // namespace gtk_util
 
 #endif  // CHROME_BROWSER_GTK_GTK_UTIL_H_