Store a QV4::ReturnedValue in QJSValue

Being careful, we can now save primitive values inline. We use the heap
pointer of QV4::Value as either QString* or QV4::Value* for complex
types. We cannot store persistent managed QV4::Value without the double
indirection as those need to be allocated in a special place.

The generic QVariant case is not supported anymore. The only place where
it was actually needed were the stream operators for QJSValue. Those
were fundamentally broken:

* A managed QJSValue saved and loaded from a stream was converted to a
  QVariant-type QJSValue
* QVariant-type QJSValues were not callable, could not be objects or
  arrays, or any of the special types.
* Cyclic references were forcibly broken when saving to a data stream.

In general the support for saving and loading of managed types to/from
a data stream was so abysmally bad that we don't lose much by dropping
it.

[ChangeLog][QML][Important Behavior Changes] When saving a QJSValue to a
QDataStream only primitive values or strings will be retained. Support
for objects and arrays was incomplete and unreliable already before. It
cannot work correctly as we don't necessarily have a JavaScript heap
when loading a QJSValue from a stream. Therefore, we don't have a proper
place to keep any managed values. Using QVariant to keep them instead is
a bad idea because QVariant cannot represent everything a QJSValue can
contain.

Fixes: QTBUG-75174
Change-Id: I75697670639bca8d4b1668763d7020c4cf871bda
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

6 files changed, 27 insertions, 14 deletions

diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp
index b9566d5862..59f2759c81 100644
--- a/src/qml/qml/qqmlbinding.cpp
+++ b/src/qml/qml/qqmlbinding.cpp
@@ -57,6 +57,7 @@
 #include <private/qv4qobjectwrapper_p.h>
 #include <private/qv4variantobject_p.h>
 #include <private/qv4jscall_p.h>
+#include <private/qjsvalue_p.h>
 
 #include <qtqml_tracepoints_p.h>
 
@@ -455,9 +456,10 @@ Q_NEVER_INLINE bool QQmlBinding::slowWrite(const QQmlPropertyData &core,
             delayedError()->setErrorDescription(QLatin1String("Invalid use of Qt.binding() in a binding declaration."));
             return false;
         }
-        QQmlPropertyPrivate::writeValueProperty(m_target.data(), core, valueTypeData, QVariant::fromValue(
-                               QJSValue(v4engine, result.asReturnedValue())),
-                           context(), flags);
+        QQmlPropertyPrivate::writeValueProperty(
+                    m_target.data(), core, valueTypeData,
+                    QVariant::fromValue(QJSValuePrivate::fromReturnedValue(result.asReturnedValue())),
+                    context(), flags);
     } else if (isUndefined) {
         const QLatin1String typeName(QMetaType::typeName(type)
                                      ? QMetaType::typeName(type)
diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp
index b347bb3829..6de4ef153e 100644
--- a/src/qml/qml/qqmlboundsignal.cpp
+++ b/src/qml/qml/qqmlboundsignal.cpp
@@ -202,10 +202,7 @@ void QQmlBoundSignalExpression::evaluate(void **a)
         //    for several cases (such as QVariant type and QObject-derived types)
         //args[ii] = engine->metaTypeToJS(type, a[ii + 1]);
         if (type == qMetaTypeId<QJSValue>()) {
-            if (QV4::Value *v4Value = QJSValuePrivate::valueForData(reinterpret_cast<QJSValue *>(a[ii + 1]), &jsCall->args[ii]))
-                jsCall->args[ii] = *v4Value;
-            else
-                jsCall->args[ii] = QV4::Encode::undefined();
+            jsCall->args[ii] = QJSValuePrivate::asReturnedValue(reinterpret_cast<QJSValue *>(a[ii + 1]));
         } else if (type == QMetaType::QVariant) {
             jsCall->args[ii] = scope.engine->fromVariant(*((QVariant *)a[ii + 1]));
         } else if (type == QMetaType::Int) {
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index c2f8459fb1..0d9326b9a9 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -2518,7 +2518,7 @@ QJSValue QQmlEnginePrivate::singletonInstance<QJSValue>(const QQmlType &type)
             // should behave identically to QML singleton types.
             q->setContextForObject(o, new QQmlContext(q->rootContext(), q));
         }
-        singletonInstances.insert(type, value);
+        singletonInstances.convertAndInsert(v4engine(), type, &value);
 
     } else if (siinfo->qobjectCallback) {
         QObject *o = siinfo->qobjectCallback(q, q);
@@ -2536,12 +2536,12 @@ QJSValue QQmlEnginePrivate::singletonInstance<QJSValue>(const QQmlType &type)
         // should behave identically to QML singleton types.
         q->setContextForObject(o, new QQmlContext(q->rootContext(), q));
         value = q->newQObject(o);
-        singletonInstances.insert(type, value);
+        singletonInstances.convertAndInsert(v4engine(), type, &value);
     } else if (!siinfo->url.isEmpty()) {
         QQmlComponent component(q, siinfo->url, QQmlComponent::PreferSynchronous);
         QObject *o = component.beginCreate(q->rootContext());
         value = q->newQObject(o);
-        singletonInstances.insert(type, value);
+        singletonInstances.convertAndInsert(v4engine(), type, &value);
         component.completeCreate();
     }
 
diff --git a/src/qml/qml/qqmlengine_p.h b/src/qml/qml/qqmlengine_p.h
index fe42d4e347..23c69651ed 100644
--- a/src/qml/qml/qqmlengine_p.h
+++ b/src/qml/qml/qqmlengine_p.h
@@ -67,6 +67,7 @@
 #include <private/qrecyclepool_p.h>
 #include <private/qfieldlist_p.h>
 #include <private/qv4engine_p.h>
+#include <private/qjsvalue_p.h>
 
 #include <QtCore/qlist.h>
 #include <QtCore/qpair.h>
@@ -285,7 +286,20 @@ public:
     }
 
 private:
-    QHash<QQmlType, QJSValue> singletonInstances;
+    class SingletonInstances : private QHash<QQmlType, QJSValue>
+    {
+    public:
+        void convertAndInsert(QV4::ExecutionEngine *engine, const QQmlType &type, QJSValue *value)
+        {
+            QJSValuePrivate::manageStringOnV4Heap(engine, value);
+            insert(type, *value);
+        }
+
+        using QHash<QQmlType, QJSValue>::value;
+        using QHash<QQmlType, QJSValue>::take;
+    };
+
+    SingletonInstances singletonInstances;
     QHash<int, QQmlGadgetPtrWrapper *> cachedValueTypeInstances;
 
     // These members must be protected by a QQmlEnginePrivate::Locker as they are required by
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 040fa16af8..e0187eb75a 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -1068,7 +1068,7 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper
                 _vmeMetaObject->setVMEProperty(bindingProperty->coreIndex(), wrappedObject);
             } else {
                 QJSValue value;
-                QJSValuePrivate::setValue(&value, v4, wrappedObject);
+                QJSValuePrivate::setValue(&value, wrappedObject);
                 argv[0] = &value;
                 QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv);
             }
diff --git a/src/qml/qml/qqmltypewrapper.cpp b/src/qml/qml/qqmltypewrapper.cpp
index 7bcc5e9900..c787d4092e 100644
--- a/src/qml/qml/qqmltypewrapper.cpp
+++ b/src/qml/qml/qqmltypewrapper.cpp
@@ -233,7 +233,7 @@ ReturnedValue QQmlTypeWrapper::virtualGet(const Managed *m, PropertyKey id, cons
                 QJSValue scriptSingleton = e->singletonInstance<QJSValue>(type);
                 if (!scriptSingleton.isUndefined()) {
                     // NOTE: if used in a binding, changes will not trigger re-evaluation since non-NOTIFYable.
-                    QV4::ScopedObject o(scope, QJSValuePrivate::convertedToValue(v4, scriptSingleton));
+                    QV4::ScopedObject o(scope, QJSValuePrivate::asReturnedValue(&scriptSingleton));
                     if (!!o)
                         return o->get(name);
                 }
@@ -349,7 +349,7 @@ bool QQmlTypeWrapper::virtualPut(Managed *m, PropertyKey id, const Value &value,
         } else {
             QJSValue scriptSingleton = e->singletonInstance<QJSValue>(type);
             if (!scriptSingleton.isUndefined()) {
-                QV4::ScopedObject apiprivate(scope, QJSValuePrivate::convertedToValue(scope.engine, scriptSingleton));
+                QV4::ScopedObject apiprivate(scope, QJSValuePrivate::asReturnedValue(&scriptSingleton));
                 if (!apiprivate) {
                     QString error = QLatin1String("Cannot assign to read-only property \"") + name->toQString() + QLatin1Char('\"');
                     scope.engine->throwError(error);