From c9ffed92a0dee0ae00a9632177ea42f85ea8a48c Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 2 May 2016 16:55:36 +0200 Subject: Fix crash with window-less QQuickItems Mark QQuickItem visual children directly in QQuickItem instead of relying on the item being a (grand) child of a window. [ChangeLog][QtQuick] Fix crash with QQuickItems created via JavaScript being garbage collected sometimes when they're not assigned to a window. This may happen even in qmlscene when between the creation of the root item and the assignment to the QQuickWindow the garbage collector runs. The previous approach of a persistent in QQuickView marking the visual item hierarchy relies on the existence of a view. The only thing left to do in the view and qml window implementation is enforcing the CppOwnership policy set on the content item in QQuickWindow by ensuring the presence of the JS wrapper, replacing the persistent with a weak value. This also introduces a new internal mechanism for QObject sub-classes to provide their own V4 JS wrapper types. Task-number: QTBUG-39888 Change-Id: Icd45a636a6d4e4528fc19165b13f4e1ca7967087 Reviewed-by: Erik Verbruggen --- src/qml/jsruntime/qv4qobjectwrapper.cpp | 10 ++++++-- src/qml/jsruntime/qv4qobjectwrapper_p.h | 2 +- src/qml/qml/qqmldata_p.h | 2 +- src/qml/qml/qqmlengine.cpp | 11 +++++---- src/qml/qml/qqmlpropertycache.cpp | 14 +++++++---- src/qml/qml/qqmlpropertycache_p.h | 16 +++++++++++++ src/quick/items/qquickitem.cpp | 41 +++++++++++++++++++++++++-------- src/quick/items/qquickitem.h | 2 ++ src/quick/items/qquickitem_p.h | 4 +--- src/quick/items/qquickview.cpp | 25 +++----------------- src/quick/items/qquickview_p.h | 29 ----------------------- src/quick/items/qquickwindowmodule.cpp | 7 +++--- 12 files changed, 83 insertions(+), 80 deletions(-) (limited to 'src') diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp index 6be86e3bdb..246df4c4e9 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper.cpp +++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp @@ -671,8 +671,14 @@ bool QObjectWrapper::isEqualTo(Managed *a, Managed *b) ReturnedValue QObjectWrapper::create(ExecutionEngine *engine, QObject *object) { - if (engine->jsEngine()) - QQmlData::ensurePropertyCache(engine->jsEngine(), object); + if (QJSEngine *jsEngine = engine->jsEngine()) { + if (QQmlPropertyCache *cache = QQmlData::ensurePropertyCache(jsEngine, object)) { + ReturnedValue result = QV4::Encode::null(); + void *args[] = { &result, &engine }; + if (cache->callJSFactoryMethod(object, args)) + return result; + } + } return (engine->memoryManager->allocObject(object))->asReturnedValue(); } diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h index 0fc39b222f..6af86ef446 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper_p.h +++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h @@ -71,7 +71,7 @@ namespace Heap { struct QQmlValueTypeWrapper; -struct QObjectWrapper : Object { +struct Q_QML_EXPORT QObjectWrapper : Object { QObjectWrapper(QObject *object); QPointer object; }; diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h index ef05dd1fd7..e0e5b221ec 100644 --- a/src/qml/qml/qqmldata_p.h +++ b/src/qml/qml/qqmldata_p.h @@ -217,7 +217,7 @@ public: static inline void flushPendingBinding(QObject *, int coreIndex); - static void ensurePropertyCache(QJSEngine *engine, QObject *object); + static QQmlPropertyCache *ensurePropertyCache(QJSEngine *engine, QObject *object); private: // For attachedProperties diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index 8f22533472..5db918dd73 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -1808,14 +1808,15 @@ void QQmlData::setPendingBindingBit(QObject *obj, int coreIndex) QQmlData_setBit(this, obj, coreIndex * 2 + 1); } -void QQmlData::ensurePropertyCache(QJSEngine *engine, QObject *object) +QQmlPropertyCache *QQmlData::ensurePropertyCache(QJSEngine *engine, QObject *object) { Q_ASSERT(engine); QQmlData *ddata = QQmlData::get(object, /*create*/true); - if (ddata->propertyCache) - return; - ddata->propertyCache = QJSEnginePrivate::get(engine)->cache(object); - if (ddata->propertyCache) ddata->propertyCache->addref(); + if (!ddata->propertyCache){ + ddata->propertyCache = QJSEnginePrivate::get(engine)->cache(object); + if (ddata->propertyCache) ddata->propertyCache->addref(); + } + return ddata->propertyCache; } void QQmlEnginePrivate::sendQuit() diff --git a/src/qml/qml/qqmlpropertycache.cpp b/src/qml/qml/qqmlpropertycache.cpp index e8c9989fdf..be730c7ac9 100644 --- a/src/qml/qml/qqmlpropertycache.cpp +++ b/src/qml/qml/qqmlpropertycache.cpp @@ -234,7 +234,7 @@ Creates a new empty QQmlPropertyCache. QQmlPropertyCache::QQmlPropertyCache(QV4::ExecutionEngine *e) : engine(e), _parent(0), propertyIndexCacheStart(0), methodIndexCacheStart(0), signalHandlerIndexCacheStart(0), _hasPropertyOverrides(false), _ownMetaObject(false), - _metaObject(0), argumentsCache(0) + _metaObject(0), argumentsCache(0), _jsFactoryMethodIndex(-1) { Q_ASSERT(engine); } @@ -245,7 +245,7 @@ Creates a new QQmlPropertyCache of \a metaObject. QQmlPropertyCache::QQmlPropertyCache(QV4::ExecutionEngine *e, const QMetaObject *metaObject) : engine(e), _parent(0), propertyIndexCacheStart(0), methodIndexCacheStart(0), signalHandlerIndexCacheStart(0), _hasPropertyOverrides(false), _ownMetaObject(false), - _metaObject(0), argumentsCache(0) + _metaObject(0), argumentsCache(0), _jsFactoryMethodIndex(-1) { Q_ASSERT(engine); Q_ASSERT(metaObject); @@ -518,10 +518,16 @@ void QQmlPropertyCache::append(const QMetaObject *metaObject, for (int ii = 0; ii < classInfoCount; ++ii) { int idx = ii + classInfoOffset; - if (0 == qstrcmp(metaObject->classInfo(idx).name(), "qt_HasQmlAccessors")) { + const char * const classInfoName = metaObject->classInfo(idx).name(); + if (0 == qstrcmp(classInfoName, "qt_HasQmlAccessors")) { hasFastProperty = true; - } else if (0 == qstrcmp(metaObject->classInfo(idx).name(), "DefaultProperty")) { + } else if (0 == qstrcmp(classInfoName, "DefaultProperty")) { _defaultPropertyName = QString::fromUtf8(metaObject->classInfo(idx).value()); + } else if (0 == qstrcmp(classInfoName, "qt_QmlJSWrapperFactoryMethod")) { + const char * const factoryMethod = metaObject->classInfo(idx).value(); + _jsFactoryMethodIndex = metaObject->indexOfSlot(factoryMethod); + if (_jsFactoryMethodIndex != -1) + _jsFactoryMethodIndex -= metaObject->methodOffset(); } } diff --git a/src/qml/qml/qqmlpropertycache_p.h b/src/qml/qml/qqmlpropertycache_p.h index 610709ef7f..046e75ad3a 100644 --- a/src/qml/qml/qqmlpropertycache_p.h +++ b/src/qml/qml/qqmlpropertycache_p.h @@ -75,6 +75,8 @@ class QQmlPropertyCacheCreator; class QQmlPropertyRawData { public: + typedef QObjectPrivate::StaticMetaCallFunction StaticMetaCallFunction; + enum Flag { NoFlags = 0x00000000, ValueTypeFlagMask = 0x0000FFFF, // Flags in valueTypeFlags must fit in this mask @@ -322,6 +324,8 @@ public: void toMetaObjectBuilder(QMetaObjectBuilder &); + inline bool callJSFactoryMethod(QObject *object, void **args) const; + protected: virtual void destroy(); virtual void clear(); @@ -390,6 +394,7 @@ private: QByteArray _dynamicStringData; QString _defaultPropertyName; QQmlPropertyCacheMethodArguments *argumentsCache; + int _jsFactoryMethodIndex; }; // QQmlMetaObject serves as a wrapper around either QMetaObject or QQmlPropertyCache. @@ -538,6 +543,17 @@ int QQmlPropertyCache::signalOffset() const return signalHandlerIndexCacheStart; } +bool QQmlPropertyCache::callJSFactoryMethod(QObject *object, void **args) const +{ + if (_jsFactoryMethodIndex != -1) { + _metaObject->d.static_metacall(object, QMetaObject::InvokeMetaMethod, _jsFactoryMethodIndex, args); + return true; + } + if (_parent) + return _parent->callJSFactoryMethod(object, args); + return false; +} + QQmlMetaObject::QQmlMetaObject() { } diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index 1c7a556540..7acb8b5ebf 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -6991,15 +6991,6 @@ void QQuickItemPrivate::setHasCursorInChild(bool hasCursor) #endif } -void QQuickItemPrivate::markObjects(QV4::ExecutionEngine *e) -{ - Q_Q(QQuickItem); - QV4::QObjectWrapper::markWrapper(q, e); - - foreach (QQuickItem *child, childItems) - QQuickItemPrivate::get(child)->markObjects(e); -} - #ifndef QT_NO_CURSOR /*! @@ -8138,6 +8129,38 @@ QAccessible::Role QQuickItemPrivate::accessibleRole() const } #endif +// helper code to let a visual parent mark its visual children for the garbage collector + +namespace QV4 { +namespace Heap { +struct QQuickItemWrapper : public QObjectWrapper { + QQuickItemWrapper(QQuickItem *item) : QObjectWrapper(item) {} +}; +} +} + +struct QQuickItemWrapper : public QV4::QObjectWrapper { + V4_OBJECT2(QQuickItemWrapper, QV4::QObjectWrapper) + static void markObjects(QV4::Heap::Base *that, QV4::ExecutionEngine *e); +}; + +DEFINE_OBJECT_VTABLE(QQuickItemWrapper); + +void QQuickItemWrapper::markObjects(QV4::Heap::Base *that, QV4::ExecutionEngine *e) +{ + QObjectWrapper::Data *This = static_cast(that); + if (QQuickItem *item = static_cast(This->object.data())) { + foreach (QQuickItem *child, QQuickItemPrivate::get(item)->childItems) + QV4::QObjectWrapper::markWrapper(child, e); + } + QV4::QObjectWrapper::markObjects(that, e); +} + +quint64 QQuickItemPrivate::_q_createJSWrapper(QV4::ExecutionEngine *engine) +{ + return (engine->memoryManager->allocObject(q_func()))->asReturnedValue(); +} + QT_END_NAMESPACE #include diff --git a/src/quick/items/qquickitem.h b/src/quick/items/qquickitem.h index bf208e1f3e..b1b0172262 100644 --- a/src/quick/items/qquickitem.h +++ b/src/quick/items/qquickitem.h @@ -143,6 +143,7 @@ class Q_QUICK_EXPORT QQuickItem : public QObject, public QQmlParserStatus Q_CLASSINFO("DefaultProperty", "data") Q_CLASSINFO("qt_HasQmlAccessors", "true") + Q_CLASSINFO("qt_QmlJSWrapperFactoryMethod", "_q_createJSWrapper(QV4::ExecutionEngine*)") public: enum Flag { @@ -434,6 +435,7 @@ protected: private: Q_PRIVATE_SLOT(d_func(), void _q_resourceObjectDeleted(QObject *)) + Q_PRIVATE_SLOT(d_func(), quint64 _q_createJSWrapper(QV4::ExecutionEngine *)) friend class QQuickWindow; friend class QQuickWindowPrivate; diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h index c5d54a390b..7f9296396b 100644 --- a/src/quick/items/qquickitem_p.h +++ b/src/quick/items/qquickitem_p.h @@ -297,6 +297,7 @@ public: void _q_resourceObjectDeleted(QObject *); void _q_windowChanged(QQuickWindow *w); + quint64 _q_createJSWrapper(QV4::ExecutionEngine *engine); enum ChangeType { Geometry = 0x01, @@ -598,9 +599,6 @@ public: virtual void mirrorChange() {} void setHasCursorInChild(bool hasCursor); - - // recursive helper to let a visual parent mark its visual children - void markObjects(QV4::ExecutionEngine *e); }; /* diff --git a/src/quick/items/qquickview.cpp b/src/quick/items/qquickview.cpp index 867f7d9d15..0094432904 100644 --- a/src/quick/items/qquickview.cpp +++ b/src/quick/items/qquickview.cpp @@ -49,25 +49,6 @@ QT_BEGIN_NAMESPACE -DEFINE_OBJECT_VTABLE(QV4::QQuickRootItemMarker); - -QV4::Heap::QQuickRootItemMarker *QV4::QQuickRootItemMarker::create(QQmlEngine *engine, QQuickWindow *window) -{ - QV4::ExecutionEngine *e = QQmlEnginePrivate::getV4Engine(engine); - return e->memoryManager->allocObject(window); -} - -void QV4::QQuickRootItemMarker::markObjects(QV4::Heap::Base *that, QV4::ExecutionEngine *e) -{ - QQuickItem *root = static_cast(that)->window->contentItem(); - if (root) { - QQuickItemPrivate *rootPrivate = QQuickItemPrivate::get(root); - rootPrivate->markObjects(e); - } - - QV4::Object::markObjects(that, e); -} - void QQuickViewPrivate::init(QQmlEngine* e) { Q_Q(QQuickView); @@ -81,10 +62,10 @@ void QQuickViewPrivate::init(QQmlEngine* e) engine.data()->setIncubationController(q->incubationController()); { + // The content item has CppOwnership policy (set in QQuickWindow). Ensure the presence of a JS + // wrapper so that the garbage collector can see the policy. QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(engine.data()); - QV4::Scope scope(v4); - QV4::Scoped v(scope, QV4::QQuickRootItemMarker::create(engine.data(), q)); - rootItemMarker.set(v4, v); + QV4::QObjectWrapper::wrap(v4, contentItem); } QQmlInspectorService *service = QQmlDebugConnector::service(); diff --git a/src/quick/items/qquickview_p.h b/src/quick/items/qquickview_p.h index 71b39f5b0f..0a454a1463 100644 --- a/src/quick/items/qquickview_p.h +++ b/src/quick/items/qquickview_p.h @@ -102,37 +102,8 @@ public: QQuickView::ResizeMode resizeMode; QSize initialSize; QElapsedTimer frameTimer; - QV4::PersistentValue rootItemMarker; }; -namespace QV4 { -namespace Heap { - -struct QQuickRootItemMarker : Object { - inline QQuickRootItemMarker(QQuickWindow *window) - : window(window) - { - } - - QQuickWindow *window; -}; - -} - -struct QQuickRootItemMarker : public Object -{ - V4_OBJECT2(QQuickRootItemMarker, Object) - - static Heap::QQuickRootItemMarker *create(QQmlEngine *engine, QQuickWindow *window); - - static void markObjects(QV4::Heap::Base *that, QV4::ExecutionEngine *e); - -}; - - - -} - QT_END_NAMESPACE #endif // QQUICKVIEW_P_H diff --git a/src/quick/items/qquickwindowmodule.cpp b/src/quick/items/qquickwindowmodule.cpp index 5c66a2ef84..5278f25f21 100644 --- a/src/quick/items/qquickwindowmodule.cpp +++ b/src/quick/items/qquickwindowmodule.cpp @@ -100,12 +100,11 @@ void QQuickWindowQmlImpl::classBegin() if (e && !e->incubationController()) e->setIncubationController(incubationController()); } - Q_ASSERT(e); { + // The content item has CppOwnership policy (set in QQuickWindow). Ensure the presence of a JS + // wrapper so that the garbage collector can see the policy. QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(e); - QV4::Scope scope(v4); - QV4::ScopedObject v(scope, QV4::QQuickRootItemMarker::create(e, this)); - d->rootItemMarker = v; + QV4::QObjectWrapper::wrap(v4, d->contentItem); } } -- cgit v1.2.3 From 0d8a2831779ccb15984e2fe80be04e9856668434 Mon Sep 17 00:00:00 2001 From: Robin Burchell Date: Fri, 30 Sep 2016 10:37:28 +0200 Subject: qv4jsonobject: Make use of QVariant::toString in stringification This covers a whole host of missing cases, notably QUrl stored in a QV4::Value. Task-number: QTBUG-50592 Change-Id: I8afd772046c7bfbbcf916a7e90a57be5257b9df8 Reviewed-by: Simon Hausmann --- src/qml/jsruntime/qv4jsonobject.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/qml/jsruntime/qv4jsonobject.cpp b/src/qml/jsruntime/qv4jsonobject.cpp index 2e5283c639..fa3eeba745 100644 --- a/src/qml/jsruntime/qv4jsonobject.cpp +++ b/src/qml/jsruntime/qv4jsonobject.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include "qv4string_p.h" #include @@ -726,6 +727,10 @@ QString Stringify::Str(const QString &key, const Value &v) return std::isfinite(d) ? value->toQString() : QStringLiteral("null"); } + if (const QV4::VariantObject *v = value->as()) { + return v->d()->data.toString(); + } + o = value->asReturnedValue(); if (o) { if (!o->as()) { -- cgit v1.2.3