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

@@ -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 ())
+  if (!meta_window_is_attached_dialog (window))
-    return TRUE;
-  if (window->type != META_WINDOW_MODAL_DIALOG || !parent)
     return TRUE;
 
   x = parent->rect.x + (parent->rect.width / 2  - info->current.width / 2);

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 () &&
+  if (window && meta_window_is_attached_dialog (window) &&
-      window && window->type == META_WINDOW_MODAL_DIALOG &&
       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

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

src/core/window-props.c

@@ -1469,27 +1469,19 @@ 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,
+      transient_for = value->v.xwindow;
-                                             window->xtransient_for);
+
+      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);
+                        transient_for, window->desc);
-          window->xtransient_for = None;
+          transient_for = None;
-        }
         }
 
       /* Make sure there is not a loop */
@@ -1499,14 +1491,26 @@ reload_transient_for (MetaWindow    *window,
             {
               meta_warning (_("WM_TRANSIENT_FOR window 0x%lx for %s "
                               "would create loop.\n"),
-                        window->xtransient_for, window->desc);
+                            transient_for, window->desc);
-          window->xtransient_for = None;
+              transient_for = None;
               break;
             }
 
           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);

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
+      /* If we're unmanaging a window that is not withdrawn, then
-       * know that it isn't Withdrawn
+       * 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,
+maybe_move_attached_dialog (MetaWindow *window,
                             void       *data)
 {
-  MetaWindow *parent = meta_window_get_transient_for (window);
+  if (meta_window_is_attached_dialog (window))
-
-  if (window->type == META_WINDOW_MODAL_DIALOG && parent)
     /* 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, maybe_move_attached_dialog, NULL);
-    meta_window_foreach_transient (window, 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 ())
+  if (meta_window_is_attached_dialog (window))
-    {
-      MetaWindow *parent = meta_window_get_transient_for (window);
-      if (parent)
     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 &&
+  if (meta_window_is_attached_dialog (window))
-      meta_prefs_get_attach_modal_dialogs () &&
-      meta_window_get_transient_for (window) != NULL)
     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 () &&
+      if (meta_window_is_attached_dialog (window))
-          meta_window_get_transient_for (window) != NULL)
         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;
+}

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);