QML: Try QML conversion before metatype conversion

and guard against null gadgetPtr. This is what we get for uninitialized
value type properties.

Fixes: QTBUG-114494
Change-Id: I86ad23abcc4fec219017d1aad6b7add1c9a9af5d
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit a2104034d404179f5fad98fd54a133b288ded47d)

8 files changed, 144 insertions, 65 deletions

diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 8d3d5e6b46..de5940fc17 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -2622,8 +2622,23 @@ bool ExecutionEngine::metaTypeFromJS(const Value &value, QMetaType metaType, voi
         const QMetaType valueType = vtw->type();
         if (valueType == metaType)
             return vtw->toGadget(data);
-        if (QMetaType::canConvert(valueType, metaType))
-            return QMetaType::convert(valueType, vtw->d()->gadgetPtr(), metaType, data);
+
+        Heap::QQmlValueTypeWrapper *d = vtw->d();
+        if (d->isReference())
+            d->readReference();
+
+        if (void *gadgetPtr = d->gadgetPtr()) {
+            if (QQmlValueTypeProvider::createValueType(valueType, gadgetPtr, metaType, data))
+                return true;
+            if (QMetaType::canConvert(valueType, metaType))
+                return QMetaType::convert(valueType, gadgetPtr, metaType, data);
+        } else {
+            QVariant empty(valueType);
+            if (QQmlValueTypeProvider::createValueType(valueType, empty.data(), metaType, data))
+                return true;
+            if (QMetaType::canConvert(valueType, metaType))
+                return QMetaType::convert(valueType, empty.data(), metaType, data);
+        }
     }
 
     // Try to use magic; for compatibility with qjsvalue_cast.
@@ -2636,7 +2651,7 @@ bool ExecutionEngine::metaTypeFromJS(const Value &value, QMetaType metaType, voi
     if (variantObject) {
         // Actually a reference, because we're poking it for its data() below and we want
         // the _original_ data, not some copy.
-        const QVariant &var = variantObject->d()->data();
+        QVariant &var = variantObject->d()->data();
 
         if (var.metaType() == metaType) {
             metaType.destruct(data);
@@ -2685,7 +2700,8 @@ bool ExecutionEngine::metaTypeFromJS(const Value &value, QMetaType metaType, voi
                     proto = proto->getPrototypeOf();
                 }
             }
-        } else if (QQmlValueTypeProvider::createValueType(var, metaType, data)) {
+        } else if (QQmlValueTypeProvider::createValueType(
+                       var.metaType(), var.data(), metaType, data)) {
             return true;
         }
     } else if (value.isNull() && isPointer) {
diff --git a/src/qml/jsruntime/qv4sequenceobject.cpp b/src/qml/jsruntime/qv4sequenceobject.cpp
index 132084705c..a059ee93b7 100644
--- a/src/qml/jsruntime/qv4sequenceobject.cpp
+++ b/src/qml/jsruntime/qv4sequenceobject.cpp
@@ -716,7 +716,7 @@ QVariant SequencePrototype::toVariant(const QV4::Value &array, QMetaType typeHin
                 if (originalType != valueMetaType) {
                     QVariant converted(valueMetaType);
                     if (QQmlValueTypeProvider::createValueType(
-                                variant, valueMetaType, converted.data())) {
+                            originalType, variant.data(), valueMetaType, converted.data())) {
                         variant = converted;
                     } else if (!variant.convert(valueMetaType)) {
                         qWarning().noquote()
diff --git a/src/qml/qml/qqmlglobal.cpp b/src/qml/qml/qqmlglobal.cpp
index 01734b92c9..398011226f 100644
--- a/src/qml/qml/qqmlglobal.cpp
+++ b/src/qml/qml/qqmlglobal.cpp
@@ -95,31 +95,32 @@ bool fromMatchingType(
             continue;
 
         const QMetaType parameterType = ctor.parameterMetaType(0);
-
-        QVariant source = retrieve(parameterType);
-        const QMetaType sourceMetaType = source.metaType();
-        if (sourceMetaType == parameterType) {
-            callConstructor(targetMetaObject, i, targetMetaType, target, source.data());
-            return true;
-        }
-
-        if (const QMetaObject *parameterMetaObject = parameterType.metaObject()) {
-            if (const QMetaObject *sourceMetaObject = sourceMetaType.metaObject();
-                    sourceMetaObject && sourceMetaObject->inherits(parameterMetaObject)) {
-                // Allow construction from derived types.
-                callConstructor(targetMetaObject, i, targetMetaType, target, source.data());
+        if (retrieve(parameterType, [&](QMetaType sourceMetaType, void *sourceData) {
+            if (sourceMetaType == parameterType) {
+                callConstructor(targetMetaObject, i, targetMetaType, target, sourceData);
                 return true;
             }
-        }
+            if (const QMetaObject *parameterMetaObject = parameterType.metaObject()) {
+                if (const QMetaObject *sourceMetaObject = sourceMetaType.metaObject();
+                    sourceMetaObject && sourceMetaObject->inherits(parameterMetaObject)) {
+                    // Allow construction from derived types.
+                    callConstructor(targetMetaObject, i, targetMetaType, target, sourceData);
+                    return true;
+                }
+            }
 
-        // Do not recursively try to create parameters here. This may end up in infinite recursion.
+            // Do not recursively try to create parameters here.
+            // This may end up in infinite recursion.
 
-        // At this point, s should be a builtin type. For builtin types
-        // the QMetaType converters are good enough.
-        QVariant converted(parameterType);
-        if (QMetaType::convert(sourceMetaType, source.constData(),
-                               parameterType, converted.data())) {
-            callConstructor(targetMetaObject, i, targetMetaType, target, converted.data());
+            // At this point, s should be a builtin type. For builtin types
+            // the QMetaType converters are good enough.
+            QVariant converted(parameterType);
+            if (QMetaType::convert(sourceMetaType, sourceData, parameterType, converted.data())) {
+                callConstructor(targetMetaObject, i, targetMetaType, target, converted.data());
+                return true;
+            }
+            return false;
+        })) {
             return true;
         }
     }
@@ -131,18 +132,12 @@ static bool fromMatchingType(
     const QMetaObject *targetMetaObject, QMetaType targetMetaType, void *target,
     const QV4::Value &source)
 {
-    return fromMatchingType(targetMetaObject, targetMetaType, target, [&](QMetaType parameterType) {
-        return QV4::ExecutionEngine::toVariant(source, parameterType);
-    });
-}
-
-static bool fromMatchingType(
-        const QMetaObject *targetMetaObject, QMetaType targetMetaType, void *target,
-        QVariant source)
-{
-    return fromMatchingType(targetMetaObject, targetMetaType, target, [&](QMetaType) {
-        return source;
-    });
+    return fromMatchingType(
+        targetMetaObject, targetMetaType, target,
+        [&](QMetaType parameterType, auto callback) {
+            QVariant variant = QV4::ExecutionEngine::toVariant(source, parameterType);
+            return callback(variant.metaType(), variant.data());
+        });
 }
 
 static bool fromString(
@@ -267,7 +262,8 @@ static void doWriteProperties(
             continue;
         }
         QVariant converted(propertyType);
-        if (QQmlValueTypeProvider::createValueType(property, propertyType, converted.data())) {
+        if (QQmlValueTypeProvider::createValueType(
+                property.metaType(), property.data(), propertyType, converted.data())) {
             metaProperty.writeOnGadget(target, std::move(converted));
             continue;
         }
@@ -332,7 +328,8 @@ void doWriteProperties(const QMetaObject *targetMetaObject, void *target, const
             continue;
         }
         QVariant converted(propertyType);
-        if (QQmlValueTypeProvider::createValueType(property, propertyType, converted.data())) {
+        if (QQmlValueTypeProvider::createValueType(
+                property.metaType(), property.data(), propertyType, converted.data())) {
             metaProperty.writeOnGadget(target, std::move(converted));
             continue;
         }
@@ -357,31 +354,32 @@ bool byProperties(
 }
 
 static bool byProperties(
-        const QMetaObject *targetMetaObject, void *target, const QVariant &source)
+    const QMetaObject *targetMetaObject, void *target,
+    QMetaType sourceMetaType, const void *source)
 {
     if (!targetMetaObject)
         return false;
 
-    if (source.metaType() == QMetaType::fromType<QJSValue>()) {
-        QJSValue val = source.value<QJSValue>();
+    if (sourceMetaType == QMetaType::fromType<QJSValue>()) {
+        const QJSValue *val = static_cast<const QJSValue *>(source);
         return byProperties(
-            targetMetaObject, target, QV4::Value(QJSValuePrivate::asReturnedValue(&val)));
+            targetMetaObject, target, QV4::Value(QJSValuePrivate::asReturnedValue(val)));
     }
 
-    if (source.metaType() == QMetaType::fromType<QVariantMap>()) {
+    if (sourceMetaType == QMetaType::fromType<QVariantMap>()) {
         return byProperties(
             targetMetaObject, target,
-            *static_cast<const QVariantMap *>(source.constData()));
+            *static_cast<const QVariantMap *>(source));
     }
-    if (source.metaType() == QMetaType::fromType<QVariantHash>()) {
+    if (sourceMetaType == QMetaType::fromType<QVariantHash>()) {
         return byProperties(
             targetMetaObject, target,
-            *static_cast<const QVariantHash *>(source.constData()));
+            *static_cast<const QVariantHash *>(source));
     }
-    if (source.metaType().flags() & QMetaType::PointerToQObject)
-        return byProperties(targetMetaObject, target, source.value<QObject *>());
-    if (const QMetaObject *sourceMeta = QQmlMetaType::metaObjectForValueType(source.metaType()))
-        return byProperties(targetMetaObject, target, sourceMeta, source.constData());
+    if (sourceMetaType.flags() & QMetaType::PointerToQObject)
+        return byProperties(targetMetaObject, target, *static_cast<QObject *const *>(source));
+    if (const QMetaObject *sourceMeta = QQmlMetaType::metaObjectForValueType(sourceMetaType))
+        return byProperties(targetMetaObject, target, sourceMeta, source);
 
     return false;
 }
@@ -439,9 +437,11 @@ bool QQmlValueTypeProvider::createValueType(
 
         if (targetType.canConstructValueType()
                 && fromMatchingType(
-                    targetMetaObject, targetMetaType, target, [&](QMetaType parameterType) {
-                        return QV4::ExecutionEngine::toVariant(
+                    targetMetaObject, targetMetaType, target,
+                    [&](QMetaType parameterType, auto callback) {
+                        QVariant variant = QV4::ExecutionEngine::toVariant(
                             QJSValuePrivate::asReturnedValue(&source), parameterType);
+                        return callback(variant.metaType(), variant.data());
                     })) {
             return true;
         }
@@ -475,25 +475,60 @@ bool QQmlValueTypeProvider::createValueType(
 
 }
 
-/*!
- * \internal
- * This should only be called with either builtin types or wrapped QJSValues as source.
- */
 bool QQmlValueTypeProvider::createValueType(
-        const QVariant &source, QMetaType targetMetaType, void *target)
+    const QVariant &source, QMetaType targetMetaType, void *target)
 {
     if (!isConstructibleMetaType(targetMetaType))
         return false;
     const QQmlType targetType = QQmlMetaType::qmlType(targetMetaType);
-    if (const QMetaObject *targetMeaObject = QQmlMetaType::metaObjectForValueType(targetType)) {
-        if (targetType.canPopulateValueType() && byProperties(targetMeaObject, target, source))
+    if (const QMetaObject *targetMetaObject = QQmlMetaType::metaObjectForValueType(targetType)) {
+        if (targetType.canPopulateValueType()
+                && byProperties(targetMetaObject, target, source.metaType(), source.data())) {
             return true;
+        }
+
         if (targetType.canConstructValueType()) {
-            if (fromMatchingType(targetMeaObject, targetMetaType, target, source))
+            if (fromMatchingType(targetMetaObject, targetMetaType, target,
+                                 [&](QMetaType, auto callback) {
+                QVariant nonConstSource = source;
+                return callback(nonConstSource.metaType(), nonConstSource.data());
+            })) {
                 return true;
+            }
+
             qWarning().noquote()
-                    << "Could not find any constructor for value type"
-                    << targetMeaObject->className() << "to call with value" << source;
+                << "Could not find any constructor for value type"
+                << targetMetaObject->className() << "to call with value" << source;
+        }
+    }
+
+    return false;
+}
+
+bool QQmlValueTypeProvider::createValueType(
+    QMetaType sourceMetaType, void *source, QMetaType targetMetaType, void *target)
+{
+    if (!isConstructibleMetaType(targetMetaType))
+        return false;
+    const QQmlType targetType = QQmlMetaType::qmlType(targetMetaType);
+    if (const QMetaObject *targetMetaObject = QQmlMetaType::metaObjectForValueType(targetType)) {
+        if (targetType.canPopulateValueType()
+                && byProperties(targetMetaObject, target, sourceMetaType, source)) {
+            return true;
+        }
+
+        if (targetType.canConstructValueType()) {
+            if (fromMatchingType(targetMetaObject, targetMetaType, target,
+                                 [&](QMetaType, auto callback) {
+                    return callback(sourceMetaType, source);
+                })) {
+                return true;
+            }
+
+            qWarning().noquote()
+                << "Could not find any constructor for value type"
+                << targetMetaObject->className() << "to call with value"
+                << QVariant(sourceMetaType, source);
         }
     }
 
diff --git a/src/qml/qml/qqmlglobal_p.h b/src/qml/qml/qqmlglobal_p.h
index f7a50e8350..2a7b5eaad1 100644
--- a/src/qml/qml/qqmlglobal_p.h
+++ b/src/qml/qml/qqmlglobal_p.h
@@ -191,6 +191,7 @@ public:
     static bool createValueType(const QJSValue &, QMetaType, void *);
     static bool createValueType(const QV4::Value &, QMetaType, void *);
     static bool createValueType(const QVariant &, QMetaType, void *);
+    static bool createValueType(QMetaType, void *, QMetaType, void *);
 };
 
 class Q_QML_PRIVATE_EXPORT QQmlColorProvider
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 62e7096577..a4b1205204 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -616,7 +616,8 @@ void QQmlObjectCreator::setPropertyValue(const QQmlPropertyData *property, const
             }
 
             QVariant target(propertyType);
-            if (QQmlValueTypeProvider::createValueType(source, propertyType, target.data())) {
+            if (QQmlValueTypeProvider::createValueType(
+                    source.metaType(), source.data(), propertyType, target.data())) {
                 property->writeProperty(_qobject, target.data(), propertyWriteFlags);
                 break;
             }
diff --git a/tests/auto/qml/qqmlvaluetypeproviders/data/structuredValueTypes.qml b/tests/auto/qml/qqmlvaluetypeproviders/data/structuredValueTypes.qml
index b0ae1cda1c..abd6f6af08 100644
--- a/tests/auto/qml/qqmlvaluetypeproviders/data/structuredValueTypes.qml
+++ b/tests/auto/qml/qqmlvaluetypeproviders/data/structuredValueTypes.qml
@@ -53,4 +53,9 @@ MyTypeObject {
 
     property var insanity: ({})
     property structured fromInsanity: acceptStructured(insanity)
+
+    property rect newItemPadding
+    function updatePadding() {
+        setEffectPadding(newItemPadding);
+    }
 }
diff --git a/tests/auto/qml/qqmlvaluetypeproviders/testtypes.h b/tests/auto/qml/qqmlvaluetypeproviders/testtypes.h
index 424995b594..096212822d 100644
--- a/tests/auto/qml/qqmlvaluetypeproviders/testtypes.h
+++ b/tests/auto/qml/qqmlvaluetypeproviders/testtypes.h
@@ -371,6 +371,15 @@ public:
         return a;
     }
 
+    Q_INVOKABLE void setEffectPadding(const QRect &r)
+    {
+        m_hasEffectPadding = true;
+        m_effectPadding = r;
+    }
+
+    bool hasEffectPadding() const { return m_hasEffectPadding; }
+    QRectF effectPadding() const { return m_effectPadding; }
+
 signals:
     void changed();
     void runScript();
@@ -396,6 +405,8 @@ private:
     QTime m_aTime;
     QVariant m_aVariant;
     BarrenValueType m_barren;
+    QRectF m_effectPadding;
+    bool m_hasEffectPadding = false;
 };
 
 void registerTypes();
diff --git a/tests/auto/qml/qqmlvaluetypeproviders/tst_qqmlvaluetypeproviders.cpp b/tests/auto/qml/qqmlvaluetypeproviders/tst_qqmlvaluetypeproviders.cpp
index fa4bf6e058..5f4d7c1042 100644
--- a/tests/auto/qml/qqmlvaluetypeproviders/tst_qqmlvaluetypeproviders.cpp
+++ b/tests/auto/qml/qqmlvaluetypeproviders/tst_qqmlvaluetypeproviders.cpp
@@ -446,6 +446,16 @@ void tst_qqmlvaluetypeproviders::structured()
     fromObject.setP(QPointF(3, 4));
 
     QCOMPARE(o->property("fromInsanity").value<StructuredValueType>(), fromObject);
+
+    const MyTypeObject *m = static_cast<const MyTypeObject *>(po);
+    QVERIFY(!m->hasEffectPadding());
+    QMetaObject::invokeMethod(po, "updatePadding");
+    QVERIFY(m->hasEffectPadding());
+    QCOMPARE(m->effectPadding(), QRectF());
+    po->setProperty("newItemPadding", QRectF(1, 2, 3, 4));
+    QMetaObject::invokeMethod(po, "updatePadding");
+    QVERIFY(m->hasEffectPadding());
+    QCOMPARE(m->effectPadding(), QRectF(1, 2, 3, 4));
 }
 
 void tst_qqmlvaluetypeproviders::recursive()