Detect binding loops.

Don't crash when a looping binding has been established. Instead, print
a warning and stop looping.

3 files changed, 135 insertions, 90 deletions

diff --git a/src/declarative/qml/qmlbindablevalue.cpp b/src/declarative/qml/qmlbindablevalue.cpp
index 9f4886a..d5157b6 100644
--- a/src/declarative/qml/qmlbindablevalue.cpp
+++ b/src/declarative/qml/qmlbindablevalue.cpp
@@ -43,16 +43,17 @@
 #include "qmlbindablevalue.h"
 #include "qmlbindablevalue_p.h"
 #include <qmlcontext.h>
+#include <qmlinfo.h>
 #include <QVariant>
 #include <qfxperf.h>
 #include <QtCore/qdebug.h>
 
-
 QT_BEGIN_NAMESPACE
+
 DEFINE_BOOL_CONFIG_OPTION(scriptWarnings, QML_SCRIPT_WARNINGS);
 
 QmlBindableValuePrivate::QmlBindableValuePrivate()
-: inited(false), mePtr(0)
+: inited(false), updating(false), mePtr(0)
 {
 }
 
@@ -121,102 +122,110 @@ void QmlBindableValue::update()
     if (!d->inited)
         return;
 
-    if (d->property.propertyCategory() == QmlMetaProperty::List) {
-        QVariant value = this->value();
-        int listType = QmlMetaType::listType(d->property.propertyType());
-
-        if (value.userType() == qMetaTypeId<QList<QObject *> >()) {
-            const QList<QObject *> &list = 
-                qvariant_cast<QList<QObject *> >(value);
-            QVariant listVar = d->property.read();
-            QmlMetaType::clear(listVar);
-            for (int ii = 0; ii < list.count(); ++ii) {
-                QVariant v = QmlMetaType::fromObject(list.at(ii), listType);
-                QmlMetaType::append(listVar, v);
-            }
+    if (!d->updating) {
+        d->updating = true;
+
+        if (d->property.propertyCategory() == QmlMetaProperty::List) {
+            QVariant value = this->value();
+            int listType = QmlMetaType::listType(d->property.propertyType());
+
+            if (value.userType() == qMetaTypeId<QList<QObject *> >()) {
+                const QList<QObject *> &list =
+                    qvariant_cast<QList<QObject *> >(value);
+                QVariant listVar = d->property.read();
+                QmlMetaType::clear(listVar);
+                for (int ii = 0; ii < list.count(); ++ii) {
+                    QVariant v = QmlMetaType::fromObject(list.at(ii), listType);
+                    QmlMetaType::append(listVar, v);
+                }
 
-        } else if (value.type() == uint(listType) ||
-                  value.userType() == listType) {
-            QVariant listVar = d->property.read();
-            QmlMetaType::clear(listVar);
-            QmlMetaType::append(listVar, value);
-        }
-    } else if (d->property.propertyCategory() == QmlMetaProperty::QmlList) {
-        // XXX - optimize!
-        QVariant value = this->value();
-        QVariant list = d->property.read();
-        QmlPrivate::ListInterface *li =
-            *(QmlPrivate::ListInterface **)list.constData();
-
-        int type = li->type();
-
-        if (QObject *obj = QmlMetaType::toQObject(value)) {
-            const QMetaObject *mo =
-                QmlMetaType::rawMetaObjectForType(type);
-
-            const QMetaObject *objMo = obj->metaObject();
-            bool found = false;
-            while(!found && objMo) {
-                if (objMo == mo)
-                    found = true;
-                else
-                    objMo = objMo->superClass();
+            } else if (value.type() == uint(listType) ||
+                      value.userType() == listType) {
+                QVariant listVar = d->property.read();
+                QmlMetaType::clear(listVar);
+                QmlMetaType::append(listVar, value);
             }
+        } else if (d->property.propertyCategory() == QmlMetaProperty::QmlList) {
+            // XXX - optimize!
+            QVariant value = this->value();
+            QVariant list = d->property.read();
+            QmlPrivate::ListInterface *li =
+                *(QmlPrivate::ListInterface **)list.constData();
+
+            int type = li->type();
+
+            if (QObject *obj = QmlMetaType::toQObject(value)) {
+                const QMetaObject *mo =
+                    QmlMetaType::rawMetaObjectForType(type);
+
+                const QMetaObject *objMo = obj->metaObject();
+                bool found = false;
+                while(!found && objMo) {
+                    if (objMo == mo)
+                        found = true;
+                    else
+                        objMo = objMo->superClass();
+                }
 
-            if (!found) {
-                qWarning() << "Unable to assign object to list";
-                return;
-            }
+                if (!found) {
+                    qWarning() << "Unable to assign object to list";
+                    return;
+                }
 
-            // NOTE: This assumes a cast to QObject does not alter
-            // the object pointer
-            void *d = (void *)&obj;
-            li->append(d);
-        }
-    } else if (d->property.propertyCategory() == QmlMetaProperty::Bindable) {
-
-        // NOTE: We assume that only core properties can have 
-        // propertyType == Bindable
-        int idx = d->property.coreIndex();
-        Q_ASSERT(idx != -1);
-
-        void *a[1];
-        QmlBindableValue *t = this;
-        a[0] = (void *)&t;
-        d->property.object()->qt_metacall(QMetaObject::WriteProperty,
-                                          idx, a);
-
-    } else if (d->property.propertyCategory() == QmlMetaProperty::Object) {
-
-        QVariant value = this->value();
-        if ((int)value.type() != qMetaTypeId<QObject *>()) {
-            if (scriptWarnings()) {
-                if (!value.isValid()) {
-                    qWarning() << "QmlBindableValue: Unable to assign invalid value to object property";
-                } else {
-                    qWarning() << "QmlBindableValue: Unable to assign non-object to object property";
+                // NOTE: This assumes a cast to QObject does not alter
+                // the object pointer
+                void *d = (void *)&obj;
+                li->append(d);
+            }
+        } else if (d->property.propertyCategory() == QmlMetaProperty::Bindable) {
+
+            // NOTE: We assume that only core properties can have
+            // propertyType == Bindable
+            int idx = d->property.coreIndex();
+            Q_ASSERT(idx != -1);
+
+            void *a[1];
+            QmlBindableValue *t = this;
+            a[0] = (void *)&t;
+            d->property.object()->qt_metacall(QMetaObject::WriteProperty,
+                                              idx, a);
+
+        } else if (d->property.propertyCategory() == QmlMetaProperty::Object) {
+
+            QVariant value = this->value();
+            if ((int)value.type() != qMetaTypeId<QObject *>()) {
+                if (scriptWarnings()) {
+                    if (!value.isValid()) {
+                        qWarning() << "QmlBindableValue: Unable to assign invalid value to object property";
+                    } else {
+                        qWarning() << "QmlBindableValue: Unable to assign non-object to object property";
+                    }
                 }
+                return;
             }
-            return;
-        }
 
-        // NOTE: This assumes a cast to QObject does not alter the 
-        // object pointer
-        QObject *obj = *(QObject **)value.data();
+            // NOTE: This assumes a cast to QObject does not alter the
+            // object pointer
+            QObject *obj = *(QObject **)value.data();
+
+            // NOTE: We assume that only core properties can have
+            // propertyType == Object
+            int idx = d->property.coreIndex();
+            Q_ASSERT(idx != -1);
 
-        // NOTE: We assume that only core properties can have 
-        // propertyType == Object
-        int idx = d->property.coreIndex();
-        Q_ASSERT(idx != -1);
+            void *a[1];
+            a[0] = (void *)&obj;
+            d->property.object()->qt_metacall(QMetaObject::WriteProperty,
+                                            idx, a);
 
-        void *a[1];
-        a[0] = (void *)&obj;
-        d->property.object()->qt_metacall(QMetaObject::WriteProperty,
-                                        idx, a);
+        } else if (d->property.propertyCategory() == QmlMetaProperty::Normal) {
+            QVariant value = this->value();
+            d->property.write(value);
+        }
 
-    } else if (d->property.propertyCategory() == QmlMetaProperty::Normal) {
-        QVariant value = this->value();
-        d->property.write(value);
+        d->updating = false;
+    } else {
+        qmlInfo(d->property.object()) << "Binding loop detected for property" << d->property.name();
     }
 }
 
diff --git a/src/declarative/qml/qmlbindablevalue_p.h b/src/declarative/qml/qmlbindablevalue_p.h
index 70c001b..d9af0ef 100644
--- a/src/declarative/qml/qmlbindablevalue_p.h
+++ b/src/declarative/qml/qmlbindablevalue_p.h
@@ -55,6 +55,7 @@ public:
     QmlBindableValuePrivate();
 
     bool inited;
+    bool updating;
     QmlMetaProperty property;
 
     QmlBindableValue **mePtr;
diff --git a/tests/auto/declarative/qmlbindengine/tst_qmlbindengine.cpp b/tests/auto/declarative/qmlbindengine/tst_qmlbindengine.cpp
index 0875690..ca840f4 100644
--- a/tests/auto/declarative/qmlbindengine/tst_qmlbindengine.cpp
+++ b/tests/auto/declarative/qmlbindengine/tst_qmlbindengine.cpp
@@ -7,7 +7,7 @@ class MyQmlObject : public QObject
     Q_OBJECT
     Q_PROPERTY(bool trueProperty READ trueProperty)
     Q_PROPERTY(bool falseProperty READ falseProperty)
-    Q_PROPERTY(QString stringProperty READ stringProperty WRITE setStringProperty)
+    Q_PROPERTY(QString stringProperty READ stringProperty WRITE setStringProperty NOTIFY stringChanged)
 public:
     MyQmlObject(): m_methodCalled(false), m_methodIntCalled(false) {}
 
@@ -15,7 +15,13 @@ public:
     bool falseProperty() const { return false; }
 
     QString stringProperty() const { return m_string; }
-    void setStringProperty(const QString &s) { m_string = s; }
+    void setStringProperty(const QString &s)
+    {
+        if (s == m_string)
+            return;
+        m_string = s;
+        emit stringChanged();
+    }
 
     bool methodCalled() const { return m_methodCalled; }
     bool methodIntCalled() const { return m_methodIntCalled; }
@@ -24,6 +30,7 @@ public:
 signals:
     void basicSignal();
     void argumentSignal(int a, QString b, qreal c);
+    void stringChanged();
 
 public slots:
     void method() { m_methodCalled = true; }
@@ -41,6 +48,22 @@ private:
 QML_DECLARE_TYPE(MyQmlObject);
 QML_DEFINE_TYPE(MyQmlObject,MyQmlObject);
 
+class MyQmlContainer : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(QList<MyQmlContainer*>* children READ children)
+public:
+    MyQmlContainer() {}
+
+    QList<MyQmlContainer*> *children() { return &m_children; }
+
+private:
+    QList<MyQmlContainer*> m_children;
+};
+
+QML_DECLARE_TYPE(MyQmlContainer);
+QML_DEFINE_TYPE(MyQmlContainer,MyQmlContainer);
+
 class tst_qmlbindengine : public QObject
 {
     Q_OBJECT
@@ -51,6 +74,7 @@ private slots:
     void boolPropertiesEvaluateAsBool();
     void methods();
     void signalAssignment();
+    void bindingLoop();
 
 private:
     QmlEngine engine;
@@ -117,6 +141,17 @@ void tst_qmlbindengine::methods()
         QCOMPARE(object->methodIntCalled(), true);
     }
 }
+#include <QDebug>
+void tst_qmlbindengine::bindingLoop()
+{
+    QmlComponent component(&engine, "MyQmlContainer { children : [ "\
+                                    "MyQmlObject { id: Object1; stringProperty: \"hello\" + Object2.stringProperty }, "\
+                                    "MyQmlObject { id: Object2; stringProperty: \"hello\" + Object1.stringProperty } ] }");
+    //### ignoreMessage doesn't seem to work here
+    //QTest::ignoreMessage(QtWarningMsg, "QML MyQmlObject (unknown location): Binding loop detected for property \"stringProperty\"");
+    QObject *object = component.create();
+    QVERIFY(object != 0);
+}
 
 QTEST_MAIN(tst_qmlbindengine)