Fix binding re-evaluation when list model properties change

This is a regression from commit
4876ea6a18ccdfd72014582aa5d50ab9f6b6ec9e, which avoided returning an
expensive QObject when calling get() but also lost the ability to
perform binding captures when accessing the properties. This change
restores the captures by performing them by hand in get() and also
triggering the notifiers directly when the values change, without
creating the QObject.

Task-number: QTBUG-52356
Change-Id: Ia429ffafd4032b63d3e592aa63bb0864a24e0965
Reviewed-by: Michael Brasser <michael.brasser@live.com>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

6 files changed, 91 insertions, 3 deletions

diff --git a/src/qml/qml/qqmlnotifier.cpp b/src/qml/qml/qqmlnotifier.cpp
index ea3f7a1530..e3f23b3e5d 100644
--- a/src/qml/qml/qqmlnotifier.cpp
+++ b/src/qml/qml/qqmlnotifier.cpp
@@ -65,6 +65,12 @@ namespace {
     };
 }
 
+void QQmlNotifier::notify(QQmlData *ddata, int notifierIndex)
+{
+    if (QQmlNotifierEndpoint *ep = ddata->notify(notifierIndex))
+        emitNotify(ep, Q_NULLPTR);
+}
+
 void QQmlNotifier::emitNotify(QQmlNotifierEndpoint *endpoint, void **a)
 {
     QVarLengthArray<NotifyListTraversalData> stack;
diff --git a/src/qml/qml/qqmlnotifier_p.h b/src/qml/qml/qqmlnotifier_p.h
index ac0aab892f..60ba041b4a 100644
--- a/src/qml/qml/qqmlnotifier_p.h
+++ b/src/qml/qml/qqmlnotifier_p.h
@@ -59,6 +59,8 @@ public:
     inline ~QQmlNotifier();
     inline void notify();
 
+    static void notify(QQmlData *ddata, int notifierIndex);
+
 private:
     friend class QQmlData;
     friend class QQmlNotifierEndpoint;
diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp
index 3d71621290..299097d0c0 100644
--- a/src/qml/types/qqmllistmodel.cpp
+++ b/src/qml/types/qqmllistmodel.cpp
@@ -40,10 +40,12 @@
 
 #include <private/qqmlcustomparser_p.h>
 #include <private/qqmlengine_p.h>
+#include <private/qqmlnotifier_p.h>
 
 #include <private/qv4object_p.h>
 #include <private/qv4dateobject_p.h>
 #include <private/qv4objectiterator_p.h>
+#include <private/qv4alloca_p.h>
 
 #include <qqmlcontext.h>
 #include <qqmlinfo.h>
@@ -52,6 +54,7 @@
 #include <QtCore/qstack.h>
 #include <QXmlStreamReader>
 #include <QtCore/qdatetime.h>
+#include <QScopedValueRollback>
 
 QT_BEGIN_NAMESPACE
 
@@ -1259,9 +1262,14 @@ ModelNodeMetaObject *ModelNodeMetaObject::get(QObject *obj)
 
 void ModelNodeMetaObject::updateValues()
 {
-    if (!m_initialized)
+    const int roleCount = m_model->m_listModel->roleCount();
+    if (!m_initialized) {
+        int *changedRoles = reinterpret_cast<int *>(alloca(roleCount * sizeof(int)));
+        for (int i = 0; i < roleCount; ++i)
+            changedRoles[i] = i;
+        emitDirectNotifies(changedRoles, roleCount);
         return;
-    int roleCount = m_model->m_listModel->roleCount();
+    }
     for (int i=0 ; i < roleCount ; ++i) {
         const ListLayout::Role &role = m_model->m_listModel->getExistingRole(i);
         QByteArray name = role.name.toUtf8();
@@ -1272,8 +1280,10 @@ void ModelNodeMetaObject::updateValues()
 
 void ModelNodeMetaObject::updateValues(const QVector<int> &roles)
 {
-    if (!m_initialized)
+    if (!m_initialized) {
+        emitDirectNotifies(roles.constData(), roles.count());
         return;
+    }
     int roleCount = roles.count();
     for (int i=0 ; i < roleCount ; ++i) {
         int roleIndex = roles.at(i);
@@ -1303,6 +1313,22 @@ void ModelNodeMetaObject::propertyWritten(int index)
     }
 }
 
+// Does the emission of the notifiers when we haven't created the meta-object yet
+void ModelNodeMetaObject::emitDirectNotifies(const int *changedRoles, int roleCount)
+{
+    Q_ASSERT(!m_initialized);
+    QQmlData *ddata = QQmlData::get(object(), /*create*/false);
+    if (!ddata)
+        return;
+    QQmlEnginePrivate *ep = QQmlEnginePrivate::get(qmlEngine(m_model));
+    if (!ep)
+        return;
+    for (int i = 0; i < roleCount; ++i) {
+        const int changedRole = changedRoles[i];
+        QQmlNotifier::notify(ddata, changedRole);
+    }
+}
+
 namespace QV4 {
 
 void ModelObject::put(Managed *m, String *name, const Value &value)
@@ -1332,6 +1358,18 @@ ReturnedValue ModelObject::get(const Managed *m, String *name, bool *hasProperty
         return QObjectWrapper::get(m, name, hasProperty);
     if (hasProperty)
         *hasProperty = true;
+
+    if (QQmlEngine *qmlEngine = that->engine()->qmlEngine()) {
+        QQmlEnginePrivate *ep = QQmlEnginePrivate::get(qmlEngine);
+        if (ep && ep->propertyCapture) {
+            QObjectPrivate *op = QObjectPrivate::get(that->object());
+            // Temporarily hide the dynamic meta-object, to prevent it from being created when the capture
+            // triggers a QObject::connectNotify() by calling obj->metaObject().
+            QScopedValueRollback<QDynamicMetaObjectData*> metaObjectBlocker(op->metaObject, 0);
+            ep->propertyCapture->captureProperty(that->object(), -1, role->index);
+        }
+    }
+
     const int elementIndex = that->d()->m_elementIndex;
     QVariant value = that->d()->m_model->data(elementIndex, role->index);
     return that->engine()->fromVariant(value);
diff --git a/src/qml/types/qqmllistmodel_p_p.h b/src/qml/types/qqmllistmodel_p_p.h
index d7e0defaec..f23fec1e59 100644
--- a/src/qml/types/qqmllistmodel_p_p.h
+++ b/src/qml/types/qqmllistmodel_p_p.h
@@ -146,6 +146,8 @@ private:
         setValue(name, val);
     }
 
+    void emitDirectNotifies(const int *changedRoles, int roleCount);
+
     void initialize();
     bool m_initialized;
 };
diff --git a/tests/auto/qml/qqmllistmodel/data/bindingsOnGetResult.qml b/tests/auto/qml/qqmllistmodel/data/bindingsOnGetResult.qml
new file mode 100644
index 0000000000..6bf750dcda
--- /dev/null
+++ b/tests/auto/qml/qqmllistmodel/data/bindingsOnGetResult.qml
@@ -0,0 +1,27 @@
+import QtQuick 2.0
+
+QtObject {
+    property ListModel model: ListModel {
+        ListElement { modified: false }
+        ListElement { modified: false }
+        ListElement { modified: false }
+        ListElement { modified: false }
+        ListElement { modified: false }
+    }
+
+    property bool isModified: {
+        for (var i = 0; i < model.count; ++i) {
+            if (model.get(i).modified)
+                return true;
+        }
+        return false;
+    }
+
+    property bool success: false
+    Component.onCompleted: {
+        // trigger read and setup of property captures
+        success = isModified
+        model.setProperty(0, "modified", true)
+        success = isModified
+    }
+}
diff --git a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
index 6b1deceaf2..8a90be601a 100644
--- a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
+++ b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
@@ -127,6 +127,7 @@ private slots:
     void datetime_data();
     void about_to_be_signals();
     void modify_through_delegate();
+    void bindingsOnGetResult();
 };
 
 bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
@@ -1474,6 +1475,18 @@ void tst_qqmllistmodel::modify_through_delegate()
     QCOMPARE(model->data(model->index(1, 0, QModelIndex()), roleNames.key("age")).toInt(), 18);
 }
 
+void tst_qqmllistmodel::bindingsOnGetResult()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine, testFileUrl("bindingsOnGetResult.qml"));
+    QVERIFY2(!component.isError(), qPrintable(component.errorString()));
+
+    QScopedPointer<QObject> obj(component.create());
+    QVERIFY(!obj.isNull());
+
+    QVERIFY(obj->property("success").toBool());
+}
+
 QTEST_MAIN(tst_qqmllistmodel)
 
 #include "tst_qqmllistmodel.moc"