QQmlPropertyBinding: Correctly link observers after undefined valued binding

Calling both setObservers and prependObservers doesn't make sense:
setObservers is for the case where we don't have a binding, but
obviously we do have one here.

Note that the test case still passes even without the fix - it will
however cause a memory leak which can be detected by ASAN. The leak is
fixed by this patch.

Fixes: QTBUG-115251
Pick-to: 6.6 6.5 6.2
Change-Id: I4b420e05f49acf764da6a05af522390005276f49
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

4 files changed, 27 insertions, 3 deletions

diff --git a/src/qml/qml/qqmlpropertybinding.cpp b/src/qml/qml/qqmlpropertybinding.cpp
index d1c44f205d..5f646b62de 100644
--- a/src/qml/qml/qqmlpropertybinding.cpp
+++ b/src/qml/qml/qqmlpropertybinding.cpp
@@ -251,10 +251,8 @@ void QQmlPropertyBinding::handleUndefinedAssignment(QQmlEnginePrivate *ep, void
         // reset might have changed observers (?), so refresh firstObserver
         firstObserver = bindingDataPointer.firstObserver();
         bindingData->d_ref() = reinterpret_cast<quintptr>(this) | QtPrivate::QPropertyBindingData::BindingBit;
-        if (firstObserver) {
-            bindingDataPointer.setObservers(firstObserver.ptr);
+        if (firstObserver)
             prependObserver(firstObserver);
-        }
     } else {
         QQmlError qmlError;
         auto location = jsExpression()->sourceLocation();
diff --git a/tests/auto/qml/qqmlecmascript/CMakeLists.txt b/tests/auto/qml/qqmlecmascript/CMakeLists.txt
index 6a203eb736..12cab47a36 100644
--- a/tests/auto/qml/qqmlecmascript/CMakeLists.txt
+++ b/tests/auto/qml/qqmlecmascript/CMakeLists.txt
@@ -30,6 +30,7 @@ qt_internal_add_test(tst_qqmlecmascript
         Qt::Network
         Qt::QmlPrivate
         Qt::QuickTestUtilsPrivate
+        Qt::QuickPrivate
     TESTDATA ${test_data}
 )
 
diff --git a/tests/auto/qml/qqmlecmascript/data/qpropertyResetCorrectlyLinked.qml b/tests/auto/qml/qqmlecmascript/data/qpropertyResetCorrectlyLinked.qml
new file mode 100644
index 0000000000..490fec2dc8
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/qpropertyResetCorrectlyLinked.qml
@@ -0,0 +1,8 @@
+import QtQuick
+
+Item {
+    property var val: undefined
+    property var observes: width
+    width: val
+    implicitWidth: 200
+}
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 788e78b0f1..69a342bcb9 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -29,6 +29,7 @@
 #include <private/qqmlvaluetypeproxybinding_p.h>
 #include <QtCore/private/qproperty_p.h>
 #include <QtQuick/qquickwindow.h>
+#include <QtQuick/private/qquickitem_p.h>
 #include <QtQuickTestUtils/private/qmlutils_p.h>
 #include <QtQuickTestUtils/private/testhttpserver_p.h>
 
@@ -379,6 +380,7 @@ private slots:
     void qpropertyBindingHandlesUndefinedWithoutResetCorrectly_data();
     void qpropertyBindingHandlesUndefinedWithoutResetCorrectly();
     void qpropertyBindingRestoresObserverAfterReset();
+    void qpropertyBindingObserverCorrectlyLinkedAfterReset();
     void hugeRegexpQuantifiers();
     void singletonTypeWrapperLookup();
     void getThisObject();
@@ -9441,6 +9443,21 @@ void tst_qqmlecmascript::qpropertyBindingRestoresObserverAfterReset()
     QVERIFY(o->property("steps").toInt() > 3);
 }
 
+void tst_qqmlecmascript::qpropertyBindingObserverCorrectlyLinkedAfterReset()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("qpropertyResetCorrectlyLinked.qml"));
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    std::unique_ptr<QObject> o(c.create());
+    QVERIFY(o);
+    QCOMPARE(o->property("width"), 200);
+    auto item = qobject_cast<QQuickItem *>(o.get());
+    auto itemPriv = QQuickItemPrivate::get(item);
+    QBindingStorage *storage = qGetBindingStorage(itemPriv);
+    QPropertyBindingDataPointer ptr { storage->bindingData(&itemPriv->width) };
+    QCOMPARE(ptr.observerCount(), 1);
+}
+
 void tst_qqmlecmascript::hugeRegexpQuantifiers()
 {
     QJSEngine engine;