From d219ea1ebcd8dee171dab279498961d68dac8812 Mon Sep 17 00:00:00 2001
From: Kent Hansen <kent.hansen@nokia.com>
Date: Wed, 4 Jul 2012 23:17:49 +0200
Subject: [PATCH] statemachine: Don't assign properties for transitions with no
 targets

If the transition has no target states, that means the current state
won't change; hence, property assignments should not be performed.
In particular, properties should not be restored to the values they
had before the state was entered.

Change-Id: I237bbb541f939c272777e70c5f26c886ec457a17
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@nokia.com>
---
 src/corelib/statemachine/qstatemachine.cpp    | 10 ++--
 .../qstatemachine/tst_qstatemachine.cpp       | 55 +++++++++++++++++++
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/src/corelib/statemachine/qstatemachine.cpp b/src/corelib/statemachine/qstatemachine.cpp
index 276e3f603f..b64a0f5eb3 100644
--- a/src/corelib/statemachine/qstatemachine.cpp
+++ b/src/corelib/statemachine/qstatemachine.cpp
@@ -378,7 +378,8 @@ void QStateMachinePrivate::microstep(QEvent *event, const QList<QAbstractTransit
     executeTransitionContent(event, enabledTransitions);
     QList<QAbstractState*> enteredStates = enterStates(event, enabledTransitions);
 #ifndef QT_NO_PROPERTIES
-    applyProperties(enabledTransitions, exitedStates, enteredStates);
+    if (!enteredStates.isEmpty()) // Ignore transitions with no targets
+        applyProperties(enabledTransitions, exitedStates, enteredStates);
 #endif
 #ifdef QSTATEMACHINE_DEBUG
     qDebug() << q_func() << ": configuration after entering states:" << configuration;
@@ -662,6 +663,7 @@ void QStateMachinePrivate::applyProperties(const QList<QAbstractTransition*> &tr
 #else
     Q_Q(QStateMachine);
 #endif
+    Q_ASSERT(!enteredStates.isEmpty());
     // Process the property assignments of the entered states.
     QHash<QAbstractState*, QList<QPropertyAssignment> > propertyAssignmentsForState;
     QHash<RestorableId, QVariant> pendingRestorables = registeredRestorables;
@@ -705,11 +707,7 @@ void QStateMachinePrivate::applyProperties(const QList<QAbstractTransition*> &tr
         }
     }
     if (!pendingRestorables.isEmpty()) {
-        QAbstractState *s;
-        if (!enteredStates.isEmpty())
-            s = enteredStates.last(); // ### handle if parallel
-        else
-            s = 0;
+        QAbstractState *s = enteredStates.last(); // ### handle if parallel
         propertyAssignmentsForState[s] << restorablesToPropertyList(pendingRestorables);
     }
 
diff --git a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp
index 0a1d0ea35f..6598a74e31 100644
--- a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp
+++ b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp
@@ -191,6 +191,8 @@ private slots:
     void deletePropertyAssignmentObjectBeforeRestore();
     void deleteInitialState();
     void setPropertyAfterRestore();
+    void transitionWithNoTarget_data();
+    void transitionWithNoTarget();
 };
 
 class TestState : public QState
@@ -4132,5 +4134,58 @@ void tst_QStateMachine::setPropertyAfterRestore()
     delete object;
 }
 
+void tst_QStateMachine::transitionWithNoTarget_data()
+{
+    QTest::addColumn<int>("restorePolicy");
+    QTest::newRow("DontRestoreProperties") << int(QStateMachine::DontRestoreProperties);
+    QTest::newRow("RestoreProperties") << int(QStateMachine::RestoreProperties);
+}
+
+void tst_QStateMachine::transitionWithNoTarget()
+{
+    QFETCH(int, restorePolicy);
+
+    QStateMachine machine;
+    machine.setGlobalRestorePolicy(static_cast<QStateMachine::RestorePolicy>(restorePolicy));
+
+    QObject *object = new QObject;
+    object->setProperty("a", 1);
+
+    QState *s1 = new QState(&machine);
+    machine.setInitialState(s1);
+    s1->assignProperty(object, "a", 2);
+    EventTransition *t1 = new EventTransition(QEvent::User, /*target=*/0);
+    s1->addTransition(t1);
+
+    QSignalSpy s1EnteredSpy(s1, SIGNAL(entered()));
+    QSignalSpy s1ExitedSpy(s1, SIGNAL(exited()));
+    QSignalSpy t1TriggeredSpy(t1, SIGNAL(triggered()));
+
+    machine.start();
+    QTRY_VERIFY(machine.configuration().contains(s1));
+    QCOMPARE(s1EnteredSpy.count(), 1);
+    QCOMPARE(s1ExitedSpy.count(), 0);
+    QCOMPARE(t1TriggeredSpy.count(), 0);
+    QCOMPARE(object->property("a").toInt(), 2);
+
+    object->setProperty("a", 3);
+
+    machine.postEvent(new QEvent(QEvent::User));
+    QTRY_COMPARE(t1TriggeredSpy.count(), 1);
+    QCOMPARE(s1EnteredSpy.count(), 1);
+    QCOMPARE(s1ExitedSpy.count(), 0);
+    // the assignProperty should not be re-executed, nor should the old value
+    // be restored
+    QCOMPARE(object->property("a").toInt(), 3);
+
+    machine.postEvent(new QEvent(QEvent::User));
+    QTRY_COMPARE(t1TriggeredSpy.count(), 2);
+    QCOMPARE(s1EnteredSpy.count(), 1);
+    QCOMPARE(s1ExitedSpy.count(), 0);
+    QCOMPARE(object->property("a").toInt(), 3);
+
+    delete object;
+}
+
 QTEST_MAIN(tst_QStateMachine)
 #include "tst_qstatemachine.moc"