From 57e272fbd2bb4af083c640e8721e5ecbe869e384 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Tue, 9 May 2023 09:56:06 +0200
Subject: Value types: Prefer assigning bindings instead of converting

When we're assigning a QJSValue to a value type property, we must first
check whether it is a binding created by Qt.binding. In that case, we
should directly set that binding up. Going through the conversion code
path might not necessarily fail, but might instead yield nonsense.

Pick-to: 6.5
Fixes: QTBUG-113472
Change-Id: If91e0843f0caf36c96b4c811b9ce8076adfc984f
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
---
 src/qml/qml/qqmlproperty.cpp                       | 22 ++++++++++++----------
 .../qqmlcomponent/data/createObjectWithScript.qml  | 12 ++++++++++++
 tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp |  8 +++++++-
 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index 0e0d798537..ba82449692 100644
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -1373,6 +1373,18 @@ static ConvertAndAssignResult tryConvertAndAssign(
         return {false, false};
     }
 
+    if (variantMetaType == QMetaType::fromType<QJSValue>()) {
+        // Handle Qt.binding bindings here to avoid mistaken conversion below
+        const QJSValue &jsValue = get<QJSValue>(value);
+        const QV4::FunctionObject *f
+                = QJSValuePrivate::asManagedType<QV4::FunctionObject>(&jsValue);
+        if (f && f->isBinding()) {
+            QV4::QObjectWrapper::setProperty(
+                    f->engine(), object, &property, f->asReturnedValue());
+            return {true, true};
+        }
+    }
+
     // common cases:
     switch (propertyMetaType.id()) {
     case QMetaType::Bool:
@@ -1597,16 +1609,6 @@ bool QQmlPropertyPrivate::write(
                 sequence.addValue(list.data(), value.data());
             property.writeProperty(object, list.data(), flags);
         }
-    } else if (variantMetaType == QMetaType::fromType<QJSValue>()) {
-        QJSValue jsValue = qvariant_cast<QJSValue>(value);
-        const QV4::FunctionObject *f
-                = QJSValuePrivate::asManagedType<QV4::FunctionObject>(&jsValue);
-        if (f && f->isBinding()) {
-            QV4::QObjectWrapper::setProperty(
-                    f->engine(), object, &property, f->asReturnedValue());
-            return true;
-        }
-        return false;
     } else if (enginePriv && propertyMetaType == QMetaType::fromType<QJSValue>()) {
         // We can convert everything into a QJSValue if we have an engine.
         QJSValue jsValue = QJSValuePrivate::fromReturnedValue(
diff --git a/tests/auto/qml/qqmlcomponent/data/createObjectWithScript.qml b/tests/auto/qml/qqmlcomponent/data/createObjectWithScript.qml
index 3391b3a266..3cc87afa16 100644
--- a/tests/auto/qml/qqmlcomponent/data/createObjectWithScript.qml
+++ b/tests/auto/qml/qqmlcomponent/data/createObjectWithScript.qml
@@ -11,6 +11,9 @@ Item{
     property QtObject badRequired: null
     property QtObject goodRequired: null
 
+    property QtObject bindingAsInitial: null
+    property bool bindingUsed: false
+
     Component{
         id: a
         Rectangle {
@@ -43,6 +46,11 @@ Item{
         }
     }
 
+    Component {
+        id: e
+        Rectangle {}
+    }
+
     Component.onCompleted: {
         root.declarativerectangle = a.createObject(root, {"x":17,"y":17, "color":"white", "border.width":3, "innerRect.border.width": 20});
         root.declarativeitem = b.createObject(root, {"x":17,"y":17,"testBool":true,"testInt":17,"testObject":root});
@@ -52,5 +60,9 @@ Item{
 
         root.badRequired = d.createObject(root, { "not_i": 42 });
         root.goodRequired = d.createObject(root, { "i": 42 });
+
+        root.bindingAsInitial = e.createObject(root, {color: Qt.binding(() => {
+            root.bindingUsed = true; return '#ff0000'
+        })});
     }
 }
diff --git a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
index 0af3db58fa..d6a14cd3ef 100644
--- a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
+++ b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
@@ -284,7 +284,7 @@ void tst_qqmlcomponent::qmlCreateObjectWithProperties()
     QTest::ignoreMessage(
             QtMsgType::QtWarningMsg,
             QRegularExpression(
-                    ".*createObjectWithScript.qml:42:13: Required property i was not initialized"));
+                    ".*createObjectWithScript.qml:45:13: Required property i was not initialized"));
 
     QQmlComponent component(&engine, testFileUrl("createObjectWithScript.qml"));
     QVERIFY2(component.errorString().isEmpty(), component.errorString().toUtf8());
@@ -344,6 +344,12 @@ void tst_qqmlcomponent::qmlCreateObjectWithProperties()
         QCOMPARE(goodRequired->parent(), object.data());
         QCOMPARE(goodRequired->property("i").value<int>(), 42);
     }
+
+    {
+        QScopedPointer<QObject> bindingAsInitial(object->property("bindingAsInitial").value<QObject *>());
+        QVERIFY(bindingAsInitial);
+        QVERIFY(object->property("bindingUsed").toBool());
+    }
 }
 
 void tst_qqmlcomponent::qmlCreateObjectClean()
-- 
cgit v1.2.1