From b39781b214e2502e0884bce88aa3ac324f2d0b12 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Thu, 7 May 2009 13:03:59 +0200 Subject: Make it impossible to have root state as source or target of transition or as error state Since the root state has no ancestors, it cannot be source or target in transitions since there will be no LCA for the transition, which is required for the algorithm of enterStates and exitStates. In SCXML the root state cannot be target or source of a transition. By the same logic, it cannot be an error state. The root state will always have a valid machine, since it's added to a machine immediately, which makes this code possible. --- src/corelib/statemachine/qabstracttransition.cpp | 15 ++++++++++++--- src/corelib/statemachine/qstate.cpp | 13 ++++++++++++- src/corelib/statemachine/qstatemachine.cpp | 2 ++ tests/auto/qstatemachine/tst_qstatemachine.cpp | 7 +++++-- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/corelib/statemachine/qabstracttransition.cpp b/src/corelib/statemachine/qabstracttransition.cpp index 1897aa6..c6a261d 100644 --- a/src/corelib/statemachine/qabstracttransition.cpp +++ b/src/corelib/statemachine/qabstracttransition.cpp @@ -180,7 +180,7 @@ QAbstractTransition::QAbstractTransition(const QList &targets, d_ptr->q_ptr = this; #endif Q_D(QAbstractTransition); - d->targetStates = targets; + setTargetStates(targets); } /*! @@ -221,7 +221,7 @@ QAbstractTransition::QAbstractTransition(QAbstractTransitionPrivate &dd, d_ptr->q_ptr = this; #endif Q_D(QAbstractTransition); - d->targetStates = targets; + setTargetStates(targets); } /*! @@ -265,7 +265,7 @@ void QAbstractTransition::setTargetState(QAbstractState* target) if (!target) d->targetStates.clear(); else - d->targetStates = QList() << target; + setTargetStates(QList() << target); } /*! @@ -284,6 +284,15 @@ QList QAbstractTransition::targetStates() const void QAbstractTransition::setTargetStates(const QList &targets) { Q_D(QAbstractTransition); + + for (int i=0; imachine() != 0 && target->machine()->rootState() == target) { + qWarning("QAbstractTransition::setTargetStates: root state cannot be target of transition"); + return; + } + } + d->targetStates = targets; } diff --git a/src/corelib/statemachine/qstate.cpp b/src/corelib/statemachine/qstate.cpp index 4c9e033..acee27d 100644 --- a/src/corelib/statemachine/qstate.cpp +++ b/src/corelib/statemachine/qstate.cpp @@ -280,11 +280,15 @@ QAbstractState *QState::errorState() const void QState::setErrorState(QAbstractState *state) { Q_D(QState); - if (state != 0 && QAbstractStatePrivate::get(state)->machine() != d->machine()) { + if (state != 0 && state->machine() != machine()) { qWarning("QState::setErrorState: error state cannot belong " "to a different state machine"); return; } + if (state->machine() != 0 && state->machine()->rootState() == state) { + qWarning("QStateMachine::setErrorState: root state cannot be error state"); + return; + } d->errorState = state; } @@ -301,6 +305,13 @@ QAbstractTransition *QState::addTransition(QAbstractTransition *transition) qWarning("QState::addTransition: cannot add null transition"); return 0; } + + // machine() will always be non-null for root state + if (machine() != 0 && machine()->rootState() == this) { + qWarning("QState::addTransition: cannot add transition from root state"); + return 0; + } + const QList &targets = QAbstractTransitionPrivate::get(transition)->targetStates; for (int i = 0; i < targets.size(); ++i) { QAbstractState *t = targets.at(i); diff --git a/src/corelib/statemachine/qstatemachine.cpp b/src/corelib/statemachine/qstatemachine.cpp index 21e564c..110a4f8 100644 --- a/src/corelib/statemachine/qstatemachine.cpp +++ b/src/corelib/statemachine/qstatemachine.cpp @@ -1011,6 +1011,8 @@ void QStateMachinePrivate::setError(QStateMachine::Error errorCode, QAbstractSta } Q_ASSERT(currentErrorState != 0); + Q_ASSERT(currentErrorState != rootState); + QState *lca = findLCA(QList() << currentErrorState << currentContext); addStatesToEnter(currentErrorState, lca, pendingErrorStates, pendingErrorStatesForDefaultEntry); } diff --git a/tests/auto/qstatemachine/tst_qstatemachine.cpp b/tests/auto/qstatemachine/tst_qstatemachine.cpp index edd6459..9b7bff3 100644 --- a/tests/auto/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/qstatemachine/tst_qstatemachine.cpp @@ -254,6 +254,8 @@ private: void tst_QStateMachine::transitionToRootState() { + s_countWarnings = false; + QStateMachine machine; QState *initialState = new QState(); @@ -668,6 +670,8 @@ void tst_QStateMachine::errorStateHasErrors() void tst_QStateMachine::errorStateIsRootState() { + s_countWarnings = false; + QStateMachine machine; machine.setErrorState(machine.rootState()); @@ -691,9 +695,8 @@ void tst_QStateMachine::errorStateIsRootState() machine.postEvent(new QEvent(QEvent::Type(QEvent::User + 1))); QCoreApplication::processEvents(); - QEXPECT_FAIL("", "Covered by transitionToRootState test. Remove this when that test passes", Continue); QCOMPARE(machine.configuration().count(), 1); - QVERIFY(machine.configuration().contains(initialState)); + QVERIFY(machine.configuration().contains(machine.errorState())); } void tst_QStateMachine::errorStateEntersParentFirst() -- cgit v0.12 From 00f04527d3d28ab1aa5b90b9a05366012ff5d1e5 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Thu, 7 May 2009 17:18:51 +0200 Subject: don't add signal transition if target is null or signal doesn't exist --- src/corelib/statemachine/qstate.cpp | 9 +++++++++ tests/auto/qstatemachine/tst_qstatemachine.cpp | 23 +++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/corelib/statemachine/qstate.cpp b/src/corelib/statemachine/qstate.cpp index acee27d..4d12219 100644 --- a/src/corelib/statemachine/qstate.cpp +++ b/src/corelib/statemachine/qstate.cpp @@ -346,6 +346,15 @@ QSignalTransition *QState::addTransition(QObject *sender, const char *signal, qWarning("QState::addTransition: signal cannot be null"); return 0; } + if (!target) { + qWarning("QState::addTransition: cannot add transition to null state"); + return 0; + } + if (*signal && sender->metaObject()->indexOfSignal(signal+1) == -1) { + qWarning("QState::addTransition: no such signal %s::%s", + sender->metaObject()->className(), signal+1); + return 0; + } QSignalTransition *trans = new QSignalTransition(sender, signal, QList() << target); addTransition(trans); return trans; diff --git a/tests/auto/qstatemachine/tst_qstatemachine.cpp b/tests/auto/qstatemachine/tst_qstatemachine.cpp index 9b7bff3..f7fce94 100644 --- a/tests/auto/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/qstatemachine/tst_qstatemachine.cpp @@ -1579,9 +1579,28 @@ void tst_QStateMachine::signalTransitions() { QStateMachine machine; QState *s0 = new QState(machine.rootState()); - QFinalState *s1 = new QFinalState(machine.rootState()); + QTest::ignoreMessage(QtWarningMsg, "QState::addTransition: sender cannot be null"); + QCOMPARE(s0->addTransition(0, SIGNAL(noSuchSignal()), 0), (QObject*)0); + SignalEmitter emitter; - s0->addTransition(&emitter, SIGNAL(signalWithNoArg()), s1); + QTest::ignoreMessage(QtWarningMsg, "QState::addTransition: signal cannot be null"); + QCOMPARE(s0->addTransition(&emitter, 0, 0), (QObject*)0); + + QTest::ignoreMessage(QtWarningMsg, "QState::addTransition: cannot add transition to null state"); + QCOMPARE(s0->addTransition(&emitter, SIGNAL(signalWithNoArg()), 0), (QObject*)0); + + QFinalState *s1 = new QFinalState(machine.rootState()); + QTest::ignoreMessage(QtWarningMsg, "QState::addTransition: no such signal SignalEmitter::noSuchSignal()"); + QCOMPARE(s0->addTransition(&emitter, SIGNAL(noSuchSignal()), s1), (QObject*)0); + + { + QSignalTransition *trans = s0->addTransition(&emitter, SIGNAL(signalWithNoArg()), s1); + QVERIFY(trans != 0); + QCOMPARE(trans->sourceState(), s0); + QCOMPARE(trans->targetState(), (QAbstractState*)s1); + QCOMPARE(trans->senderObject(), (QObject*)&emitter); + QCOMPARE(trans->signal(), QByteArray(SIGNAL(signalWithNoArg()))); + } QSignalSpy finishedSpy(&machine, SIGNAL(finished())); machine.setInitialState(s0); -- cgit v0.12 From 58bf96d7252df3346d0f325af265873897e56172 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 8 May 2009 09:55:28 +0200 Subject: don't create transition to null state --- src/corelib/statemachine/qstate.cpp | 7 +++++-- tests/auto/qstatemachine/tst_qstatemachine.cpp | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/corelib/statemachine/qstate.cpp b/src/corelib/statemachine/qstate.cpp index 4d12219..63a0a69 100644 --- a/src/corelib/statemachine/qstate.cpp +++ b/src/corelib/statemachine/qstate.cpp @@ -381,9 +381,12 @@ protected: */ QAbstractTransition *QState::addTransition(QAbstractState *target) { + if (!target) { + qWarning("QState::addTransition: cannot add transition to null state"); + return 0; + } UnconditionalTransition *trans = new UnconditionalTransition(target); - addTransition(trans); - return trans; + return addTransition(trans); } /*! diff --git a/tests/auto/qstatemachine/tst_qstatemachine.cpp b/tests/auto/qstatemachine/tst_qstatemachine.cpp index f7fce94..93cd1b3 100644 --- a/tests/auto/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/qstatemachine/tst_qstatemachine.cpp @@ -1091,11 +1091,28 @@ void tst_QStateMachine::stateEntryAndExit() QStateMachine machine; TestState *s1 = new TestState(machine.rootState()); + QTest::ignoreMessage(QtWarningMsg, "QState::addTransition: cannot add transition to null state"); + s1->addTransition((QAbstractState*)0); + QTest::ignoreMessage(QtWarningMsg, "QState::addTransition: cannot add null transition"); + s1->addTransition((QAbstractTransition*)0); + QTest::ignoreMessage(QtWarningMsg, "QState::removeTransition: cannot remove null transition"); + s1->removeTransition((QAbstractTransition*)0); + TestState *s2 = new TestState(machine.rootState()); QFinalState *s3 = new QFinalState(machine.rootState()); TestTransition *t = new TestTransition(s2); - s1->addTransition(t); - s2->addTransition(s3); + QCOMPARE(s1->addTransition(t), (QAbstractTransition*)t); + { + QAbstractTransition *trans = s2->addTransition(s3); + QVERIFY(trans != 0); + QCOMPARE(trans->sourceState(), (QAbstractState*)s2); + QCOMPARE(trans->targetState(), (QAbstractState*)s3); + s2->removeTransition(trans); + QCOMPARE(trans->sourceState(), (QAbstractState*)0); + QCOMPARE(trans->targetState(), (QAbstractState*)s3); + QCOMPARE(s2->addTransition(trans), trans); + QCOMPARE(trans->sourceState(), (QAbstractState*)s2); + } QSignalSpy startedSpy(&machine, SIGNAL(started())); QSignalSpy stoppedSpy(&machine, SIGNAL(stopped())); @@ -1311,6 +1328,8 @@ void tst_QStateMachine::assignPropertyWithAnimation() QStateMachine machine; QObject obj; QState *s1 = new QState(machine.rootState()); + QCOMPARE(s1->childMode(), QState::ExclusiveStates); + QCOMPARE(s1->initialState(), (QAbstractState*)0); s1->setObjectName("s1"); s1->assignProperty(&obj, "foo", 123); s1->assignProperty(&obj, "bar", 456); @@ -1324,6 +1343,7 @@ void tst_QStateMachine::assignPropertyWithAnimation() s22->setObjectName("s22"); s22->assignProperty(&obj, "bar", 789); s2->setInitialState(s21); + QCOMPARE(s2->initialState(), (QAbstractState*)s21); QAbstractTransition *trans = s1->addTransition(s2); QPropertyAnimation anim(&obj, "foo"); @@ -1448,6 +1468,7 @@ void tst_QStateMachine::parallelStates() QStateMachine machine; QState *s1 = new QState(QState::ParallelStates); + QCOMPARE(s1->childMode(), QState::ParallelStates); QState *s1_1 = new QState(s1); QState *s1_1_1 = new QState(s1_1); QFinalState *s1_1_f = new QFinalState(s1_1); -- cgit v0.12 From f64170262441cce51b52d05a2f2ba2f0021ba8e4 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 8 May 2009 12:59:03 +0200 Subject: make sure connections and event filters are removed when state machine halts --- src/corelib/statemachine/qstatemachine.cpp | 16 ++++++++++++++++ src/corelib/statemachine/qstatemachine_p.h | 1 + tests/auto/qstatemachine/tst_qstatemachine.cpp | 4 ++++ 3 files changed, 21 insertions(+) diff --git a/src/corelib/statemachine/qstatemachine.cpp b/src/corelib/statemachine/qstatemachine.cpp index 110a4f8..24af8e4 100644 --- a/src/corelib/statemachine/qstatemachine.cpp +++ b/src/corelib/statemachine/qstatemachine.cpp @@ -1224,10 +1224,12 @@ void QStateMachinePrivate::_q_process() break; case Finished: state = NotRunning; + unregisterAllTransitions(); emit q->finished(); break; case Stopped: state = NotRunning; + unregisterAllTransitions(); emit q->stopped(); break; } @@ -1358,6 +1360,20 @@ void QStateMachinePrivate::unregisterSignalTransition(QSignalTransition *transit #endif } +void QStateMachinePrivate::unregisterAllTransitions() +{ + { + QList transitions = qFindChildren(rootState); + for (int i = 0; i < transitions.size(); ++i) + unregisterSignalTransition(transitions.at(i)); + } + { + QList transitions = qFindChildren(rootState); + for (int i = 0; i < transitions.size(); ++i) + unregisterEventTransition(transitions.at(i)); + } +} + #ifndef QT_NO_STATEMACHINE_EVENTFILTER void QStateMachinePrivate::registerEventTransition(QEventTransition *transition) { diff --git a/src/corelib/statemachine/qstatemachine_p.h b/src/corelib/statemachine/qstatemachine_p.h index bb4a78c..47b139c 100644 --- a/src/corelib/statemachine/qstatemachine_p.h +++ b/src/corelib/statemachine/qstatemachine_p.h @@ -150,6 +150,7 @@ public: void unregisterEventTransition(QEventTransition *transition); #endif void unregisterTransition(QAbstractTransition *transition); + void unregisterAllTransitions(); void handleTransitionSignal(const QObject *sender, int signalIndex, void **args); void scheduleProcess(); diff --git a/tests/auto/qstatemachine/tst_qstatemachine.cpp b/tests/auto/qstatemachine/tst_qstatemachine.cpp index 93cd1b3..288cbec 100644 --- a/tests/auto/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/qstatemachine/tst_qstatemachine.cpp @@ -1631,6 +1631,8 @@ void tst_QStateMachine::signalTransitions() emitter.emitSignalWithNoArg(); QTRY_COMPARE(finishedSpy.count(), 1); + + emitter.emitSignalWithNoArg(); } { QStateMachine machine; @@ -1722,6 +1724,8 @@ void tst_QStateMachine::eventTransitions() QCoreApplication::processEvents(); QTRY_COMPARE(finishedSpy.count(), 1); + + QTest::mousePress(&button, Qt::LeftButton); } { QStateMachine machine; -- cgit v0.12 From a70e9bc07f2252a49060132fc88ddd4c5963d456 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 8 May 2009 13:34:22 +0200 Subject: Test what happens when target state doesn't have a parent --- tests/auto/qstatemachine/tst_qstatemachine.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/auto/qstatemachine/tst_qstatemachine.cpp b/tests/auto/qstatemachine/tst_qstatemachine.cpp index 288cbec..fa49ecb 100644 --- a/tests/auto/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/qstatemachine/tst_qstatemachine.cpp @@ -105,6 +105,7 @@ private slots: void eventTransitions(); void historyStates(); void startAndStop(); + void targetStateWithNoParent(); void transitionToRootState(); void transitionEntersParent(); @@ -1490,6 +1491,8 @@ void tst_QStateMachine::parallelStates() QSignalSpy finishedSpy(&machine, SIGNAL(finished())); machine.start(); QTRY_COMPARE(finishedSpy.count(), 1); + QCOMPARE(machine.configuration().size(), 1); + QVERIFY(machine.configuration().contains(s2)); } void tst_QStateMachine::allSourceToTargetConfigurations() @@ -1915,6 +1918,28 @@ void tst_QStateMachine::startAndStop() QVERIFY(machine.configuration().contains(s1)); } +void tst_QStateMachine::targetStateWithNoParent() +{ + QStateMachine machine; + QState *s1 = new QState(machine.rootState()); + s1->setObjectName("s1"); + QState *s2 = new QState(); + s1->addTransition(s2); + machine.setInitialState(s1); + QSignalSpy startedSpy(&machine, SIGNAL(started())); + QSignalSpy stoppedSpy(&machine, SIGNAL(stopped())); + QSignalSpy finishedSpy(&machine, SIGNAL(finished())); + machine.start(); + QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: No common ancestor for targets and source of transition from state 's1'"); + QTRY_COMPARE(machine.isRunning(), true); + QTRY_COMPARE(startedSpy.count(), 1); + QCOMPARE(stoppedSpy.count(), 0); + QCOMPARE(finishedSpy.count(), 0); + QCOMPARE(machine.configuration().size(), 1); + QVERIFY(machine.configuration().contains(machine.errorState())); + QCOMPARE(machine.error(), QStateMachine::NoCommonAncestorForTransitionError); +} + void tst_QStateMachine::defaultGlobalRestorePolicy() { QStateMachine machine; -- cgit v0.12 From 110473e9eece3231c3df4fc50c3a958c6c25f2de Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 8 May 2009 14:29:28 +0200 Subject: get rid of warnings --- src/corelib/statemachine/qabstracttransition.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/corelib/statemachine/qabstracttransition.cpp b/src/corelib/statemachine/qabstracttransition.cpp index c6a261d..696da9f 100644 --- a/src/corelib/statemachine/qabstracttransition.cpp +++ b/src/corelib/statemachine/qabstracttransition.cpp @@ -179,7 +179,6 @@ QAbstractTransition::QAbstractTransition(const QList &targets, #ifdef QT_STATEMACHINE_SOLUTION d_ptr->q_ptr = this; #endif - Q_D(QAbstractTransition); setTargetStates(targets); } @@ -220,7 +219,6 @@ QAbstractTransition::QAbstractTransition(QAbstractTransitionPrivate &dd, #ifdef QT_STATEMACHINE_SOLUTION d_ptr->q_ptr = this; #endif - Q_D(QAbstractTransition); setTargetStates(targets); } -- cgit v0.12 From 98f3ebcf1f6751cb76f9268d33faf0bc5ac70f6e Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 8 May 2009 16:37:21 +0200 Subject: gracefully handle deletion of transition's target state --- src/corelib/statemachine/qabstracttransition.cpp | 16 ++++++++++++++-- src/corelib/statemachine/qabstracttransition_p.h | 3 ++- src/corelib/statemachine/qstate.cpp | 2 +- tests/auto/qstatemachine/tst_qstatemachine.cpp | 13 +++++++++++++ 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/corelib/statemachine/qabstracttransition.cpp b/src/corelib/statemachine/qabstracttransition.cpp index 696da9f..6ddb416 100644 --- a/src/corelib/statemachine/qabstracttransition.cpp +++ b/src/corelib/statemachine/qabstracttransition.cpp @@ -273,7 +273,13 @@ void QAbstractTransition::setTargetState(QAbstractState* target) QList QAbstractTransition::targetStates() const { Q_D(const QAbstractTransition); - return d->targetStates; + QList result; + for (int i = 0; i < d->targetStates.size(); ++i) { + QAbstractState *target = d->targetStates.at(i); + if (target) + result.append(target); + } + return result; } /*! @@ -285,13 +291,19 @@ void QAbstractTransition::setTargetStates(const QList &targets) for (int i=0; imachine() != 0 && target->machine()->rootState() == target) { qWarning("QAbstractTransition::setTargetStates: root state cannot be target of transition"); return; } } - d->targetStates = targets; + d->targetStates.clear(); + for (int i = 0; i < targets.size(); ++i) + d->targetStates.append(targets.at(i)); } /*! diff --git a/src/corelib/statemachine/qabstracttransition_p.h b/src/corelib/statemachine/qabstracttransition_p.h index b4e1c88..eb0ec21 100644 --- a/src/corelib/statemachine/qabstracttransition_p.h +++ b/src/corelib/statemachine/qabstracttransition_p.h @@ -58,6 +58,7 @@ #endif #include +#include QT_BEGIN_NAMESPACE @@ -83,7 +84,7 @@ public: QState *sourceState() const; QStateMachine *machine() const; - QList targetStates; + QList > targetStates; #ifndef QT_NO_ANIMATION QList animations; diff --git a/src/corelib/statemachine/qstate.cpp b/src/corelib/statemachine/qstate.cpp index 63a0a69..5f61865 100644 --- a/src/corelib/statemachine/qstate.cpp +++ b/src/corelib/statemachine/qstate.cpp @@ -312,7 +312,7 @@ QAbstractTransition *QState::addTransition(QAbstractTransition *transition) return 0; } - const QList &targets = QAbstractTransitionPrivate::get(transition)->targetStates; + const QList > &targets = QAbstractTransitionPrivate::get(transition)->targetStates; for (int i = 0; i < targets.size(); ++i) { QAbstractState *t = targets.at(i); if (!t) { diff --git a/tests/auto/qstatemachine/tst_qstatemachine.cpp b/tests/auto/qstatemachine/tst_qstatemachine.cpp index fa49ecb..dbc67d1 100644 --- a/tests/auto/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/qstatemachine/tst_qstatemachine.cpp @@ -106,6 +106,7 @@ private slots: void historyStates(); void startAndStop(); void targetStateWithNoParent(); + void targetStateDeleted(); void transitionToRootState(); void transitionEntersParent(); @@ -1940,6 +1941,18 @@ void tst_QStateMachine::targetStateWithNoParent() QCOMPARE(machine.error(), QStateMachine::NoCommonAncestorForTransitionError); } +void tst_QStateMachine::targetStateDeleted() +{ + QStateMachine machine; + QState *s1 = new QState(machine.rootState()); + s1->setObjectName("s1"); + QState *s2 = new QState(machine.rootState()); + QAbstractTransition *trans = s1->addTransition(s2); + delete s2; + QCOMPARE(trans->targetState(), (QAbstractState*)0); + QVERIFY(trans->targetStates().isEmpty()); +} + void tst_QStateMachine::defaultGlobalRestorePolicy() { QStateMachine machine; -- cgit v0.12