Removing a binding while it is being applied caused a crash.

Use a weak pointer to the binding in case applying
the binding (while fast forwarding) causes it to be removed.

Task-number: QTBUG-14830
Reviewed-by: Michael Brasser

6 files changed, 59 insertions, 16 deletions

diff --git a/src/declarative/util/qdeclarativestate.cpp b/src/declarative/util/qdeclarativestate.cpp
index 0f5413e..3915485 100644
--- a/src/declarative/util/qdeclarativestate.cpp
+++ b/src/declarative/util/qdeclarativestate.cpp
@@ -58,7 +58,7 @@ QT_BEGIN_NAMESPACE
 DEFINE_BOOL_CONFIG_OPTION(stateChangeDebug, STATECHANGE_DEBUG);
 
 QDeclarativeAction::QDeclarativeAction()
-: restore(true), actionDone(false), reverseEvent(false), deletableToBinding(false), fromBinding(0), toBinding(0), event(0),
+: restore(true), actionDone(false), reverseEvent(false), deletableToBinding(false), fromBinding(0), event(0),
   specifiedObject(0)
 {
 }
@@ -67,7 +67,7 @@ QDeclarativeAction::QDeclarativeAction(QObject *target, const QString &propertyN
                const QVariant &value)
 : restore(true), actionDone(false), reverseEvent(false), deletableToBinding(false), 
   property(target, propertyName), toValue(value), 
-  fromBinding(0), toBinding(0), event(0), 
+  fromBinding(0), event(0),
   specifiedObject(target), specifiedProperty(propertyName)
 {
     if (property.isValid())
@@ -78,7 +78,7 @@ QDeclarativeAction::QDeclarativeAction(QObject *target, const QString &propertyN
                QDeclarativeContext *context, const QVariant &value)
 : restore(true), actionDone(false), reverseEvent(false), deletableToBinding(false),
   property(target, propertyName, context), toValue(value),
-  fromBinding(0), toBinding(0), event(0),
+  fromBinding(0), event(0),
   specifiedObject(target), specifiedProperty(propertyName)
 {
     if (property.isValid())
@@ -503,11 +503,11 @@ void QDeclarativeState::addEntriesToRevertList(const QList<QDeclarativeAction> &
             const QDeclarativeAction &action = actionListIterator.next();
             QDeclarativeSimpleAction simpleAction(action);
             action.property.write(action.toValue);
-            if (action.toBinding) {
+            if (!action.toBinding.isNull()) {
                 QDeclarativeAbstractBinding *oldBinding = QDeclarativePropertyPrivate::binding(simpleAction.property());
                 if (oldBinding)
                     QDeclarativePropertyPrivate::setBinding(simpleAction.property(), 0);
-                QDeclarativePropertyPrivate::setBinding(simpleAction.property(), action.toBinding, QDeclarativePropertyPrivate::DontRemoveBinding);
+                QDeclarativePropertyPrivate::setBinding(simpleAction.property(), action.toBinding.data(), QDeclarativePropertyPrivate::DontRemoveBinding);
             }
 
             simpleActionList.append(simpleAction);
@@ -675,7 +675,7 @@ void QDeclarativeState::apply(QDeclarativeStateGroup *group, QDeclarativeTransit
             a.property = d->revertList.at(ii).property();
             a.fromValue = cur;
             a.toValue = d->revertList.at(ii).value();
-            a.toBinding = d->revertList.at(ii).binding();
+            a.toBinding = QDeclarativeAbstractBinding::getPointer(d->revertList.at(ii).binding());
             a.specifiedObject = d->revertList.at(ii).specifiedObject();
             a.specifiedProperty = d->revertList.at(ii).specifiedProperty();
             a.event = d->revertList.at(ii).event();
diff --git a/src/declarative/util/qdeclarativestate_p.h b/src/declarative/util/qdeclarativestate_p.h
index fc7c940..7b9c18a 100644
--- a/src/declarative/util/qdeclarativestate_p.h
+++ b/src/declarative/util/qdeclarativestate_p.h
@@ -45,6 +45,7 @@
 #include <qdeclarative.h>
 #include <qdeclarativeproperty.h>
 #include <QtCore/qobject.h>
+#include <private/qdeclarativebinding_p.h>
 #include <private/qdeclarativeglobal_p.h>
 
 QT_BEGIN_HEADER
@@ -75,7 +76,7 @@ public:
     QVariant toValue;
 
     QDeclarativeAbstractBinding *fromBinding;
-    QDeclarativeAbstractBinding *toBinding;
+    QDeclarativeAbstractBinding::Pointer toBinding;
     QDeclarativeActionEvent *event;
 
     //strictly for matching
diff --git a/src/declarative/util/qdeclarativestate_p_p.h b/src/declarative/util/qdeclarativestate_p_p.h
index 4fd8f21..98c3f7b 100644
--- a/src/declarative/util/qdeclarativestate_p_p.h
+++ b/src/declarative/util/qdeclarativestate_p_p.h
@@ -85,7 +85,7 @@ public:
             m_reverseEvent = true;
         } else {
             m_value = a.toValue;
-            m_binding = QDeclarativeAbstractBinding::getPointer(a.toBinding);
+            m_binding = a.toBinding;
             m_reverseEvent = false;
         }
     }
diff --git a/src/declarative/util/qdeclarativetransitionmanager.cpp b/src/declarative/util/qdeclarativetransitionmanager.cpp
index 89b0044..d19e6f2 100644
--- a/src/declarative/util/qdeclarativetransitionmanager.cpp
+++ b/src/declarative/util/qdeclarativetransitionmanager.cpp
@@ -99,8 +99,8 @@ void QDeclarativeTransitionManager::complete()
 void QDeclarativeTransitionManagerPrivate::applyBindings()
 {
     foreach(const QDeclarativeAction &action, bindingsList) {
-        if (action.toBinding) {
-            QDeclarativePropertyPrivate::setBinding(action.property, action.toBinding);
+        if (!action.toBinding.isNull()) {
+            QDeclarativePropertyPrivate::setBinding(action.property, action.toBinding.data());
         } else if (action.event) {
             if (action.reverseEvent)
                 action.event->reverse();
@@ -145,8 +145,8 @@ void QDeclarativeTransitionManager::transition(const QList<QDeclarativeAction> &
         // Apply all the property and binding changes
         for (int ii = 0; ii < applyList.size(); ++ii) {
             const QDeclarativeAction &action = applyList.at(ii);
-            if (action.toBinding) {
-                QDeclarativePropertyPrivate::setBinding(action.property, action.toBinding, QDeclarativePropertyPrivate::BypassInterceptor | QDeclarativePropertyPrivate::DontRemoveBinding);
+            if (!action.toBinding.isNull()) {
+                QDeclarativePropertyPrivate::setBinding(action.property, action.toBinding.data(), QDeclarativePropertyPrivate::BypassInterceptor | QDeclarativePropertyPrivate::DontRemoveBinding);
             } else if (!action.event) {
                 QDeclarativePropertyPrivate::write(action.property, action.toValue, QDeclarativePropertyPrivate::BypassInterceptor | QDeclarativePropertyPrivate::DontRemoveBinding);
             } else if (action.event->isReversable()) {
@@ -165,7 +165,7 @@ void QDeclarativeTransitionManager::transition(const QList<QDeclarativeAction> &
                 continue;
             }
             const QDeclarativeProperty &prop = action->property;
-            if (action->toBinding || !action->toValue.isValid()) {
+            if (!action->toBinding.isNull() || !action->toValue.isValid()) {
                 action->toValue = prop.read();
             }
         }
@@ -259,10 +259,10 @@ void QDeclarativeTransitionManager::cancel()
 
     for(int i = 0; i < d->bindingsList.count(); ++i) {
         QDeclarativeAction action = d->bindingsList[i];
-        if (action.toBinding && action.deletableToBinding) {
+        if (!action.toBinding.isNull() && action.deletableToBinding) {
             QDeclarativePropertyPrivate::setBinding(action.property, 0);
-            action.toBinding->destroy();
-            action.toBinding = 0;
+            action.toBinding.data()->destroy();
+            action.toBinding.clear();
             action.deletableToBinding = false;
         } else if (action.event) {
             //### what do we do here?
diff --git a/tests/auto/declarative/qdeclarativestates/data/QTBUG-14830.qml b/tests/auto/declarative/qdeclarativestates/data/QTBUG-14830.qml
new file mode 100644
index 0000000..5eccdda
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativestates/data/QTBUG-14830.qml
@@ -0,0 +1,29 @@
+import Qt 4.7
+
+Rectangle {
+    width: 1024
+    height: 768
+
+    Item {
+        id: area
+        objectName: "area"
+        property int numx: 6
+        property int cellwidth: 1024/numx
+
+        onWidthChanged: {
+            width = width>1024?1024:width;
+        }
+
+        state: 'minimal'
+        states: [
+            State {
+                name: 'minimal'
+                PropertyChanges {
+                    target: area
+                    width: cellwidth
+                }
+            }
+        ]
+
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativestates/tst_qdeclarativestates.cpp b/tests/auto/declarative/qdeclarativestates/tst_qdeclarativestates.cpp
index 0d10c10..b8409a5 100644
--- a/tests/auto/declarative/qdeclarativestates/tst_qdeclarativestates.cpp
+++ b/tests/auto/declarative/qdeclarativestates/tst_qdeclarativestates.cpp
@@ -143,6 +143,7 @@ private slots:
     void returnToBase();
     void extendsBug();
     void editProperties();
+    void QTBUG_14830();
 };
 
 void tst_qdeclarativestates::initTestCase()
@@ -1375,6 +1376,18 @@ void tst_qdeclarativestates::editProperties()
     QCOMPARE(childRect->height(), qreal(40));
 }
 
+void tst_qdeclarativestates::QTBUG_14830()
+{
+    QDeclarativeEngine engine;
+
+    QDeclarativeComponent c(&engine, SRCDIR "/data/QTBUG-14830.qml");
+    QDeclarativeRectangle *rect = qobject_cast<QDeclarativeRectangle*>(c.create());
+    QVERIFY(rect != 0);
+    QDeclarativeItem *item = rect->findChild<QDeclarativeItem*>("area");
+
+    QCOMPARE(item->width(), qreal(171));
+}
+
 QTEST_MAIN(tst_qdeclarativestates)
 
 #include "tst_qdeclarativestates.moc"