Blob Blame History Raw
From be7ac5afa6227484b997a1770dab01a42ea2a035 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 1/2] 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
(cherry picked from commit f376a318ba90fc29d3d661df4f55698459f31cfa)
---
 clutter/clutter/clutter-actor.c       |  3 ++-
 clutter/tests/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 7111d824d3..0e531eea52 100644
--- a/clutter/clutter/clutter-actor.c
+++ b/clutter/clutter/clutter-actor.c
@@ -4315,7 +4315,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))
     g_signal_emit (child, actor_signals[PARENT_SET], 0, self);
 
   /* if the child was mapped then we need to relayout ourselves to account
diff --git a/clutter/tests/conform/actor-destroy.c b/clutter/tests/conform/actor-destroy.c
index 03092a0108..14ad1a25c8 100644
--- a/clutter/tests/conform/actor-destroy.c
+++ b/clutter/tests/conform/actor-destroy.c
@@ -159,15 +159,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);
 
@@ -179,6 +192,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 ())
@@ -186,6 +201,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);
 }
-- 
2.28.0