Fix signal emission order for zero-duration animations

The order for non-zero-duration animations are:

* started()
* runningChanged(true)
* stopped()
* runningChanged(false)
* finished()

This patch tries to ensure that zero-duration animations have the same signal
emission order.

The problem was that when setRunning(true) was called on zero-duration
animation, it would call itself (setRunning(false)) lower in the call stack in
order to immediately stop again. This had the implication that we could emit
stopped() even before started() was emitted (since the recursive call to
setRunning(false) would actually complete before the ancestor stack frame
setRunning(true) was completed)

To fix this we emit started() *before* we call start() on the animationInstance.

There is still a bug in that runningChanged(true) is still not emitted for a
zero-duration animation, but this patch should improve the current behavior in
the sense that stopped() is not emitted _before_ started().

Task-number: QTBUG-48193
Change-Id: Ic2bc85e648e6746f6a058e2e9136515e7fdb6192
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

1 files changed, 52 insertions, 0 deletions

diff --git a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp
index 55957fa71a..e62d49ed6b 100644
--- a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp
+++ b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp
@@ -89,6 +89,8 @@ private slots:
     void easingProperties();
     void rotation();
     void startStopSignals();
+    void signalOrder_data();
+    void signalOrder();
     void runningTrueBug();
     void nonTransitionBug();
     void registrationBug();
@@ -1323,6 +1325,56 @@ void tst_qquickanimations::startStopSignals()
     QVERIFY(timer.elapsed() >= 200);
 }
 
+void tst_qquickanimations::signalOrder_data()
+{
+    QTest::addColumn<QByteArray>("animationType");
+    QTest::addColumn<int>("duration");
+
+    QTest::addRow("ColorAnimation, duration = 10") << QByteArray("ColorAnimation") << 10;
+    QTest::addRow("ColorAnimation, duration = 0") << QByteArray("ColorAnimation") << 0;
+    QTest::addRow("ParallelAnimation, duration = 0") << QByteArray("ParallelAnimation") << 0;
+}
+
+void tst_qquickanimations::signalOrder()
+{
+    QFETCH(QByteArray, animationType);
+    QFETCH(int, duration);
+
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("signalorder.qml"));
+    QScopedPointer<QObject> obj(c.create());
+    auto *root = qobject_cast<QQuickItem *>(obj.data());
+    QVERIFY(root);
+    QQuickAbstractAnimation *animation = root->findChild<QQuickAbstractAnimation*>(animationType);
+
+    const QVector<void (QQuickAbstractAnimation::*)()> signalsToConnect = {
+        &QQuickAbstractAnimation::started,
+        &QQuickAbstractAnimation::stopped,
+        &QQuickAbstractAnimation::finished
+    };
+    const QVector<const char*> expectedSignalOrder = {
+        "started",
+        "stopped",
+        "finished"
+    };
+
+    QVector<const char*> actualSignalOrder;
+
+    for (int i = 0; i < signalsToConnect.size(); ++i) {
+        const char *str = expectedSignalOrder.at(i);
+        connect(animation, signalsToConnect.at(i) , [str, &actualSignalOrder] () {
+            actualSignalOrder.append(str);
+        });
+    }
+    QSignalSpy finishedSpy(animation, SIGNAL(finished()));
+    if (QQuickColorAnimation *colorAnimation = qobject_cast<QQuickColorAnimation*>(animation))
+        colorAnimation->setDuration(duration);
+
+    animation->start();
+    QTRY_VERIFY(finishedSpy.count());
+    QCOMPARE(actualSignalOrder, expectedSignalOrder);
+}
+
 void tst_qquickanimations::runningTrueBug()
 {
     //ensure we start correctly when "running: true" is explicitly set