wayland/subsurface: Move actor unparenting back to rebuild_surface_tree()

Unparenting the surface actor when the subsurface object is destroyed
has several issues:

subsurface actors can be unparented while a close animation is
still ongoing, breaking the animation for e.g. Firefox.
adding and removing the actor to/from the parent is not handled in
one place, making the code harder to follow.
if the destroyed subsurface had children of its own, they potentially
stick around until a surface-tree rebuild. This makes the Firefox
hamburger menu not close with the "compositor" backend.

Move the unparenting back to
meta_window_actor_wayland_rebuild_surface_tree() and instead just
notify the parent of a state change, if it still exist. This will ensure
a correct mapping between the subsurface node tree and the flat surface
actor list. In case of the closing animation the parent will already be
removed and the call is skipped.

Author: Sunderland93

src/compositor/meta-window-actor-wayland.c

@@ -39,28 +39,43 @@ typedef struct _SurfaceTreeTraverseData
 } SurfaceTreeTraverseData;
 
 static gboolean
-set_surface_actor_index (GNode    *node,
-                         gpointer  data)
+get_surface_actor_list (GNode    *node,
+                        gpointer  data)
 {
   MetaWaylandSurface *surface = node->data;
   MetaSurfaceActor *surface_actor = meta_wayland_surface_get_actor (surface);
+  GList **surface_actors = data;
+
+  *surface_actors = g_list_prepend (*surface_actors, surface_actor);
+  return FALSE;
+}
 
+static gboolean
+set_surface_actor_index (GNode    *node,
+                         gpointer  data)
+{
+  MetaWaylandSurface *surface = node->data;
   SurfaceTreeTraverseData *traverse_data = data;
 
-  if (clutter_actor_contains (CLUTTER_ACTOR (traverse_data->window_actor),
-                              CLUTTER_ACTOR (surface_actor)))
+  ClutterActor *window_actor = CLUTTER_ACTOR (traverse_data->window_actor);
+  ClutterActor *surface_actor =
+    CLUTTER_ACTOR (meta_wayland_surface_get_actor (surface));
+
+  if (clutter_actor_contains (window_actor, surface_actor))
     {
-      clutter_actor_set_child_at_index (
-        CLUTTER_ACTOR (traverse_data->window_actor),
-        CLUTTER_ACTOR (surface_actor),
-        traverse_data->index);
+      if (clutter_actor_get_child_at_index (window_actor, traverse_data->index) !=
+          surface_actor)
+        {
+          clutter_actor_set_child_at_index (window_actor,
+                                            surface_actor,
+                                            traverse_data->index);
+        }
     }
   else
     {
-      clutter_actor_insert_child_at_index (
-        CLUTTER_ACTOR (traverse_data->window_actor),
-        CLUTTER_ACTOR (surface_actor),
-        traverse_data->index);
+      clutter_actor_insert_child_at_index (window_actor,
+                                           surface_actor,
+                                           traverse_data->index);
     }
   traverse_data->index++;
 
@@ -75,8 +90,28 @@ meta_window_actor_wayland_rebuild_surface_tree (MetaWindowActor *actor)
   MetaWaylandSurface *surface = meta_surface_actor_wayland_get_surface (
     META_SURFACE_ACTOR_WAYLAND (surface_actor));
   GNode *root_node = surface->subsurface_branch_node;
+  g_autoptr (GList) surface_actors = NULL;
+  g_autoptr (GList) children = NULL;
+  GList *l;
   SurfaceTreeTraverseData traverse_data;
 
+  g_node_traverse (root_node,
+                   G_IN_ORDER,
+                   G_TRAVERSE_LEAVES,
+                   -1,
+                   get_surface_actor_list,
+                   &surface_actors);
+
+  children = clutter_actor_get_children (CLUTTER_ACTOR (actor));
+  for (l = children; l; l = l->next)
+    {
+      ClutterActor *child_actor = l->data;
+
+      if (META_IS_SURFACE_ACTOR_WAYLAND (child_actor) &&
+          !g_list_find (surface_actors, child_actor))
+        clutter_actor_remove_child (CLUTTER_ACTOR (actor), child_actor);
+    }
+
   traverse_data = (SurfaceTreeTraverseData) {
     .window_actor = actor,
     .index = 0,

src/wayland/meta-wayland-subsurface.c

@@ -281,31 +281,16 @@ meta_wayland_subsurface_class_init (MetaWaylandSubsurfaceClass *klass)
     meta_wayland_subsurface_sync_actor_state;
 }
 
-static void
-unparent_actor (MetaWaylandSurface *surface)
-{
-  ClutterActor *actor;
-  ClutterActor *parent_actor;
-
-  actor = CLUTTER_ACTOR (meta_wayland_surface_get_actor (surface));
-  if (!actor)
-    return;
-
-  parent_actor = clutter_actor_get_parent (actor);
-  if (parent_actor)
-    clutter_actor_remove_child (parent_actor, actor);
-}
-
 static void
 wl_subsurface_destructor (struct wl_resource *resource)
 {
   MetaWaylandSurface *surface = wl_resource_get_user_data (resource);
 
   g_node_unlink (surface->subsurface_branch_node);
-  unparent_actor (surface);
 
   if (surface->sub.parent)
     {
+      meta_wayland_surface_notify_subsurface_state_changed (surface->sub.parent);
       wl_list_remove (&surface->sub.parent_destroy_listener.link);
       surface->sub.parent = NULL;
     }