Revert 58259 - reland r57885 with a fix for linux/views.

We believe this caused a lot of login prompt auth tests to fail with : 

[5860:5871:2463125642585:FATAL:chrome/browser/gtk/owned_widget_gtk.cc(39)] Check failed: ((((GObject*) g_type_check_instance_cast ((GTypeInstance*) ((widget)), (((GType) ((20) << (2))))))))->ref_count == 1U (3 vs. 1)
Backtrace:
	StackTrace::StackTrace() [0x8b2d896]
	logging::LogMessage::~LogMessage() [0x8b47321]
	OwnedWidgetGtk::Destroy() [0x81b2536]
	LoginHandlerGtk::~LoginHandlerGtk() [0x860a419]
	base::RefCountedThreadSafe<>::DeleteInternal() [0x82911a4]
	base::DefaultRefCountedThreadSafeTraits<>::Destruct() [0x82911b7]
	base::RefCountedThreadSafe<>::Release() [0x8291215]
	
------
Original message:

[GTK] a couple of constrained window fixes:

1) don't grab focus when the parent tab isn't showing. Grab the focus when the tab is brought to the front.
2) handle escape via normal key handling rather than accelerator keys
3) don't allow the content view to take focus (via tab) when the constrained window is showing.

BUG=53242, 50799
TEST=see bugs. Also, tabbing between constrained window and url bar should work as expected.

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

TBR=estade@chromium.org
Review URL: http://codereview.chromium.org/3293005

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

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_