From a4d1e6f6f117c477750c1df32bdab94596462ca2 Mon Sep 17 00:00:00 2001
From: Emmanuele Bassi <ebassi@gnome.org>
Date: Sun, 22 Jul 2012 10:52:46 +0100
Subject: [PATCH] actor: Ensure the invariants of transition-stopped

We need to make sure that ClutterActor::transition-stopped is emitted
after the transition was removed from the actor's list of transitions.

We cannot just remove the TransitionClosure from the hash table that
holds the transitions associated to an actor, and let the
TransitionClosure free function stop the timeline, thus emitting the
::transition-stopped signal - otherwise, stopping the timeline will end
up trying to remove the transition from the hash table, and we'll get
either a warning or a segfault.

Since we know we're removing the timeline anyway, we can emit the signal
ourselves in case the timeline is playing, in both the implicit and
explicit cases.
---
 clutter/clutter-actor.c | 66 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c
index ef13de2f6..910320ed7 100644
--- a/clutter/clutter-actor.c
+++ b/clutter/clutter-actor.c
@@ -3927,7 +3927,18 @@ _clutter_actor_stop_transitions (ClutterActor *self)
   while (g_hash_table_iter_next (&iter, NULL, &value))
     {
       TransitionClosure *closure = value;
-      clutter_timeline_stop (CLUTTER_TIMELINE (closure->transition));
+
+      /* if the transition is implicit we just remove it now */
+      if (closure->is_implicit)
+        g_hash_table_iter_remove (&iter);
+      else
+        {
+          /* otherwise we stop it, and the transition will be removed
+           * later, either by the actor's destruction or by explicit
+           * removal
+           */
+          clutter_timeline_stop (CLUTTER_TIMELINE (closure->transition));
+        }
     }
 }
 
@@ -18358,11 +18369,15 @@ transition_closure_free (gpointer data)
 
       timeline = CLUTTER_TIMELINE (clos->transition);
 
+      /* we disconnect the signal handler before stopping the timeline,
+       * so that we don't end up inside on_transition_stopped() from
+       * a call to g_hash_table_remove().
+       */
+      g_signal_handler_disconnect (clos->transition, clos->completed_id);
+
       if (clutter_timeline_is_playing (timeline))
         clutter_timeline_stop (timeline);
 
-      g_signal_handler_disconnect (clos->transition, clos->completed_id);
-
       g_object_unref (clos->transition);
       g_free (clos->name);
 
@@ -18386,9 +18401,6 @@ on_transition_stopped (ClutterTransition *transition,
   /* reset the caches used by animations */
   clutter_actor_store_content_box (actor, NULL);
 
-  if (!is_finished)
-    return;
-
   info = _clutter_actor_get_animation_info (actor);
 
   t_quark = g_quark_from_string (clos->name);
@@ -18403,6 +18415,10 @@ on_transition_stopped (ClutterTransition *transition,
        * the end of the frame processing.
        */
       g_object_ref (transition);
+
+      /* this is safe, because the timeline has now stopped,
+       * so we won't recurse
+       */
       g_hash_table_remove (info->transitions, clos->name);
     }
 
@@ -18412,7 +18428,7 @@ on_transition_stopped (ClutterTransition *transition,
    */
   g_signal_emit (actor, actor_signals[TRANSITION_STOPPED], t_quark,
                  t_name,
-                 TRUE);
+                 is_finished);
 
   g_free (t_name);
 
@@ -18709,6 +18725,10 @@ clutter_actor_remove_transition (ClutterActor *self,
                                  const char   *name)
 {
   const ClutterAnimationInfo *info;
+  TransitionClosure *clos;
+  gboolean was_playing;
+  GQuark t_quark;
+  gchar *t_name;
 
   g_return_if_fail (CLUTTER_IS_ACTOR (self));
   g_return_if_fail (name != NULL);
@@ -18718,12 +18738,36 @@ clutter_actor_remove_transition (ClutterActor *self,
   if (info->transitions == NULL)
     return;
 
-  g_signal_emit (self, actor_signals[TRANSITION_STOPPED],
-                 g_quark_from_string (name),
-                 name,
-                 FALSE);
+  clos = g_hash_table_lookup (info->transitions, name);
+
+  was_playing =
+    clutter_timeline_is_playing (CLUTTER_TIMELINE (clos->transition));
+  t_quark = g_quark_from_string (clos->name);
+  t_name = g_strdup (clos->name);
 
   g_hash_table_remove (info->transitions, name);
+
+  /* we want to maintain the invariant that ::transition-stopped is
+   * emitted after the transition has been removed, to allow replacing
+   * or chaining; removing the transition from the hash table will
+   * stop it, but transition_closure_free() will disconnect the signal
+   * handler we install in add_transition_internal(), to avoid loops
+   * or segfaults.
+   *
+   * since we know already that a transition will stop once it's removed
+   * from an actor, we can simply emit the ::transition-stopped here
+   * ourselves, if the timeline was playing (if it wasn't, then the
+   * signal was already emitted at least once).
+   */
+  if (was_playing)
+    {
+      g_signal_emit (self, actor_signals[TRANSITION_STOPPED],
+                     t_quark,
+                     t_name,
+                     FALSE);
+    }
+
+  g_free (t_name);
 }
 
 /**