From 748411fa64412db1650e04ee7b4405b8fbc53d42 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 4 Mar 2020 16:46:42 +0100 Subject: 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 --- src/qml/qml/qqmlbinding.cpp | 8 +++++--- src/qml/qml/qqmlboundsignal.cpp | 5 +---- src/qml/qml/qqmlengine.cpp | 6 +++--- src/qml/qml/qqmlengine_p.h | 16 +++++++++++++++- src/qml/qml/qqmlobjectcreator.cpp | 2 +- src/qml/qml/qqmltypewrapper.cpp | 4 ++-- 6 files changed, 27 insertions(+), 14 deletions(-) (limited to 'src/qml/qml') 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 #include #include +#include #include @@ -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()) { - if (QV4::Value *v4Value = QJSValuePrivate::valueForData(reinterpret_cast(a[ii + 1]), &jsCall->args[ii])) - jsCall->args[ii] = *v4Value; - else - jsCall->args[ii] = QV4::Encode::undefined(); + jsCall->args[ii] = QJSValuePrivate::asReturnedValue(reinterpret_cast(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(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(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 #include #include +#include #include #include @@ -285,7 +286,20 @@ public: } private: - QHash singletonInstances; + class SingletonInstances : private QHash + { + public: + void convertAndInsert(QV4::ExecutionEngine *engine, const QQmlType &type, QJSValue *value) + { + QJSValuePrivate::manageStringOnV4Heap(engine, value); + insert(type, *value); + } + + using QHash::value; + using QHash::take; + }; + + SingletonInstances singletonInstances; QHash 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(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(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); -- cgit v1.2.3