Fix a crash in animation groups when deleting uncontrolled animations

The problem was that we were not removing their references from the
private object hash and at some point we could access it.

Task-number: QTBUG-8910
Reviewed-by: gabi

6 files changed, 35 insertions, 16 deletions

diff --git a/src/corelib/animation/qanimationgroup.cpp b/src/corelib/animation/qanimationgroup.cpp
index a89f9497d8..2decda3408 100644
--- a/src/corelib/animation/qanimationgroup.cpp
+++ b/src/corelib/animation/qanimationgroup.cpp
@@ -244,7 +244,7 @@ QAbstractAnimation *QAnimationGroup::takeAnimation(int index)
     // in ChildRemoved event
     d->animations.removeAt(index);
     animation->setParent(0);
-    d->animationRemovedAt(index);
+    d->animationRemoved(index, animation);
     return animation;
 }
 
@@ -285,7 +285,7 @@ bool QAnimationGroup::event(QEvent *event)
 }
 
 
-void QAnimationGroupPrivate::animationRemovedAt(int index)
+void QAnimationGroupPrivate::animationRemoved(int index, QAbstractAnimation *)
 {
     Q_Q(QAnimationGroup);
     Q_UNUSED(index);
diff --git a/src/corelib/animation/qanimationgroup_p.h b/src/corelib/animation/qanimationgroup_p.h
index 394773b0dd..45603b3bdf 100644
--- a/src/corelib/animation/qanimationgroup_p.h
+++ b/src/corelib/animation/qanimationgroup_p.h
@@ -72,8 +72,19 @@ public:
         isGroup = true;
     }
 
-    virtual void animationInsertedAt(int index) { Q_UNUSED(index) };
-    virtual void animationRemovedAt(int index);
+    virtual void animationInsertedAt(int) { }
+    virtual void animationRemoved(int, QAbstractAnimation *);
+
+    void disconnectUncontrolledAnimation(QAbstractAnimation *anim)
+    {
+        //0 for the signal here because we might be called from the animation destructor
+        QObject::disconnect(anim, 0, q_func(), SLOT(_q_uncontrolledAnimationFinished()));
+    }
+
+    void connectUncontrolledAnimation(QAbstractAnimation *anim)
+    {
+        QObject::connect(anim, SIGNAL(finished()), q_func(), SLOT(_q_uncontrolledAnimationFinished()));
+    }
 
     QList<QAbstractAnimation *> animations;
 };
diff --git a/src/corelib/animation/qparallelanimationgroup.cpp b/src/corelib/animation/qparallelanimationgroup.cpp
index eaa036435d..280afed28b 100644
--- a/src/corelib/animation/qparallelanimationgroup.cpp
+++ b/src/corelib/animation/qparallelanimationgroup.cpp
@@ -246,11 +246,9 @@ void QParallelAnimationGroupPrivate::_q_uncontrolledAnimationFinished()
 
 void QParallelAnimationGroupPrivate::disconnectUncontrolledAnimations()
 {
-    Q_Q(QParallelAnimationGroup);
-
     QHash<QAbstractAnimation *, int>::iterator it = uncontrolledFinishTime.begin();
     while (it != uncontrolledFinishTime.end()) {
-        QObject::disconnect(it.key(), SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished()));
+        disconnectUncontrolledAnimation(it.key());
         ++it;
     }
 
@@ -259,13 +257,11 @@ void QParallelAnimationGroupPrivate::disconnectUncontrolledAnimations()
 
 void QParallelAnimationGroupPrivate::connectUncontrolledAnimations()
 {
-    Q_Q(QParallelAnimationGroup);
-
     for (int i = 0; i < animations.size(); ++i) {
         QAbstractAnimation *animation = animations.at(i);
         if (animation->duration() == -1 || animation->loopCount() < 0) {
             uncontrolledFinishTime[animation] = -1;
-            QObject::connect(animation, SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished()));
+            connectUncontrolledAnimation(animation);
         }
     }
 }
@@ -305,6 +301,13 @@ bool QParallelAnimationGroupPrivate::isUncontrolledAnimationFinished(QAbstractAn
     return uncontrolledFinishTime.value(anim, -1) >= 0;
 }
 
+void QParallelAnimationGroupPrivate::animationRemoved(int index, QAbstractAnimation *anim)
+{
+    QAnimationGroupPrivate::animationRemoved(index, anim);
+    disconnectUncontrolledAnimation(anim);
+    uncontrolledFinishTime.remove(anim);
+}
+
 /*!
     \reimp
 */
diff --git a/src/corelib/animation/qparallelanimationgroup_p.h b/src/corelib/animation/qparallelanimationgroup_p.h
index a74d496bdd..cab4fa9440 100644
--- a/src/corelib/animation/qparallelanimationgroup_p.h
+++ b/src/corelib/animation/qparallelanimationgroup_p.h
@@ -80,6 +80,8 @@ public:
     void connectUncontrolledAnimations();
     void disconnectUncontrolledAnimations();
 
+    void animationRemoved(int index, QAbstractAnimation *);
+
     // private slot
     void _q_uncontrolledAnimationFinished();
 };
diff --git a/src/corelib/animation/qsequentialanimationgroup.cpp b/src/corelib/animation/qsequentialanimationgroup.cpp
index 8ab084a2bc..7617c1f0b1 100644
--- a/src/corelib/animation/qsequentialanimationgroup.cpp
+++ b/src/corelib/animation/qsequentialanimationgroup.cpp
@@ -479,7 +479,7 @@ void QSequentialAnimationGroupPrivate::activateCurrentAnimation(bool intermediat
 
     // connects to the finish signal of uncontrolled animations
     if (currentAnimation->totalDuration() == -1)
-        QObject::connect(currentAnimation, SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished()));
+        connectUncontrolledAnimation(currentAnimation);
 
     currentAnimation->start();
     if (!intermediate && state == QSequentialAnimationGroup::Paused)
@@ -496,7 +496,7 @@ void QSequentialAnimationGroupPrivate::_q_uncontrolledAnimationFinished()
         actualDuration.append(-1);
     actualDuration[currentAnimationIndex] = currentAnimation->currentTime();
 
-    QObject::disconnect(currentAnimation, SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished()));
+    disconnectUncontrolledAnimation(currentAnimation);
 
     if ((direction == QAbstractAnimation::Forward && currentAnimation == animations.last())
         || (direction == QAbstractAnimation::Backward && currentAnimationIndex == 0)) {
@@ -543,10 +543,10 @@ void QSequentialAnimationGroupPrivate::animationInsertedAt(int index)
     the group at index \a index. The animation is no more listed when this
     method is called.
 */
-void QSequentialAnimationGroupPrivate::animationRemovedAt(int index)
+void QSequentialAnimationGroupPrivate::animationRemoved(int index, QAbstractAnimation *anim)
 {
     Q_Q(QSequentialAnimationGroup);
-    QAnimationGroupPrivate::animationRemovedAt(index);
+    QAnimationGroupPrivate::animationRemoved(index, anim);
 
     Q_ASSERT(currentAnimation); // currentAnimation should always be set
 
@@ -555,7 +555,10 @@ void QSequentialAnimationGroupPrivate::animationRemovedAt(int index)
 
     const int currentIndex = animations.indexOf(currentAnimation);
     if (currentIndex == -1) {
-        //we're removing the current animation, let's update it to another one
+        //we're removing the current animation
+
+        disconnectUncontrolledAnimation(currentAnimation);
+
         if (index < animations.count())
             setCurrentAnimation(index); //let's try to take the next one
         else if (index > 0)
diff --git a/src/corelib/animation/qsequentialanimationgroup_p.h b/src/corelib/animation/qsequentialanimationgroup_p.h
index a55e136b5c..de69916745 100644
--- a/src/corelib/animation/qsequentialanimationgroup_p.h
+++ b/src/corelib/animation/qsequentialanimationgroup_p.h
@@ -85,7 +85,7 @@ public:
     void activateCurrentAnimation(bool intermediate = false);
 
     void animationInsertedAt(int index);
-    void animationRemovedAt(int index);
+    void animationRemoved(int index, QAbstractAnimation *anim);
 
     bool atEnd() const;