[mutter] window-actor: Handle changing surface actor on window reparenting

[Jonas Ådahl <jadahl src gnome org>]

commit 2f27b8d5fa9cbe3798a74ce804c48f901768720c
Author: Jonas Ådahl <jadahl gmail com>
Date:   Wed Sep 4 16:36:52 2019 +0200

    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

 src/compositor/meta-window-actor.c | 15 ++++++++++++---
 1 file 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);