[mutter] window: make determination of attached dialog windows more consistent

[Dan Winship <danw src gnome org>]

commit 7f8c59614ede92aec6e3ddee937d920a66b7dd6c
Author: Dan Winship <danw gnome org>
Date:   Tue Apr 5 10:14:52 2011 -0400

    window: make determination of attached dialog windows more consistent
    
    Different bits of code were using slightly different checks to test
    whether a window was an attached dialog. Add a new
    meta_window_is_attached_dialog(), and use that everywhere.
    
    Also, freeze the is-attached status when the window is first shown,
    rather than recomputing it each time the caller asks, since this could
    cause problems if a window changes its type after it has already been
    attached, etc. However, if an attached window's parent is destroyed,
    or an attached window changes its transient-for, then fix things up by
    destroying the old MetaWindow and creating a new one (causing
    compositor unmap and map events to be fired off, allowing the display
    of the window to be fixed up).
    
    Remove some code in display.c that tried to fix existing windows if
    the gconf setting changed, but which didn't actually do anything (at
    least under gnome-shell). However, if 654643 was fixed then the new
    behavior with this patch would be that changing the gconf setting
    would affect new dialogs, but not existing ones.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=646761

 src/core/constraints.c    |    4 +-
 src/core/display.c        |   31 +------------
 src/core/window-private.h |    5 ++
 src/core/window-props.c   |   75 ++++++++++++++++++++-----------
 src/core/window.c         |  108 +++++++++++++++++++++++++++++++++++----------
 src/meta/window.h         |    1 +
 6 files changed, 141 insertions(+), 83 deletions(-)
---
diff --git a/src/core/constraints.c b/src/core/constraints.c
index 2da398f..fea4ff4 100644
--- a/src/core/constraints.c
+++ b/src/core/constraints.c
@@ -749,9 +749,7 @@ constrain_modal_dialog (MetaWindow         *window,
   MetaWindow *parent = meta_window_get_transient_for (window);
   gboolean constraint_already_satisfied;
 
-  if (!meta_prefs_get_attach_modal_dialogs ())
-    return TRUE;
-  if (window->type != META_WINDOW_MODAL_DIALOG || !parent)
+  if (!meta_window_is_attached_dialog (window))
     return TRUE;
 
   x = parent->rect.x + (parent->rect.width / 2  - info->current.width / 2);
diff --git a/src/core/display.c b/src/core/display.c
index 763d8f5..25182ac 100644
--- a/src/core/display.c
+++ b/src/core/display.c
@@ -3524,8 +3524,7 @@ meta_display_begin_grab_op (MetaDisplay *display,
   /* If window is a modal dialog attached to its parent,
    * grab the parent instead for moving.
    */
-  if (meta_prefs_get_attach_modal_dialogs () &&
-      window && window->type == META_WINDOW_MODAL_DIALOG &&
+  if (window && meta_window_is_attached_dialog (window) &&
       meta_grab_op_is_moving (op))
     grab_window = meta_window_get_transient_for (window);
 
@@ -5167,34 +5166,6 @@ prefs_changed_callback (MetaPreference pref,
     {
       meta_bell_set_audible (display, meta_prefs_bell_is_audible ());
     }
-  else if (pref == META_PREF_ATTACH_MODAL_DIALOGS)
-    {
-      MetaDisplay *display = data;
-      GSList *windows;
-      GSList *tmp;
-
-      windows = meta_display_list_windows (display, META_LIST_DEFAULT);
-
-      for (tmp = windows; tmp != NULL; tmp = tmp->next)
-        {
-          MetaWindow *w = tmp->data;
-          MetaWindow *parent = meta_window_get_transient_for (w);
-          meta_window_recalc_features (w);
-
-          if (w->type == META_WINDOW_MODAL_DIALOG && parent)
-            {
-              int x, y;
-              /* Forcing a call to move_resize() does two things: first, it handles
-               * resizing the dialog frame window to the correct size when we remove
-               * or add the decorations. Second, it will take care of positioning the
-               * dialog as "attached" to the parent when we turn the preference on
-               * via the constrain_modal_dialog() constraint.
-               **/
-              meta_window_get_position (w, &x, &y);
-              meta_window_move (w, FALSE, x, y);
-            }
-        }
-    }
 }
 
 void
diff --git a/src/core/window-private.h b/src/core/window-private.h
index 3f7d882..1f6c85e 100644
--- a/src/core/window-private.h
+++ b/src/core/window-private.h
@@ -318,6 +318,9 @@ struct _MetaWindow
   /* if TRUE, application is buggy and SYNC resizing is turned off */
   guint disable_sync : 1;
 
+  /* if TRUE, window is attached to its parent */
+  guint attached : 1;
+
   /* if non-NULL, the bounds of the window frame */
   cairo_region_t *frame_bounds;
 
@@ -643,4 +646,6 @@ void meta_window_update_on_all_workspaces (MetaWindow *window);
 void meta_window_propagate_focus_appearance (MetaWindow *window,
                                              gboolean    focused);
 
+gboolean meta_window_should_attach_to_parent (MetaWindow *window);
+
 #endif
diff --git a/src/core/window-props.c b/src/core/window-props.c
index 42438c2..837e055 100644
--- a/src/core/window-props.c
+++ b/src/core/window-props.c
@@ -1469,44 +1469,48 @@ reload_transient_for (MetaWindow    *window,
                       gboolean       initial)
 {
   MetaWindow *parent = NULL;
+  Window transient_for, old_transient_for;
 
-  if (meta_window_appears_focused (window) && window->xtransient_for != None)
-    meta_window_propagate_focus_appearance (window, FALSE);
-
-  window->xtransient_for = None;
-  
   if (value->type != META_PROP_VALUE_INVALID)
-    window->xtransient_for = value->v.xwindow;
-
-  /* Make sure transient_for is valid */
-  if (window->xtransient_for != None)
     {
-      parent = meta_display_lookup_x_window (window->display,
-                                             window->xtransient_for);
+      transient_for = value->v.xwindow;
+
+      parent = meta_display_lookup_x_window (window->display, transient_for);
       if (!parent)
         {
           meta_warning (_("Invalid WM_TRANSIENT_FOR window 0x%lx specified "
                           "for %s.\n"),
-                        window->xtransient_for, window->desc);
-          window->xtransient_for = None;
+                        transient_for, window->desc);
+          transient_for = None;
         }
-    }
 
-  /* Make sure there is not a loop */
-  while (parent)
-    {
-      if (parent == window)
+      /* Make sure there is not a loop */
+      while (parent)
         {
-          meta_warning (_("WM_TRANSIENT_FOR window 0x%lx for %s "
-                          "would create loop.\n"),
-                        window->xtransient_for, window->desc);
-          window->xtransient_for = None;
-          break;
+          if (parent == window)
+            {
+              meta_warning (_("WM_TRANSIENT_FOR window 0x%lx for %s "
+                              "would create loop.\n"),
+                            transient_for, window->desc);
+              transient_for = None;
+              break;
+            }
+
+          parent = meta_display_lookup_x_window (parent->display,
+                                                 parent->xtransient_for);
         }
-
-      parent = meta_display_lookup_x_window (parent->display,
-                                             parent->xtransient_for);
     }
+  else
+    transient_for = None;
+
+  if (transient_for == window->xtransient_for)
+    return;
+
+  if (meta_window_appears_focused (window) && window->xtransient_for != None)
+    meta_window_propagate_focus_appearance (window, FALSE);
+
+  old_transient_for = window->xtransient_for;
+  window->xtransient_for = transient_for;
 
   window->transient_parent_is_root_window =
     window->xtransient_for == window->screen->xroot;
@@ -1520,6 +1524,25 @@ reload_transient_for (MetaWindow    *window,
   /* may now be a dialog */
   meta_window_recalc_window_type (window);
 
+  if (!window->constructing)
+    {
+      /* If the window attaches, detaches, or changes attached
+       * parents, we need to destroy the MetaWindow and let a new one
+       * be created (which happens as a side effect of
+       * meta_window_unmanage()). The condition below is correct
+       * because we know window->xtransient_for has changed.
+       */
+      if (window->attached || meta_window_should_attach_to_parent (window))
+        {
+          guint32 timestamp;
+
+          window->xtransient_for = old_transient_for;
+          timestamp = meta_display_get_current_time_roundtrip (window->display);
+          meta_window_unmanage (window, timestamp);
+          return;
+        }
+    }
+
   /* update stacking constraints */
   if (!window->override_redirect)
     meta_stack_update_transient (window->screen->stack, window);
diff --git a/src/core/window.c b/src/core/window.c
index 43da56c..0d19855 100644
--- a/src/core/window.c
+++ b/src/core/window.c
@@ -669,6 +669,22 @@ maybe_filter_window (MetaDisplay       *display,
   return filtered;
 }
 
+gboolean
+meta_window_should_attach_to_parent (MetaWindow *window)
+{
+  MetaWindow *parent;
+
+  if (!meta_prefs_get_attach_modal_dialogs () ||
+      window->type != META_WINDOW_MODAL_DIALOG)
+    return FALSE;
+
+  parent = meta_window_get_transient_for (window);
+  if (!parent)
+    return FALSE;
+
+  return TRUE;
+}
+
 MetaWindow*
 meta_window_new_with_attrs (MetaDisplay       *display,
                             Window             xwindow,
@@ -1134,6 +1150,10 @@ meta_window_new_with_attrs (MetaDisplay       *display,
         meta_display_get_current_time_roundtrip (window->display);
   }
 
+  window->attached = meta_window_should_attach_to_parent (window);
+  if (window->attached)
+    recalc_window_features (window);
+
   if (window->decorated)
     meta_window_ensure_frame (window);
 
@@ -1490,6 +1510,24 @@ meta_window_apply_session_info (MetaWindow *window,
     }
 }
 
+static gboolean
+detach_foreach_func (MetaWindow *window,
+                     void       *data)
+{
+  GList **children = data;
+  MetaWindow *parent;
+
+  if (window->attached)
+    {
+      /* Only return the immediate children of the window being unmanaged */
+      parent = meta_window_get_transient_for (window);
+      if (parent->unmanaging)
+        *children = g_list_prepend (*children, window);
+    }
+
+  return TRUE;
+}
+
 void
 meta_window_unmanage (MetaWindow  *window,
                       guint32      timestamp)
@@ -1520,6 +1558,21 @@ meta_window_unmanage (MetaWindow  *window,
 
   window->unmanaging = TRUE;
 
+  if (meta_prefs_get_attach_modal_dialogs ())
+    {
+      GList *attached_children = NULL, *iter;
+
+      /* Detach any attached dialogs by unmapping and letting them
+       * be remapped after @window is destroyed.
+       */
+      meta_window_foreach_transient (window,
+                                     detach_foreach_func,
+                                     &attached_children);
+      for (iter = attached_children; iter; iter = iter->next)
+        meta_window_unmanage (iter->data, timestamp);
+      g_list_free (attached_children);
+    }
+
   if (window->fullscreen)
     {
       MetaGroup *group;
@@ -1678,8 +1731,12 @@ meta_window_unmanage (MetaWindow  *window,
           meta_error_trap_pop (window->display);
         }
 
-      /* And we need to be sure the window is mapped so other WMs
-       * know that it isn't Withdrawn
+      /* If we're unmanaging a window that is not withdrawn, then
+       * either (a) mutter is exiting, in which case we need to map
+       * the window so the next WM will know that it's not Withdrawn,
+       * or (b) we want to create a new MetaWindow to replace the
+       * current one, which will happen automatically if we re-map
+       * the X Window.
        */
       meta_error_trap_push (window->display);
       XMapWindow (window->display->xdisplay,
@@ -4238,12 +4295,10 @@ send_sync_request (MetaWindow *window)
 #endif
 
 static gboolean
-move_attached_dialog (MetaWindow *window,
-                      void       *data)
+maybe_move_attached_dialog (MetaWindow *window,
+                            void       *data)
 {
-  MetaWindow *parent = meta_window_get_transient_for (window);
-
-  if (window->type == META_WINDOW_MODAL_DIALOG && parent)
+  if (meta_window_is_attached_dialog (window))
     /* It ignores x,y for such a dialog  */
     meta_window_move (window, FALSE, 0, 0);
 
@@ -4765,8 +4820,7 @@ meta_window_move_resize_internal (MetaWindow          *window,
       window->frame_bounds = NULL;
     }
 
-  if (meta_prefs_get_attach_modal_dialogs ())
-    meta_window_foreach_transient (window, move_attached_dialog, NULL);
+  meta_window_foreach_transient (window, maybe_move_attached_dialog, NULL);
 }
 
 /**
@@ -6491,14 +6545,11 @@ meta_window_propagate_focus_appearance (MetaWindow *window,
 {
   MetaWindow *child, *parent, *focus_window;
 
-  if (!meta_prefs_get_attach_modal_dialogs ())
-    return;
-
   focus_window = window->display->focus_window;
 
   child = window;
   parent = meta_window_get_transient_for (child);
-  while (parent && (!focused || child->type == META_WINDOW_MODAL_DIALOG))
+  while (parent && (!focused || meta_window_is_attached_dialog (child)))
     {
       gboolean child_focus_state_changed;
 
@@ -7636,12 +7687,8 @@ recalc_window_features (MetaWindow *window)
   if (window->type == META_WINDOW_TOOLBAR)
     window->decorated = FALSE;
 
-  if (window->type == META_WINDOW_MODAL_DIALOG && meta_prefs_get_attach_modal_dialogs ())
-    {
-      MetaWindow *parent = meta_window_get_transient_for (window);
-      if (parent)
-        window->border_only = TRUE;
-    }
+  if (meta_window_is_attached_dialog (window))
+    window->border_only = TRUE;
 
   if (window->type == META_WINDOW_DESKTOP ||
       window->type == META_WINDOW_DOCK ||
@@ -8626,9 +8673,7 @@ update_resize (MetaWindow *window,
    * size changes apply to both sides, so that the dialog
    * remains centered to the parent.
    */
-  if (window->type == META_WINDOW_MODAL_DIALOG &&
-      meta_prefs_get_attach_modal_dialogs () &&
-      meta_window_get_transient_for (window) != NULL)
+  if (meta_window_is_attached_dialog (window))
     dx *= 2;
 
   new_w = window->display->grab_anchor_window_pos.width;
@@ -10240,8 +10285,7 @@ meta_window_get_frame_type (MetaWindow *window)
       break;
 
     case META_WINDOW_MODAL_DIALOG:
-      if (meta_prefs_get_attach_modal_dialogs () &&
-          meta_window_get_transient_for (window) != NULL)
+      if (meta_window_is_attached_dialog (window))
         base_type = META_FRAME_TYPE_ATTACHED;
       else
         base_type = META_FRAME_TYPE_MODAL_DIALOG;
@@ -10307,3 +10351,19 @@ meta_window_get_frame_bounds (MetaWindow *window)
 
   return window->frame_bounds;
 }
+
+/**
+ * meta_window_is_attached_dialog:
+ * @window: a #MetaWindow
+ *
+ * Tests if @window is should be attached to its parent window.
+ * (If the "attach_modal_dialogs" option is not enabled, this will
+ * always return %FALSE.)
+ *
+ * Return value: whether @window should be attached to its parent
+ */
+gboolean
+meta_window_is_attached_dialog (MetaWindow *window)
+{
+  return window->attached;
+}
diff --git a/src/meta/window.h b/src/meta/window.h
index 1b4b004..e5ac5b9 100644
--- a/src/meta/window.h
+++ b/src/meta/window.h
@@ -154,6 +154,7 @@ int         meta_window_get_pid (MetaWindow *window);
 const char *meta_window_get_client_machine (MetaWindow *window);
 gboolean    meta_window_is_remote (MetaWindow *window);
 gboolean    meta_window_is_modal (MetaWindow *window);
+gboolean    meta_window_is_attached_dialog (MetaWindow *window);
 const char *meta_window_get_mutter_hints (MetaWindow *window);
 
 MetaFrameType meta_window_get_frame_type (MetaWindow *window);