Fix potential crash and leak in Binding.

With this patch, Binding will always correctly restore the original
binding when it becomes inactive (previously it would sometimes restore
a deleted binding). In the future we may want to consider changing to
retore the most recently set binding instead (it's currently unclear
whether either is a better model).

Change-Id: Iddde49de0815b2334ec3369963048554d1c8e105
Reviewed-by: Yunqiao Yin <charles.yin@nokia.com>

diff --git a/src/quick/util/qquickbind.cpp b/src/quick/util/qquickbind.cpp
index a3fba06..e298632 100644 (file)
--- a/src/quick/util/qquickbind.cpp
+++ b/src/quick/util/qquickbind.cpp
@@ -298,7 +298,7 @@ void QQuickBind::eval()
         QQmlAbstractBinding *tmp;
         tmp = QQmlPropertyPrivate::setBinding(d->prop, 0);
         if (tmp && d->prevBind)
-            d->prevBind->destroy();
+            tmp->destroy();
         else if (!d->prevBind)
             d->prevBind = tmp;
     }

diff --git a/tests/auto/qml/qquickbinding/data/restoreBindingWithoutCrash.qml b/tests/auto/qml/qquickbinding/data/restoreBindingWithoutCrash.qml
new file mode 100644 (file)
index 0000000..0c63a16
--- /dev/null
+++ b/tests/auto/qml/qquickbinding/data/restoreBindingWithoutCrash.qml
@@ -0,0 +1,28 @@
+import QtQuick 2.0
+
+Rectangle {
+    width: 400
+    height: 400
+
+    states: State {
+        name: "state1"
+        PropertyChanges {
+            target: myItem
+            x: 200 - myItem.y
+        }
+    }
+
+    Rectangle {
+        id: myItem
+        objectName:  "myItem"
+        width: 100
+        height: 100
+        color: "green"
+        x: 100 - myItem.y
+
+        Binding on x {
+            when: myItem.y > 50
+            value: myItem.y
+        }
+    }
+}

diff --git a/tests/auto/qml/qquickbinding/tst_qquickbinding.cpp b/tests/auto/qml/qquickbinding/tst_qquickbinding.cpp
index 939c4a9..c544199 100644 (file)
--- a/tests/auto/qml/qquickbinding/tst_qquickbinding.cpp
+++ b/tests/auto/qml/qquickbinding/tst_qquickbinding.cpp
@@ -56,6 +56,7 @@ private slots:
     void whenAfterValue();
     void restoreBinding();
     void restoreBindingWithLoop();
+    void restoreBindingWithoutCrash();
     void deletedObject();
 
 private:
@@ -176,6 +177,45 @@ void tst_qquickbinding::restoreBindingWithLoop()
     delete rect;
 }
 
+void tst_qquickbinding::restoreBindingWithoutCrash()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("restoreBindingWithoutCrash.qml"));
+    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QVERIFY(rect != 0);
+
+    QQuickRectangle *myItem = qobject_cast<QQuickRectangle*>(rect->findChild<QQuickRectangle*>("myItem"));
+    QVERIFY(myItem != 0);
+
+    myItem->setY(25);
+    QCOMPARE(myItem->x(), qreal(100-25));
+
+    myItem->setY(13);
+    QCOMPARE(myItem->x(), qreal(100-13));
+
+    //Binding takes effect
+    myItem->setY(51);
+    QCOMPARE(myItem->x(), qreal(51));
+
+    myItem->setY(88);
+    QCOMPARE(myItem->x(), qreal(88));
+
+    //state sets a new binding
+    rect->setState("state1");
+    //this binding temporarily takes effect. We may want to change this behavior in the future
+    QCOMPARE(myItem->x(), qreal(112));
+
+    //Binding still controls this value
+    myItem->setY(104);
+    QCOMPARE(myItem->x(), qreal(104));
+
+    //original binding restored
+    myItem->setY(49);
+    QCOMPARE(myItem->x(), qreal(100-49));
+
+    delete rect;
+}
+
 //QTBUG-20692
 void tst_qquickbinding::deletedObject()
 {