From: Michael Brasser Date: Wed, 12 Mar 2014 15:44:28 +0000 (-0500) Subject: Don't crash if a ScriptAction changes state mid-transition. X-Git-Tag: upstream/5.3.1~280 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=11e9c49e5420bf749e4da48b1f14fa7dc9e4716f;p=platform%2Fupstream%2Fqtdeclarative.git Don't crash if a ScriptAction changes state mid-transition. Change-Id: Ia85cb128c7410e2276bf4da02f946d3d0bf44989 Reviewed-by: Gunnar Sletta --- diff --git a/src/qml/animations/qsequentialanimationgroupjob.cpp b/src/qml/animations/qsequentialanimationgroupjob.cpp index 41a83ce..ec9b2ba 100644 --- a/src/qml/animations/qsequentialanimationgroupjob.cpp +++ b/src/qml/animations/qsequentialanimationgroupjob.cpp @@ -142,20 +142,21 @@ void QSequentialAnimationGroupJob::advanceForwards(const AnimationIndex &newAnim if (m_previousLoop < m_currentLoop) { // we need to fast forward to the end for (QAbstractAnimationJob *anim = m_currentAnimation; anim; anim = anim->nextSibling()) { - setCurrentAnimation(anim, true); + RETURN_IF_DELETED(setCurrentAnimation(anim, true)); RETURN_IF_DELETED(anim->setCurrentTime(animationActualTotalDuration(anim))); } // this will make sure the current animation is reset to the beginning - if (firstChild() && !firstChild()->nextSibling()) //count == 1 + if (firstChild() && !firstChild()->nextSibling()) { //count == 1 // we need to force activation because setCurrentAnimation will have no effect - activateCurrentAnimation(); - else - setCurrentAnimation(firstChild(), true); + RETURN_IF_DELETED(activateCurrentAnimation()); + } else { + RETURN_IF_DELETED(setCurrentAnimation(firstChild(), true)); + } } // and now we need to fast forward from the current position to for (QAbstractAnimationJob *anim = m_currentAnimation; anim && anim != newAnimationIndex.animation; anim = anim->nextSibling()) { //### WRONG, - setCurrentAnimation(anim, true); + RETURN_IF_DELETED(setCurrentAnimation(anim, true)); RETURN_IF_DELETED(anim->setCurrentTime(animationActualTotalDuration(anim))); } // setting the new current animation will happen later @@ -166,21 +167,21 @@ void QSequentialAnimationGroupJob::rewindForwards(const AnimationIndex &newAnima if (m_previousLoop > m_currentLoop) { // we need to fast rewind to the beginning for (QAbstractAnimationJob *anim = m_currentAnimation; anim; anim = anim->previousSibling()) { - setCurrentAnimation(anim, true); + RETURN_IF_DELETED(setCurrentAnimation(anim, true)); RETURN_IF_DELETED(anim->setCurrentTime(0)); } // this will make sure the current animation is reset to the end - if (lastChild() && !lastChild()->previousSibling()) //count == 1 + if (lastChild() && !lastChild()->previousSibling()) { //count == 1 // we need to force activation because setCurrentAnimation will have no effect - activateCurrentAnimation(); - else { - setCurrentAnimation(lastChild(), true); + RETURN_IF_DELETED(activateCurrentAnimation()); + } else { + RETURN_IF_DELETED(setCurrentAnimation(lastChild(), true)); } } // and now we need to fast rewind from the current position to for (QAbstractAnimationJob *anim = m_currentAnimation; anim && anim != newAnimationIndex.animation; anim = anim->previousSibling()) { - setCurrentAnimation(anim, true); + RETURN_IF_DELETED(setCurrentAnimation(anim, true)); RETURN_IF_DELETED(anim->setCurrentTime(0)); } // setting the new current animation will happen later @@ -220,7 +221,7 @@ void QSequentialAnimationGroupJob::updateCurrentTime(int currentTime) RETURN_IF_DELETED(rewindForwards(newAnimationIndex)); } - setCurrentAnimation(newAnimationIndex.animation); + RETURN_IF_DELETED(setCurrentAnimation(newAnimationIndex.animation)); const int newCurrentTime = currentTime - newAnimationIndex.timeOffset; @@ -310,7 +311,7 @@ void QSequentialAnimationGroupJob::activateCurrentAnimation(bool intermediate) if (m_currentAnimation->totalDuration() == -1) resetUncontrolledAnimationFinishTime(m_currentAnimation); - m_currentAnimation->start(); + RETURN_IF_DELETED(m_currentAnimation->start()); if (!intermediate && isPaused()) m_currentAnimation->pause(); } diff --git a/tests/auto/quick/qquickanimations/data/scriptActionCrash.qml b/tests/auto/quick/qquickanimations/data/scriptActionCrash.qml new file mode 100644 index 0000000..0a428e2 --- /dev/null +++ b/tests/auto/quick/qquickanimations/data/scriptActionCrash.qml @@ -0,0 +1,72 @@ +/**************************************************************************** +** +** Copyright (C) 2014 Jolla Ltd. +** Contact: http://www.qt-project.org/legal +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and Digia. For licensing terms and +** conditions see http://qt.digia.com/licensing. For further information +** use the contact form at http://qt.digia.com/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Digia gives you certain additional +** rights. These rights are described in the Digia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3.0 as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU General Public License version 3.0 requirements will be +** met: http://www.gnu.org/copyleft/gpl.html. +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +import QtQuick 2.0 + +Item { + width: 400; height: 400 + + Component.onCompleted: rect.state = "wide" + + Rectangle { + id: rect + + color: "green" + width: 50 + height: 50 + anchors.centerIn: parent + + states: State { + name: "wide" + PropertyChanges { + target: rect + width: 100 + } + } + transitions: Transition { + to: "wide" + SequentialAnimation { + NumberAnimation { duration: 200; property: "width" } + ScriptAction { script: { rect.state = ""; rect.state = "wide" } } + } + } + } +} diff --git a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp index 21872a5..c167aa6 100644 --- a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp +++ b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp @@ -111,6 +111,7 @@ private slots: void pathAnimationInOutBackBug(); void scriptActionBug(); void groupAnimationNullChildBug(); + void scriptActionCrash(); }; #define QTIMED_COMPARE(lhs, rhs) do { \ @@ -1478,6 +1479,20 @@ void tst_qquickanimations::groupAnimationNullChildBug() } } +//ScriptAction should not crash if changing a state in a transition +void tst_qquickanimations::scriptActionCrash() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("scriptActionCrash.qml")); + QObject *obj = c.create(); + + //just testing that we don't crash + QTest::qWait(1000); //5x transition duration + + delete obj; +} + + QTEST_MAIN(tst_qquickanimations)