Required properties: Break binding to model on write

This mirrors the behavior in other parts of QML where writing to a
property in imperative code breaks the binding.

Change-Id: Id19eef17a3c5e77bc4c2772bd749b38c732606a8
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

src/qmlmodels/qqmldelegatemodel.cpp

@@ -894,6 +894,7 @@ void PropertyUpdater::doUpdate()
     auto sender = QObject::sender();
     auto mo = sender->metaObject();
     auto signalIndex = QObject::senderSignalIndex();
+    ++updateCount;
     // start at 0 instead of propertyOffset to handle properties from parent hierarchy
     for (auto i = 0; i < mo->propertyCount() + mo->propertyOffset(); ++i) {
         auto property = mo->property(i);
@@ -907,6 +908,27 @@ void PropertyUpdater::doUpdate()
     }
 }
 
+void PropertyUpdater::breakBinding()
+{
+    auto it = senderToConnection.find(QObject::senderSignalIndex());
+    if (it == senderToConnection.end())
+        return;
+    if (updateCount == 0) {
+        QObject::disconnect(*it);
+        QQmlError warning;
+        warning.setUrl(qmlContext(QObject::sender())->baseUrl());
+        auto signalName = QString::fromLatin1(QObject::sender()->metaObject()->method(QObject::senderSignalIndex()).name());
+        signalName.chop(sizeof("changed")-1);
+        QString propName = signalName;
+        propName[0] = propName[0].toLower();
+        warning.setDescription(QString::fromUtf8("Writing to \"%1\" broke the binding to the underlying model").arg(propName));
+        qmlWarning(this, warning);
+        senderToConnection.erase(it);
+    } else {
+        --updateCount;
+    }
+}
+
 void QQDMIncubationTask::initializeRequiredProperties(QQmlDelegateModelItem *modelItemToIncubate, QObject *object)
 {
     auto incubatorPriv = QQmlIncubatorPrivate::get(this);
@@ -947,10 +969,18 @@ void QQDMIncubationTask::initializeRequiredProperties(QQmlDelegateModelItem *mod
                 // only write to property if it was actually requested by the component
                 if (wasInRequired && prop.hasNotifySignal()) {
                     QMetaMethod changeSignal = prop.notifySignal();
-                    QMetaMethod updateSlot = PropertyUpdater::staticMetaObject.method(PropertyUpdater::staticMetaObject.indexOfSlot("doUpdate()"));
-                    QObject::connect(modelItemToIncubate, changeSignal, updater, updateSlot);
+                    static QMetaMethod updateSlot = PropertyUpdater::staticMetaObject.method(PropertyUpdater::staticMetaObject.indexOfSlot("doUpdate()"));
+                    QMetaObject::Connection conn = QObject::connect(modelItemToIncubate, changeSignal, updater, updateSlot);
+                    auto propIdx = object->metaObject()->indexOfProperty(propName.toUtf8());
+                    QMetaMethod writeToPropSignal = object->metaObject()->property(propIdx).notifySignal();
+                    updater->senderToConnection[writeToPropSignal.methodIndex()] = conn;
+                    static QMetaMethod breakBinding = PropertyUpdater::staticMetaObject.method(PropertyUpdater::staticMetaObject.indexOfSlot("breakBinding()"));
+                    componentProp.write(prop.read(modelItemToIncubate));
+                    // the connection needs to established after the write,
+                    // else the signal gets triggered by it and breakBinding will remove the connection
+                    QObject::connect(object, writeToPropSignal, updater, breakBinding);
                 }
-                if (wasInRequired)
+                else if (wasInRequired) // we still have to write, even if there is no change signal
                     componentProp.write(prop.read(modelItemToIncubate));
             }
         }

src/qmlmodels/qqmldelegatemodel_p_p.h

@@ -454,8 +454,11 @@ class PropertyUpdater : public QObject
 
 public:
     PropertyUpdater(QObject *parent);
+    QHash<int, QMetaObject::Connection> senderToConnection;
+    int updateCount = 0;
 public Q_SLOTS:
     void doUpdate();
+    void breakBinding();
 };
 
 QT_END_NAMESPACE

tests/auto/quick/qquickpathview/data/delegateWithRequiredProperties.3.qml

@@ -0,0 +1,64 @@
+import QtQuick 2.14
+
+Item {
+    id: root
+    width: 800
+    height: 600
+    property bool working: false
+
+    ListModel {
+        id: myModel
+        ListElement {
+            name: "Bill Jones"
+            place: "Berlin"
+        }
+        ListElement {
+            name: "Jane Doe"
+            place: "Oslo"
+        }
+        ListElement {
+            name: "John Smith"
+            place: "Oulo"
+        }
+    }
+
+    Component {
+        id: delegateComponent
+        Rectangle {
+            id: myDelegate
+            height: 50
+            width: 50
+        required property string name
+        required property int index
+        onNameChanged: () => {if (myDelegate.name === "You-know-who") root.working = false}
+            Text {
+                text: myDelegate.name
+                font.pointSize: 10
+                anchors.fill: myDelegate
+            }
+    Component.onCompleted: () => {myDelegate.name = "break binding"}
+        }
+    }
+
+    PathView {
+        anchors.fill: parent
+        model: myModel
+        delegate: delegateComponent
+        path: Path {
+            startX: 80; startY: 100
+            PathQuad { x: 120; y: 25; controlX: 260; controlY: 75 }
+            PathQuad { x: 140; y: 100; controlX: -20; controlY: 75 }
+        }
+    }
+    Timer {
+        interval: 1
+        running: true
+        repeat: false
+        onTriggered: () => {
+            myModel.setProperty(1, "name", "You-know-who")
+            myModel.setProperty(2, "name", "You-know-who")
+            root.working = true
+        }
+    }
+
+}

tests/auto/quick/qquickpathview/tst_qquickpathview.cpp

@@ -2676,6 +2676,13 @@ void tst_QQuickPathView::requiredPropertiesInDelegate()
         window->show();
         QTRY_VERIFY(window->rootObject()->property("working").toBool());
     }
+    {
+        QScopedPointer<QQuickView> window(createView());
+        QTest::ignoreMessage(QtMsgType::QtWarningMsg, QRegularExpression("Writing to \"name\" broke the binding to the underlying model"));
+        window->setSource(testFileUrl("delegateWithRequiredProperties.3.qml"));
+        window->show();
+        QTRY_VERIFY(window->rootObject()->property("working").toBool());
+    }
 }
 
 void tst_QQuickPathView::requiredPropertiesInDelegatePreventUnrelated()