window-actor: Handle changing surface actor on window reparenting

The commit f2f4af0d50ba9f2d36cf225162a30928f55d7bd6 missed one situation
where mutter does things differently, i.e. changes what surface actor is
associated with a given window actor: reparenting a Xwayland window when
changing whether it is decorated.

To summarize, there are three types of window actors:

X11 window actors - directly tied to the backing X11 window. The
corresponding surface actor is directly owned by the window actor and
will never change.

Wayland window actors - gets its surface actor from MetaWaylandSurface
at construction. A single MetaWaylandSurface may create and destroy
multiple window actors over time, but a single window actor will never
change surface actor.

Xwayland window actors - a mix between the above two types; the window
corresponds to the X11 window, and so does the window actor, but the
surface itself comes from the MetaWaylandSurface.

Normally when a X11 window is unmapped, the corresponding MetaWindow is
unmanaged. With Xwayland, this happens indirectly via the destruction of
the wl_surface. The exception to this is windows that are reparented
during changing their decoration state - in this case on plain X11, the
MetaWindow stays alive. With Xwayland however, there is a race
condition; since the MetaWindow is tied to the wl_surface, if we receive
the new surface ID atom before the destruction of the old wl_surface,
we'll try to associate the existing MetaWindow and MetaWindowActor with
the new wl_surface, hitting the assert. If the surface destruction
arrives first, the MetaWindow and MetaWindowActor will be disposed, and
the we wouldn't hit the assert.

To handle this race gracefully, reinstate handling of replacing the
surface actor of an existing window actor, to handle this race, as it
was handled before.

Eventually, it should be reconsidered whether the MetaWindow lifetime is
tied to the wl_surface or if it should be changed to be consistent with
plain X11, as this re-exposes another bug where the X11 client and
mutter will enter a feedback loop where the window is repeatedly
remapped. See https://gitlab.freedesktop.org/xorg/xserver/issues/740.

Fixes: https://gitlab.gnome.org/GNOME/mutter/issues/709

https://gitlab.gnome.org/GNOME/mutter/merge_requests/773

1 files changed, 12 insertions, 3 deletions

diff --git a/src/compositor/meta-window-actor.c b/src/compositor/meta-window-actor.c
index 24e86dee8..53f46db7b 100644
--- a/src/compositor/meta-window-actor.c
+++ b/src/compositor/meta-window-actor.c
@@ -83,7 +83,7 @@ typedef struct _MetaWindowActorPrivate
 
   int geometry_scale;
 
-  guint             size_changed_id;
+  gulong            size_changed_id;
 
   /*
    * These need to be counters rather than flags, since more plugins
@@ -386,7 +386,16 @@ meta_window_actor_real_assign_surface_actor (MetaWindowActor  *self,
   MetaWindowActorPrivate *priv =
     meta_window_actor_get_instance_private (self);
 
-  g_assert (!priv->surface);
+  if (priv->surface)
+    {
+      g_warn_if_fail (priv->window->client_type == META_WINDOW_CLIENT_TYPE_X11 &&
+                      meta_is_wayland_compositor ());
+
+      g_clear_signal_handler (&priv->size_changed_id, priv->surface);
+      clutter_actor_remove_child (CLUTTER_ACTOR (self),
+                                  CLUTTER_ACTOR (priv->surface));
+      g_clear_object (&priv->surface);
+    }
 
   priv->surface = g_object_ref_sink (surface_actor);
   priv->size_changed_id = g_signal_connect (priv->surface, "size-changed",
@@ -491,7 +500,7 @@ meta_window_actor_dispose (GObject *object)
 
   if (priv->surface)
     {
-      g_signal_handler_disconnect (priv->surface, priv->size_changed_id);
+      g_clear_signal_handler (&priv->size_changed_id, priv->surface);
       clutter_actor_remove_child (CLUTTER_ACTOR (self),
                                   CLUTTER_ACTOR (priv->surface));
       g_clear_object (&priv->surface);