From: Michael Brasser Date: Mon, 13 Feb 2012 00:11:26 +0000 (+1000) Subject: Don't crash when an animation update causes it to delete itself. X-Git-Tag: upstream/5.2.1~2646 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=815b4b463da83802f535b328d7ef699aea529a6b;p=platform%2Fupstream%2Fqtdeclarative.git Don't crash when an animation update causes it to delete itself. Change-Id: Ic108adfb99a09e6ede71d474240fb0917cee6961 Reviewed-by: Bea Lam --- diff --git a/src/declarative/animations/animations.pri b/src/declarative/animations/animations.pri index 240ee96..01ac25a 100644 --- a/src/declarative/animations/animations.pri +++ b/src/declarative/animations/animations.pri @@ -5,7 +5,8 @@ HEADERS += \ $$PWD/qanimationgroupjob_p.h \ $$PWD/qsequentialanimationgroupjob_p.h \ $$PWD/qparallelanimationgroupjob_p.h \ - $$PWD/qpauseanimationjob_p.h + $$PWD/qpauseanimationjob_p.h \ + $$PWD/qanimationjobutil_p.h SOURCES += \ $$PWD/qabstractanimationjob.cpp \ diff --git a/src/declarative/animations/qabstractanimationjob.cpp b/src/declarative/animations/qabstractanimationjob.cpp index d9fe795..cd5730d 100644 --- a/src/declarative/animations/qabstractanimationjob.cpp +++ b/src/declarative/animations/qabstractanimationjob.cpp @@ -43,6 +43,7 @@ #include "private/qabstractanimationjob_p.h" #include "private/qanimationgroupjob_p.h" +#include "private/qanimationjobutil_p.h" #define DEFAULT_TIMER_INTERVAL 16 @@ -321,12 +322,7 @@ void QAbstractAnimationJob::setState(QAbstractAnimationJob::State newState) if (newState == Running && oldState == Stopped && !m_group) topLevelAnimationLoopChanged(); - bool wasDeleted = false; - m_wasDeleted = &wasDeleted; - updateState(newState, oldState); - if (wasDeleted) - return; - m_wasDeleted = 0; + RETURN_IF_DELETED(updateState(newState, oldState)); if (newState != m_state) //this is to be safe if updateState changes the state return; @@ -438,7 +434,7 @@ void QAbstractAnimationJob::setCurrentTime(int msecs) if (m_currentLoop != oldLoop && !m_group) //### verify Running as well? topLevelAnimationLoopChanged(); - updateCurrentTime(m_currentTime); + RETURN_IF_DELETED(updateCurrentTime(m_currentTime)); if (m_currentLoop != oldLoop) currentLoopChanged(m_currentLoop); diff --git a/src/declarative/animations/qanimationjobutil_p.h b/src/declarative/animations/qanimationjobutil_p.h new file mode 100644 index 0000000..b8a627e --- /dev/null +++ b/src/declarative/animations/qanimationjobutil_p.h @@ -0,0 +1,59 @@ +/**************************************************************************** +** +** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies). +** Contact: http://www.qt-project.org/ +** +** This file is part of the QtDeclarative module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** GNU Lesser General Public License Usage +** 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, Nokia gives you certain additional +** rights. These rights are described in the Nokia 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. +** +** Other Usage +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** +** +** +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef QANIMATIONJOBUTIL_P_H +#define QANIMATIONJOBUTIL_P_H + +#define RETURN_IF_DELETED(func) \ +{ \ + bool *prevWasDeleted = m_wasDeleted; \ + bool wasDeleted = false; \ + m_wasDeleted = &wasDeleted; \ + func; \ + if (wasDeleted) { \ + if (prevWasDeleted) \ + *prevWasDeleted = true; \ + return; \ + } \ + m_wasDeleted = prevWasDeleted; \ +} + +#endif // QANIMATIONJOBUTIL_P_H diff --git a/src/declarative/animations/qparallelanimationgroupjob.cpp b/src/declarative/animations/qparallelanimationgroupjob.cpp index ff38be3..9411ad8 100644 --- a/src/declarative/animations/qparallelanimationgroupjob.cpp +++ b/src/declarative/animations/qparallelanimationgroupjob.cpp @@ -40,6 +40,7 @@ ****************************************************************************/ #include "private/qparallelanimationgroupjob_p.h" +#include "private/qanimationjobutil_p.h" QT_BEGIN_NAMESPACE @@ -84,7 +85,7 @@ void QParallelAnimationGroupJob::updateCurrentTime(int /*currentTime*/) if (dura > 0) { for (QAbstractAnimationJob *animation = firstChild(); animation; animation = animation->nextSibling()) { if (!animation->isStopped()) - animation->setCurrentTime(dura); // will stop + RETURN_IF_DELETED(animation->setCurrentTime(dura)); // will stop } } } else if (m_currentLoop < m_previousLoop) { @@ -93,7 +94,7 @@ void QParallelAnimationGroupJob::updateCurrentTime(int /*currentTime*/) //we need to make sure the animation is in the right state //and then rewind it applyGroupState(animation); - animation->setCurrentTime(0); + RETURN_IF_DELETED(animation->setCurrentTime(0)); animation->stop(); } } @@ -110,7 +111,7 @@ void QParallelAnimationGroupJob::updateCurrentTime(int /*currentTime*/) } if (animation->state() == state()) { - animation->setCurrentTime(m_currentTime); + RETURN_IF_DELETED(animation->setCurrentTime(m_currentTime)); if (dura > 0 && m_currentTime > dura) animation->stop(); } diff --git a/src/declarative/animations/qsequentialanimationgroupjob.cpp b/src/declarative/animations/qsequentialanimationgroupjob.cpp index 1d19873..f999ca3 100644 --- a/src/declarative/animations/qsequentialanimationgroupjob.cpp +++ b/src/declarative/animations/qsequentialanimationgroupjob.cpp @@ -41,6 +41,7 @@ #include "private/qsequentialanimationgroupjob_p.h" #include "private/qpauseanimationjob_p.h" +#include "private/qanimationjobutil_p.h" QT_BEGIN_NAMESPACE @@ -140,7 +141,7 @@ void QSequentialAnimationGroupJob::advanceForwards(const AnimationIndex &newAnim // we need to fast forward to the end for (QAbstractAnimationJob *anim = m_currentAnimation; anim; anim = anim->nextSibling()) { setCurrentAnimation(anim, true); - anim->setCurrentTime(animationActualTotalDuration(anim)); + 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 @@ -153,7 +154,7 @@ void QSequentialAnimationGroupJob::advanceForwards(const AnimationIndex &newAnim // 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); - anim->setCurrentTime(animationActualTotalDuration(anim)); + RETURN_IF_DELETED(anim->setCurrentTime(animationActualTotalDuration(anim))); } // setting the new current animation will happen later } @@ -164,7 +165,7 @@ void QSequentialAnimationGroupJob::rewindForwards(const AnimationIndex &newAnima // we need to fast rewind to the beginning for (QAbstractAnimationJob *anim = m_currentAnimation; anim; anim = anim->previousSibling()) { setCurrentAnimation(anim, true); - anim->setCurrentTime(0); + RETURN_IF_DELETED(anim->setCurrentTime(0)); } // this will make sure the current animation is reset to the end if (lastChild() && !lastChild()->previousSibling()) //count == 1 @@ -178,7 +179,7 @@ void QSequentialAnimationGroupJob::rewindForwards(const AnimationIndex &newAnima // 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); - anim->setCurrentTime(0); + RETURN_IF_DELETED(anim->setCurrentTime(0)); } // setting the new current animation will happen later } @@ -209,11 +210,11 @@ void QSequentialAnimationGroupJob::updateCurrentTime(int currentTime) if (m_previousLoop < m_currentLoop || (m_previousLoop == m_currentLoop && m_currentAnimation != newAnimationIndex.animation && newAnimationIndex.afterCurrent)) { // advancing with forward direction is the same as rewinding with backwards direction - advanceForwards(newAnimationIndex); + RETURN_IF_DELETED(advanceForwards(newAnimationIndex)); } else if (m_previousLoop > m_currentLoop || (m_previousLoop == m_currentLoop && m_currentAnimation != newAnimationIndex.animation && !newAnimationIndex.afterCurrent)) { // rewinding with forward direction is the same as advancing with backwards direction - rewindForwards(newAnimationIndex); + RETURN_IF_DELETED(rewindForwards(newAnimationIndex)); } setCurrentAnimation(newAnimationIndex.animation); @@ -221,7 +222,7 @@ void QSequentialAnimationGroupJob::updateCurrentTime(int currentTime) const int newCurrentTime = currentTime - newAnimationIndex.timeOffset; if (m_currentAnimation) { - m_currentAnimation->setCurrentTime(newCurrentTime); + RETURN_IF_DELETED(m_currentAnimation->setCurrentTime(newCurrentTime)); if (atEnd()) { //we make sure that we don't exceed the duration here m_currentTime += m_currentAnimation->currentTime() - newCurrentTime; diff --git a/tests/auto/qtquick2/qdeclarativesmoothedanimation/data/deleteOnUpdate.qml b/tests/auto/qtquick2/qdeclarativesmoothedanimation/data/deleteOnUpdate.qml new file mode 100644 index 0000000..ff8dfaa --- /dev/null +++ b/tests/auto/qtquick2/qdeclarativesmoothedanimation/data/deleteOnUpdate.qml @@ -0,0 +1,27 @@ +import QtQuick 2.0 + +Rectangle { + width: 300; height: 300; + + Rectangle { + color: "red" + width: 60; height: 60; + x: 100; y: 100; + + property real prevX: 100 + onXChanged: { + if (x - prevX > 10) { + anim.to += 5 + anim.restart(); //this can cause deletion of backend animation classes + prevX = x; + } + } + + SmoothedAnimation on x { + id: anim + objectName: "anim" + velocity: 100 + to: 150 + } + } +} diff --git a/tests/auto/qtquick2/qdeclarativesmoothedanimation/tst_qdeclarativesmoothedanimation.cpp b/tests/auto/qtquick2/qdeclarativesmoothedanimation/tst_qdeclarativesmoothedanimation.cpp index 3d37517..09bde4a 100644 --- a/tests/auto/qtquick2/qdeclarativesmoothedanimation/tst_qdeclarativesmoothedanimation.cpp +++ b/tests/auto/qtquick2/qdeclarativesmoothedanimation/tst_qdeclarativesmoothedanimation.cpp @@ -59,6 +59,7 @@ private slots: void simpleAnimation(); void valueSource(); void behavior(); + void deleteOnUpdate(); private: QDeclarativeEngine engine; @@ -218,6 +219,24 @@ void tst_qdeclarativesmoothedanimation::behavior() delete rect; } +void tst_qdeclarativesmoothedanimation::deleteOnUpdate() +{ + QDeclarativeEngine engine; + + QDeclarativeComponent c(&engine, testFileUrl("deleteOnUpdate.qml")); + + QQuickRectangle *rect = qobject_cast(c.create()); + QVERIFY(rect); + + QDeclarativeSmoothedAnimation *anim = rect->findChild("anim"); + QVERIFY(anim); + + //don't crash + QTest::qWait(500); + + delete rect; +} + QTEST_MAIN(tst_qdeclarativesmoothedanimation) #include "tst_qdeclarativesmoothedanimation.moc"