Improve type conversions from/to QJSValue

We can convert everything into a QJSValue if we have an engine and we
can save a binding function in a QVariant by wrapping it into QJSValue.

Change-Id: I48e7c13f3f744f1c50bf673b427fe9331250f313
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

11 files changed, 84 insertions, 13 deletions

diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 93a15669ee..a5f3336584 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -1708,7 +1708,10 @@ static QVariant objectToVariant(QV4::ExecutionEngine *e, const QV4::Object *o, V
         }
 
         result = list;
-    } else if (!o->as<FunctionObject>()) {
+    } else if (const FunctionObject *f = o->as<FunctionObject>()) {
+        // If it's a FunctionObject, we can only save it as QJSValue.
+        result = QVariant::fromValue(QJSValuePrivate::fromReturnedValue(f->asReturnedValue()));
+    } else {
         QVariantMap map;
         QV4::Scope scope(e);
         QV4::ObjectIterator it(scope, o, QV4::ObjectIterator::EnumerableOnly);
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 6657f3d6fc..517fbab6f1 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -475,7 +475,9 @@ bool QObjectWrapper::setQmlProperty(
     return true;
 }
 
-void QObjectWrapper::setProperty(ExecutionEngine *engine, QObject *object, QQmlPropertyData *property, const Value &value)
+void QObjectWrapper::setProperty(
+        ExecutionEngine *engine, QObject *object,
+        const QQmlPropertyData *property, const Value &value)
 {
     if (!property->isWritable() && !property->isQList()) {
         QString error = QLatin1String("Cannot assign to read-only property \"") +
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index 37cb4d3cac..f90ce06d58 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -183,6 +183,9 @@ struct Q_QML_EXPORT QObjectWrapper : public Object
 
     static void setProperty(ExecutionEngine *engine, QObject *object, int propertyIndex, const Value &value);
     void setProperty(ExecutionEngine *engine, int propertyIndex, const Value &value);
+    static void setProperty(
+            ExecutionEngine *engine, QObject *object,
+            const QQmlPropertyData *property, const Value &value);
 
     void destroyObject(bool lastCall);
 
@@ -198,8 +201,6 @@ struct Q_QML_EXPORT QObjectWrapper : public Object
     static bool virtualResolveLookupSetter(Object *object, ExecutionEngine *engine, Lookup *lookup, const Value &value);
 
 protected:
-    static void setProperty(ExecutionEngine *engine, QObject *object, QQmlPropertyData *property, const Value &value);
-
     static bool virtualIsEqualTo(Managed *that, Managed *o);
     static ReturnedValue create(ExecutionEngine *engine, QObject *object);
 
diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index 5a814b16c5..e81834ef1a 100644
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -376,7 +376,8 @@ QObject *QQmlComponentPrivate::doBeginCreate(QQmlComponent *q, QQmlContext *cont
 
 bool QQmlComponentPrivate::setInitialProperty(QObject *component, const QString& name, const QVariant &value)
 {
-    QQmlProperty prop = QQmlComponentPrivate::removePropertyFromRequired(component, name, requiredProperties());
+    QQmlProperty prop = QQmlComponentPrivate::removePropertyFromRequired(
+            component, name, requiredProperties(), engine);
     QQmlPropertyPrivate *privProp = QQmlPropertyPrivate::get(prop);
     const bool isValid = prop.isValid();
     if (!isValid || !privProp->writeValueProperty(value, {})) {
@@ -1037,9 +1038,11 @@ void QQmlComponentPrivate::complete(QQmlEnginePrivate *enginePriv, ConstructionS
  * classes which create components should not need it and should only need to call
  * setInitialProperties.
  */
-QQmlProperty QQmlComponentPrivate::removePropertyFromRequired(QObject *createdComponent, const QString &name, RequiredProperties &requiredProperties, bool* wasInRequiredProperties)
+QQmlProperty QQmlComponentPrivate::removePropertyFromRequired(
+        QObject *createdComponent, const QString &name, RequiredProperties &requiredProperties,
+        QQmlEngine *engine, bool *wasInRequiredProperties)
 {
-    QQmlProperty prop(createdComponent, name);
+    QQmlProperty prop(createdComponent, name, engine);
     auto privProp = QQmlPropertyPrivate::get(prop);
     if (prop.isValid()) {
         // resolve outstanding required properties
@@ -1418,7 +1421,8 @@ void QQmlComponentPrivate::setInitialProperties(QV4::ExecutionEngine *engine, QV
             qmlWarning(createdComponent, engine->catchExceptionAsQmlError());
             continue;
         } else if (isTopLevelProperty) {
-            auto prop = removePropertyFromRequired(createdComponent, name->toQString(), requiredProperties);
+            auto prop = removePropertyFromRequired(createdComponent, name->toQString(),
+                                                   requiredProperties, engine->qmlEngine());
         }
     }
 
diff --git a/src/qml/qml/qqmlcomponent_p.h b/src/qml/qml/qqmlcomponent_p.h
index abc0de7438..59ef47f2c3 100644
--- a/src/qml/qml/qqmlcomponent_p.h
+++ b/src/qml/qml/qqmlcomponent_p.h
@@ -137,7 +137,9 @@ public:
     static void completeDeferred(QQmlEnginePrivate *enginePriv, DeferredState *deferredState);
 
     static void complete(QQmlEnginePrivate *enginePriv, ConstructionState *state);
-    static QQmlProperty removePropertyFromRequired(QObject *createdComponent, const QString &name, RequiredProperties& requiredProperties, bool *wasInRequiredProperties = nullptr);
+    static QQmlProperty removePropertyFromRequired(
+            QObject *createdComponent, const QString &name, RequiredProperties &requiredProperties,
+            QQmlEngine *engine, bool *wasInRequiredProperties = nullptr);
 
     QQmlEngine *engine;
     QQmlGuardedContextData creationContext;
diff --git a/src/qml/qml/qqmlincubator.cpp b/src/qml/qml/qqmlincubator.cpp
index 7d4ffcec3f..33b04eba14 100644
--- a/src/qml/qml/qqmlincubator.cpp
+++ b/src/qml/qml/qqmlincubator.cpp
@@ -304,7 +304,8 @@ void QQmlIncubatorPrivate::incubate(QQmlInstantiationInterrupt &i)
            for (auto it = initialProperties.cbegin(); it != initialProperties.cend(); ++it) {
                auto component = tresult;
                auto name = it.key();
-               QQmlProperty prop = QQmlComponentPrivate::removePropertyFromRequired(component, name, requiredProperties);
+               QQmlProperty prop = QQmlComponentPrivate::removePropertyFromRequired(
+                           component, name, requiredProperties, QQmlEnginePrivate::get(enginePriv));
                if (!prop.isValid() || !prop.write(it.value())) {
                    QQmlError error{};
                    error.setUrl(compilationUnit->url());
diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index d5f0e6c273..707b43cfd4 100644
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -56,6 +56,8 @@
 #include "qqmlvaluetypeproxybinding_p.h"
 #include <private/qjsvalue_p.h>
 #include <private/qv4functionobject_p.h>
+#include <private/qv4qobjectwrapper_p.h>
+#include <private/qqmlbuiltinfunctions_p.h>
 
 #include <QStringList>
 #include <QVector>
@@ -1433,6 +1435,21 @@ bool QQmlPropertyPrivate::write(
                 o = nullptr;
             prop.append(&prop, o);
         }
+    } 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(
+                    enginePriv->v4engine()->metaTypeToJS(variantMetaType, value.constData()));
+        return property.writeProperty(object, &jsValue, flags);
     } else {
         Q_ASSERT(variantMetaType != propertyMetaType);
 
diff --git a/src/qmlmodels/qqmldelegatemodel.cpp b/src/qmlmodels/qqmldelegatemodel.cpp
index 84c3c60411..e43e441856 100644
--- a/src/qmlmodels/qqmldelegatemodel.cpp
+++ b/src/qmlmodels/qqmldelegatemodel.cpp
@@ -1033,7 +1033,8 @@ void QQDMIncubationTask::initializeRequiredProperties(QQmlDelegateModelItem *mod
                 auto propName = QString::fromUtf8(prop.name());
                 bool wasInRequired = false;
                 QQmlProperty componentProp = QQmlComponentPrivate::removePropertyFromRequired(
-                        object, propName, requiredProperties, &wasInRequired);
+                            object, propName, requiredProperties,
+                            QQmlEnginePrivate::get(incubatorPriv->enginePriv), &wasInRequired);
                 // only write to property if it was actually requested by the component
                 if (wasInRequired && prop.hasNotifySignal()) {
                     QMetaMethod changeSignal = prop.notifySignal();
diff --git a/src/qmlmodels/qqmltableinstancemodel.cpp b/src/qmlmodels/qqmltableinstancemodel.cpp
index 5b058481ab..10b44af90b 100644
--- a/src/qmlmodels/qqmltableinstancemodel.cpp
+++ b/src/qmlmodels/qqmltableinstancemodel.cpp
@@ -425,7 +425,8 @@ bool QQmlTableInstanceModel::setRequiredProperty(int index, const QString &name,
         return false;
 
     QQmlProperty componentProp = QQmlComponentPrivate::removePropertyFromRequired(
-                modelItem->object, name, props, &wasInRequired);
+                modelItem->object, name, props, QQmlEnginePrivate::get(task->enginePriv),
+                &wasInRequired);
     if (wasInRequired)
         componentProp.write(value);
     return wasInRequired;
diff --git a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp
index 8ee31b8509..44900ef92b 100644
--- a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp
+++ b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp
@@ -1146,7 +1146,7 @@ void tst_QJSValue::toVariant()
         QVERIFY(func2.isCallable());
         QCOMPARE(func2.call().toInt(), 10);
 
-        QCOMPARE(func.toVariant(), QVariant());
+        QCOMPARE(func.toVariant().metaType(), QMetaType::fromType<QJSValue>());
     }
 }
 
diff --git a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
index a92da98d2b..64b2207201 100644
--- a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
+++ b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
@@ -46,6 +46,28 @@
 
 #include <algorithm>
 
+class WithQJSValue : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(QJSValue v READ v WRITE setV NOTIFY vChanged)
+
+public:
+    QJSValue v() const { return m_v; }
+    void setV(const QJSValue &newV)
+    {
+        if (!m_v.strictlyEquals(newV)) {
+            m_v = newV;
+            emit vChanged();
+        }
+    }
+
+signals:
+    void vChanged();
+
+private:
+    QJSValue m_v;
+};
+
 class MyIC : public QObject, public QQmlIncubationController
 {
     Q_OBJECT
@@ -130,6 +152,7 @@ private slots:
     void testSetInitialProperties();
     void createInsideJSModule();
     void qmlErrorIsReported();
+    void initJSValueProp();
 
 private:
     QQmlEngine engine;
@@ -1024,6 +1047,22 @@ void tst_qqmlcomponent::qmlErrorIsReported()
     }));
 }
 
+void tst_qqmlcomponent::initJSValueProp()
+{
+    qmlRegisterType<WithQJSValue>("ComponentTest", 1, 0, "WithQJSValue");
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData("import ComponentTest\nWithQJSValue {}", QUrl());
+    QVERIFY2(component.isReady(), qPrintable(component.errorString()));
+    QScopedPointer<QObject> o(component.createWithInitialProperties({{ u"v"_qs, 5}}));
+    QVERIFY(!o.isNull());
+    WithQJSValue *withQJSValue = qobject_cast<WithQJSValue *>(o.data());
+    QVERIFY(withQJSValue);
+    const QJSValue jsValue = withQJSValue->v();
+    QVERIFY(jsValue.isNumber());
+    QCOMPARE(jsValue.toInt(), 5);
+}
+
 QTEST_MAIN(tst_qqmlcomponent)
 
 #include "tst_qqmlcomponent.moc"