Make GTK file dialog box modal for parent window, instead of for the entire

application.

This works by adding the top-level GtkWindow objects, such as BrowserWindowGtk
and BookmarkManagerGtk, to their own unique window groups. Without this change,
all top-level windows are added to a default application-wide window group.

This ensures that all grabs created with gtk_grab_add(...) only affect the
window group of the grabbed widget, as opposed to the entire application.

Note that gtk_window_set_modal(...) is implemented with gtk_grab_add(...) and
therefore is only modal to a specific window group.

In order for this to work correctly, changes were also made to the info
bubble and render widget popup (<select> tag) code. Since these widgets
also call gtk_grab_add(...), they must be added to the top level window
group to work correctly.

Test 1:
- Open two new chrome window: A and B
- Open "Save file as..." dialog in window A
- Verify that window A does not respond to keyboard or mouse events.
- Verify that window B does responde to keyboard and mouse events.
- Open "Save file as..." dialog in window B
- Verify that window B does not respond to keyboard or mouse events.
- Cancel dialog on window A.
- Verify that window A starts responding to keyboard and mouse events.
- Cancel dialog on window B.
- Verify that window B starts responding to keyboard and mouse events.

Test 2:
- Verify that <select> tag allows for correct selection of items.

Test 3:
- Click bookmark star and verify that info bubble works correctly.

BUG=8727
TEST=none

patch by Mohit Muthanna Cheppudira <mohit.muthanna [at] gmail>
original review url: <http://codereview.chromium.org/155852>

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

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

5 files changed, 30 insertions, 7 deletions

diff --git a/chrome/browser/gtk/bookmark_manager_gtk.cc b/chrome/browser/gtk/bookmark_manager_gtk.cc
index 76e9fb7..f5219dd 100644
--- a/chrome/browser/gtk/bookmark_manager_gtk.cc
+++ b/chrome/browser/gtk/bookmark_manager_gtk.cc
@@ -323,6 +323,11 @@ void BookmarkManagerGtk::InitWidgets() {
   gtk_window_set_title(GTK_WINDOW(window_),
       l10n_util::GetStringUTF8(IDS_BOOKMARK_MANAGER_TITLE).c_str());
 
+  // Add this window to its own unique window group to allow for
+  // window-to-parent modality.
+  gtk_window_group_add_window(gtk_window_group_new(), GTK_WINDOW(window_));
+  g_object_unref(gtk_window_get_group(GTK_WINDOW(window_)));
+
   // Set the default size of the bookmark manager.
   int width, height;
   gtk_util::GetWidgetSizeFromResources(window_,
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index bff26ed..2d4d1a9 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -392,6 +392,11 @@ BrowserWindowGtk::BrowserWindowGtk(Browser* browser)
   gtk_widget_add_events(GTK_WIDGET(window_), GDK_BUTTON_PRESS_MASK |
                                              GDK_POINTER_MOTION_MASK);
 
+  // Add this window to its own unique window group to allow for
+  // window-to-parent modality.
+  gtk_window_group_add_window(gtk_window_group_new(), window_);
+  g_object_unref(gtk_window_get_group(window_));
+
   gtk_util::SetWindowIcon(window_);
   SetBackgroundColor();
   SetGeometryHints();
diff --git a/chrome/browser/gtk/dialogs_gtk.cc b/chrome/browser/gtk/dialogs_gtk.cc
index 62de9ce6..1283d4d 100644
--- a/chrome/browser/gtk/dialogs_gtk.cc
+++ b/chrome/browser/gtk/dialogs_gtk.cc
@@ -26,12 +26,7 @@ static const int kPreviewWidth = 256;
 static const int kPreviewHeight = 512;
 
 // Implementation of SelectFileDialog that shows a Gtk common dialog for
-// choosing a file or folder.
-// This acts as a modal dialog. Ideally we want to only act modally for the
-// parent window and allow other toplevel chrome windows to still function while
-// the dialog is showing, but we need the GtkWindowGroup or something similar to
-// get that, and that API is only available in more recent versions of GTK.
-// TODO(port): fix modality: crbug.com/8727
+// choosing a file or folder. This acts as a modal dialog.
 class SelectFileDialogImpl : public SelectFileDialog {
  public:
   explicit SelectFileDialogImpl(Listener* listener);
@@ -208,7 +203,13 @@ void SelectFileDialogImpl::SelectFile(
   gtk_file_chooser_set_preview_widget(GTK_FILE_CHOOSER(dialog), preview_);
 
   params_map_[dialog] = params;
+
+  // Set window-to-parent modality by adding the dialog to the same window
+  // group as the parent.
+  gtk_window_group_add_window(gtk_window_get_group(owning_window),
+                              GTK_WINDOW(dialog));
   gtk_window_set_modal(GTK_WINDOW(dialog), TRUE);
+
   gtk_widget_show_all(dialog);
 }
 
diff --git a/chrome/browser/gtk/info_bubble_gtk.cc b/chrome/browser/gtk/info_bubble_gtk.cc
index cee6ad3..892d08e 100644
--- a/chrome/browser/gtk/info_bubble_gtk.cc
+++ b/chrome/browser/gtk/info_bubble_gtk.cc
@@ -209,6 +209,13 @@ void InfoBubbleGtk::Init(GtkWindow* transient_toplevel,
   // of the info bubble.  We don't use an X grab since that would steal
   // keystrokes from your window manager, prevent you from interacting with
   // other applications, etc.
+  //
+  // Before adding the grab, we need to ensure that the bubble is added
+  // to the window group of the top level window. This ensures that the
+  // grab only affects the current browser window, and not all the open
+  // browser windows in the application.
+  gtk_window_group_add_window(gtk_window_get_group(transient_toplevel),
+                              GTK_WINDOW(window_));
   gtk_grab_add(window_);
 
   registrar_.Add(this, NotificationType::ACTIVE_WINDOW_CHANGED,
diff --git a/chrome/browser/renderer_host/render_widget_host_view_gtk.cc b/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
index d70c601..98ed533 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
@@ -873,8 +873,13 @@ void RenderWidgetHostViewGtk::InitAsPopup(
   // and webkit will manage our destruction.
   if (activatable()) {
     // Grab all input for the app. If a click lands outside the bounds of the
-    // popup, WebKit will notice and destroy us.
+    // popup, WebKit will notice and destroy us. Before doing this we need
+    // to ensure that the the popup is added to the browser's window group,
+    // to allow for the grabs to work correctly.
+    gtk_window_group_add_window(gtk_window_get_group(
+        GTK_WINDOW(gtk_widget_get_toplevel(parent_))), GTK_WINDOW(popup));
     gtk_grab_add(view_.get());
+
     // Now grab all of X's input.
     gdk_pointer_grab(
         parent_->window,