clutter/actor: First clear all stage views before emitting they changed

If one would end up with an actor attached to mapped actor, where the
attached actor doesn't itself have an up to date stage view list while
listening on the stage for updating, when clearing the stage views of
the list, anything that would query the stage views list at this time
would end up accessing freed memory.

This could happen if

 1) An actor was added to a newly created container actor attached to
    the stage
 2) The actor got a timeline attached to it
 3) The actor was moved to a container that already was mapped
 4) A hotplug happened

After (1) both the container and actor would not have any stage views.
After (2) the timeline would listen on the stage for stage views
updates. After (3) the actor would still listen on the stage for stage
views updates. When (4) happened, the actor would be signalled when the
stage got its stage view cleared, at which point it would traverse up
its actor's tree finding an appropriate stage view to base its animation
on. The problem here would be that it'd query the already mapped
container and its yet-to-be-cleared stage view list, resulting in
use-after free, resulting in for example the following backtrace:

  0)  g_type_check_instance_cast ()
  1)  CLUTTER_STAGE_VIEW ()
  2)  clutter_actor_pick_frame_clock ()
  3)  clutter_actor_pick_frame_clock ()
  4)  update_frame_clock ()
  5)  on_frame_clock_actor_stage_views_changed ()
  6)  g_closure_invoke ()
  7)  signal_emit_unlocked_R ()
  8)  g_signal_emit_valist ()
  9)  g_signal_emit ()
  10) clear_stage_views_cb ()
  11) _clutter_actor_traverse_depth ()
  12) _clutter_actor_traverse ()
  13) clutter_actor_clear_stage_views_recursive ()
  14) clutter_stage_clear_stage_views ()
  ...

Avoid this issue by making sure that we don't emit 'stage-views-changed'
signals while the actor tree is in an invalid state. While we now end up
traversing tree twice, it doesn't change the Big-O notation. It has not
been measured whether this has any noticible performance impact.

Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/1950
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2025>

clutter/clutter/clutter-actor.c

@@ -843,6 +843,7 @@ struct _ClutterActorPrivate
   guint needs_paint_volume_update   : 1;
   guint had_effects_on_last_paint_volume_update : 1;
   guint needs_update_stage_views    : 1;
+  guint clear_stage_views_needs_stage_views_changed : 1;
 };
 
 enum
@@ -15708,7 +15709,21 @@ clear_stage_views_cb (ClutterActor *actor,
   old_stage_views = g_steal_pointer (&actor->priv->stage_views);
 
   if (old_stage_views)
-    g_signal_emit (actor, actor_signals[STAGE_VIEWS_CHANGED], 0);
+    actor->priv->clear_stage_views_needs_stage_views_changed = TRUE;
+
+  return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE;
+}
+
+static ClutterActorTraverseVisitFlags
+maybe_emit_stage_views_changed_cb (ClutterActor *actor,
+                                   int           depth,
+                                   gpointer      user_data)
+{
+  if (actor->priv->clear_stage_views_needs_stage_views_changed)
+    {
+      actor->priv->clear_stage_views_needs_stage_views_changed = FALSE;
+      g_signal_emit (actor, actor_signals[STAGE_VIEWS_CHANGED], 0);
+    }
 
   return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE;
 }
@@ -15721,6 +15736,11 @@ clutter_actor_clear_stage_views_recursive (ClutterActor *self)
                            clear_stage_views_cb,
                            NULL,
                            NULL);
+  _clutter_actor_traverse (self,
+                           CLUTTER_ACTOR_TRAVERSE_DEPTH_FIRST,
+                           maybe_emit_stage_views_changed_cb,
+                           NULL,
+                           NULL);
 }
 
 float
@@ -16033,7 +16053,7 @@ clutter_actor_pick_frame_clock (ClutterActor  *self,
 
   for (l = stage_views_list; l; l = l->next)
     {
-      ClutterStageView *view = l->data;
+      ClutterStageView *view = CLUTTER_STAGE_VIEW (l->data);
       float refresh_rate;
 
       refresh_rate = clutter_stage_view_get_refresh_rate (view);

src/tests/stage-view-tests.c

@@ -1255,6 +1255,57 @@ meta_test_timeline_actor_destroyed (void)
   clutter_actor_destroy (persistent_actor);
 }
 
+static void
+meta_test_timeline_actor_tree_clear (void)
+{
+  ClutterActor *stage;
+  ClutterActor *container1;
+  ClutterActor *container2;
+  g_autoptr (ClutterActor) floating = NULL;
+  g_autoptr (ClutterTimeline) timeline = NULL;
+  GList *stage_views;
+
+  stage = meta_backend_get_stage (meta_context_get_backend (test_context));
+
+  ensure_view_count (1);
+
+  container1 = clutter_actor_new ();
+  clutter_actor_set_size (container1, 100, 100);
+  clutter_actor_add_child (stage, container1);
+
+  wait_for_paint (stage);
+
+  container2 = clutter_actor_new ();
+  clutter_actor_set_size (container2, 100, 100);
+  clutter_actor_add_child (stage, container2);
+
+  floating = g_object_ref_sink (clutter_actor_new ());
+  clutter_actor_set_size (floating, 100, 100);
+
+  clutter_actor_add_child (container2, floating);
+  timeline = clutter_timeline_new_for_actor (floating, 100);
+  clutter_actor_remove_child (container2, floating);
+
+  clutter_actor_add_child (container1, floating);
+
+  ensure_view_count (1);
+
+  is_on_stage_views (container1, 0);
+  is_on_stage_views (container2, 0);
+  is_on_stage_views (floating, 0);
+
+  wait_for_paint (stage);
+
+  stage_views = clutter_stage_peek_stage_views (CLUTTER_STAGE (stage));
+  is_on_stage_views (container1, 1, stage_views->data);
+  is_on_stage_views (container2, 1, stage_views->data);
+  is_on_stage_views (floating, 1, stage_views->data);
+
+  clutter_actor_destroy (floating);
+  clutter_actor_destroy (container1);
+  clutter_actor_destroy (container2);
+}
+
 static void
 init_tests (void)
 {
@@ -1286,6 +1337,8 @@ init_tests (void)
                    meta_test_actor_stage_views_queue_frame_drawn);
   g_test_add_func ("/stage-views/timeline/actor-destroyed",
                    meta_test_timeline_actor_destroyed);
+  g_test_add_func ("/stage-views/timeline/tree-clear",
+                   meta_test_timeline_actor_tree_clear);
 }
 
 int