From f376a318ba90fc29d3d661df4f55698459f31cfa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 10 Sep 2019 03:19:58 +0200
Subject: [PATCH] clutter/actor: Don't emit the parent-set signal on
 destruction

Clutter actors unset their parent on dispose, after emitting the ::destroy
signal, however this could cause ::parent-set signal emission. Since we
assume that after the destruction has been completed the actor isn't valid
anymore, and that during the destroy phase we do all the signal / source
disconnections, this might create unwanted behaviors, as in the signal
callbacks we always assume that the actor isn't in disposed yet.

To avoid this, don't emit ::parent-set signal if the actor is being
destroyed.

Update the actor-destroy test to verify this behavior.

https://gitlab.gnome.org/GNOME/mutter/merge_requests/769
---
 clutter/clutter/clutter-actor.c           |  3 ++-
 src/tests/clutter/conform/actor-destroy.c | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c
index 0aa77e3d6..313317de3 100644
--- a/clutter/clutter/clutter-actor.c
+++ b/clutter/clutter/clutter-actor.c
@@ -4436,7 +4436,8 @@ clutter_actor_remove_child_internal (ClutterActor                 *self,
     }
 
   /* clutter_actor_reparent() will emit ::parent-set for us */
-  if (emit_parent_set && !CLUTTER_ACTOR_IN_REPARENT (child))
+  if (emit_parent_set && !CLUTTER_ACTOR_IN_REPARENT (child) &&
+      !CLUTTER_ACTOR_IN_DESTRUCTION (child))
     {
       child->priv->needs_compute_resource_scale = TRUE;
       g_signal_emit (child, actor_signals[PARENT_SET], 0, self);
diff --git a/src/tests/clutter/conform/actor-destroy.c b/src/tests/clutter/conform/actor-destroy.c
index a0867fb27..3de0fef7d 100644
--- a/src/tests/clutter/conform/actor-destroy.c
+++ b/src/tests/clutter/conform/actor-destroy.c
@@ -161,15 +161,28 @@ on_destroy (ClutterActor *actor,
 {
   gboolean *destroy_called = data;
 
+  g_assert_true (CLUTTER_IS_ACTOR (clutter_actor_get_parent (actor)));
+
   *destroy_called = TRUE;
 }
 
+static void
+on_parent_set (ClutterActor *actor,
+               ClutterActor *old_parent,
+               gpointer      data)
+{
+  gboolean *parent_set_called = data;
+
+  *parent_set_called = TRUE;
+}
+
 static void
 actor_destruction (void)
 {
   ClutterActor *test = g_object_new (TEST_TYPE_DESTROY, NULL);
   ClutterActor *child = clutter_rectangle_new ();
   gboolean destroy_called = FALSE;
+  gboolean parent_set_called = FALSE;
 
   g_object_ref_sink (test);
 
@@ -181,6 +194,8 @@ actor_destruction (void)
 
   clutter_actor_set_name (child, "Child");
   clutter_container_add_actor (CLUTTER_CONTAINER (test), child);
+  g_signal_connect (child, "parent-set", G_CALLBACK (on_parent_set),
+                    &parent_set_called);
   g_signal_connect (child, "destroy", G_CALLBACK (on_destroy), &destroy_called);
 
   if (g_test_verbose ())
@@ -188,6 +203,7 @@ actor_destruction (void)
 
   clutter_actor_destroy (test);
   g_assert (destroy_called);
+  g_assert_false (parent_set_called);
   g_assert_null (child);
   g_assert_null (test);
 }