From 19ded50946fa40146914771b1d8eaa22f51da803 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Tue, 4 Jun 2013 10:05:51 +0200 Subject: [PATCH] Cleanups in QObject bindings * Rename v8object to jsWrapper in QQmlData * Rename v8objectid to jsEngineId, as that's the identifier of the engine that currently owns the primary JS wrapper This is in preparation for moving newObject away from QV8QObjectWrapper Change-Id: I6432365e849d159600e22f09e7e2ab2ae2117db6 Reviewed-by: Lars Knoll --- src/qml/qml/qqmldata_p.h | 6 +++--- src/qml/qml/qqmlengine.cpp | 2 +- src/qml/qml/qqmlvmemetaobject.cpp | 2 +- src/qml/qml/v4/qv4engine.cpp | 3 +++ src/qml/qml/v4/qv4engine_p.h | 2 ++ src/qml/qml/v8/qv8engine.cpp | 4 ++-- src/qml/qml/v8/qv8qobjectwrapper.cpp | 34 +++++++++++++++---------------- src/qml/qml/v8/qv8qobjectwrapper_p.h | 1 - tests/auto/qml/qqmlecmascript/testtypes.h | 4 ++-- 9 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h index 568a43e..7db822b 100644 --- a/src/qml/qml/qqmldata_p.h +++ b/src/qml/qml/qqmldata_p.h @@ -84,7 +84,7 @@ public: hasTaintedV8Object(false), isQueuedForDeletion(false), rootObjectInCreation(false), hasVMEMetaObject(false), parentFrozen(false), notifyList(0), context(0), outerContext(0), bindings(0), signalHandlers(0), nextContextObject(0), prevContextObject(0), bindingBitsSize(0), bindingBits(0), - lineNumber(0), columnNumber(0), compiledData(0), deferredIdx(0), v8objectid(0), + lineNumber(0), columnNumber(0), compiledData(0), deferredIdx(0), jsEngineId(0), propertyCache(0), guards(0), extendedData(0) { init(); } @@ -178,8 +178,8 @@ public: QQmlCompiledData *compiledData; unsigned int deferredIdx; - quint32 v8objectid; - QV4::WeakValue v8object; + quint32 jsEngineId; // id of the engine that cerated the jsWrapper + QV4::WeakValue jsWrapper; QQmlPropertyCache *propertyCache; diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index e8dd42c..26f3ea9 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -1531,7 +1531,7 @@ void QQmlData::destroyed(QObject *object) delete extendedData; // Dispose the handle. - v8object = QV4::Value::undefinedValue(); + jsWrapper = QV4::Value::undefinedValue(); if (ownMemory) delete this; diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp index 4a939f2..108fb79 100644 --- a/src/qml/qml/qqmlvmemetaobject.cpp +++ b/src/qml/qml/qqmlvmemetaobject.cpp @@ -1224,7 +1224,7 @@ void QQmlVMEMetaObject::mark() if (ref) { QQmlData *ddata = QQmlData::get(ref); if (ddata) - ddata->v8object.markOnce(); + ddata->jsWrapper.markOnce(); } } } diff --git a/src/qml/qml/v4/qv4engine.cpp b/src/qml/qml/v4/qv4engine.cpp index a8fd2c6..fbe55d1 100644 --- a/src/qml/qml/v4/qv4engine.cpp +++ b/src/qml/qml/v4/qv4engine.cpp @@ -80,6 +80,8 @@ using namespace QV4; +static QBasicAtomicInt engineSerial = Q_BASIC_ATOMIC_INITIALIZER(1); + ExecutionEngine::ExecutionEngine(QQmlJS::EvalISelFactory *factory) : memoryManager(new QV4::MemoryManager) , executableAllocator(new QV4::ExecutableAllocator) @@ -89,6 +91,7 @@ ExecutionEngine::ExecutionEngine(QQmlJS::EvalISelFactory *factory) , globalObject(0) , globalCode(0) , functionsNeedSort(false) + , m_engineId(engineSerial.fetchAndAddOrdered(1)) , regExpCache(0) { MemoryManager::GCBlocker gcBlocker(memoryManager); diff --git a/src/qml/qml/v4/qv4engine_p.h b/src/qml/qml/v4/qv4engine_p.h index 2af7a17..6db8ebc 100644 --- a/src/qml/qml/v4/qv4engine_p.h +++ b/src/qml/qml/v4/qv4engine_p.h @@ -203,6 +203,8 @@ struct Q_QML_EXPORT ExecutionEngine mutable QVector functions; mutable bool functionsNeedSort; + quint32 m_engineId; + RegExpCache *regExpCache; // Scarce resources are "exceptionally high cost" QVariant types where allowing the diff --git a/src/qml/qml/v8/qv8engine.cpp b/src/qml/qml/v8/qv8engine.cpp index 3af2efb..094a38d 100644 --- a/src/qml/qml/v8/qv8engine.cpp +++ b/src/qml/qml/v8/qv8engine.cpp @@ -505,7 +505,7 @@ QV4::WeakValue *QV8Engine::findOwnerAndStrength(QObject *object, bool *shouldBeS // if the object has JS ownership, the object's v8object owns the lifetime of the persistent value. if (QQmlEngine::objectOwnership(object) == QQmlEngine::JavaScriptOwnership) { *shouldBeStrong = false; - return &(QQmlData::get(object)->v8object); + return &(QQmlData::get(object)->jsWrapper); } // no parent, and has CPP ownership - doesn't have an implicit parent. @@ -521,7 +521,7 @@ QV4::WeakValue *QV8Engine::findOwnerAndStrength(QObject *object, bool *shouldBeS if (QQmlEngine::objectOwnership(parent) == QQmlEngine::JavaScriptOwnership) { // root parent is owned by JS. It's v8object owns the persistent value in question. *shouldBeStrong = false; - return &(QQmlData::get(parent)->v8object); + return &(QQmlData::get(parent)->jsWrapper); } else { // root parent has CPP ownership. The persistent value should not be made weak. *shouldBeStrong = true; diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp index 15ae806..9c76ba0 100644 --- a/src/qml/qml/v8/qv8qobjectwrapper.cpp +++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp @@ -276,7 +276,7 @@ public: delete this; } - QV4::WeakValue v8object; + QV4::WeakValue jsWrapper; QV8QObjectWrapper *wrapper; }; @@ -341,10 +341,8 @@ private: }; } -static QAtomicInt objectIdCounter(1); - QV8QObjectWrapper::QV8QObjectWrapper() -: m_engine(0), m_id(objectIdCounter.fetchAndAddOrdered(1)) +: m_engine(0) { } @@ -818,17 +816,19 @@ v8::Handle QV8QObjectWrapper::newQObject(QObject *object) if (!ddata) return QV4::Value::undefinedValue(); - if (ddata->v8objectid == m_id && !ddata->v8object.isEmpty()) { + QV4::ExecutionEngine *v4 = QV8Engine::getV4(m_engine); + + if (ddata->jsEngineId == v4->m_engineId && !ddata->jsWrapper.isEmpty()) { // We own the v8object - return ddata->v8object.value(); - } else if (ddata->v8object.isEmpty() && - (ddata->v8objectid == m_id || // We own the QObject - ddata->v8objectid == 0 || // No one owns the QObject + return ddata->jsWrapper.value(); + } else if (ddata->jsWrapper.isEmpty() && + (ddata->jsEngineId == v4->m_engineId || // We own the QObject + ddata->jsEngineId == 0 || // No one owns the QObject !ddata->hasTaintedV8Object)) { // Someone else has used the QObject, but it isn't tainted v8::Handle rv = newQObject(object, ddata, m_engine); - ddata->v8object = rv->v4Value(); - ddata->v8objectid = m_id; + ddata->jsWrapper = rv->v4Value(); + ddata->jsEngineId = v4->m_engineId; return rv; } else { @@ -840,10 +840,10 @@ v8::Handle QV8QObjectWrapper::newQObject(QObject *object) // If our tainted handle doesn't exist or has been collected, and there isn't // a handle in the ddata, we can assume ownership of the ddata->v8object - if ((!found || (*iter)->v8object.isEmpty()) && ddata->v8object.isEmpty()) { + if ((!found || (*iter)->jsWrapper.isEmpty()) && ddata->jsWrapper.isEmpty()) { v8::Handle rv = newQObject(object, ddata, m_engine); - ddata->v8object = rv->v4Value(); - ddata->v8objectid = m_id; + ddata->jsWrapper = rv->v4Value(); + ddata->jsEngineId = v4->m_engineId; if (found) { delete (*iter); @@ -857,12 +857,12 @@ v8::Handle QV8QObjectWrapper::newQObject(QObject *object) ddata->hasTaintedV8Object = true; } - if ((*iter)->v8object.isEmpty()) { + if ((*iter)->jsWrapper.isEmpty()) { v8::Handle rv = newQObject(object, ddata, m_engine); - (*iter)->v8object = rv->v4Value(); + (*iter)->jsWrapper = rv->v4Value(); } - return (*iter)->v8object.value(); + return (*iter)->jsWrapper.value(); } } diff --git a/src/qml/qml/v8/qv8qobjectwrapper_p.h b/src/qml/qml/v8/qv8qobjectwrapper_p.h index 4262dd8..5862a18 100644 --- a/src/qml/qml/v8/qv8qobjectwrapper_p.h +++ b/src/qml/qml/v8/qv8qobjectwrapper_p.h @@ -196,7 +196,6 @@ private: static QPair ExtractQtSignal(QV8Engine *, const QV4::Value &value); QV8Engine *m_engine; - quint32 m_id; QHash m_connections; typedef QHash TaintedHash; TaintedHash m_taintedObjects; diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h index 3752dd8..7205155 100644 --- a/tests/auto/qml/qqmlecmascript/testtypes.h +++ b/tests/auto/qml/qqmlecmascript/testtypes.h @@ -1205,13 +1205,13 @@ public: { QQmlData *ddata = QQmlData::get(this); assert(ddata); - QV4::QObjectWrapper *thisObject = ddata->v8object.value().as(); + QV4::QObjectWrapper *thisObject = ddata->jsWrapper.value().as(); assert(thisObject); QQmlData *otherDData = QQmlData::get(other); assert(otherDData); - thisObject->defineDefaultProperty(thisObject->engine(), QStringLiteral("autoTestStrongRef"), otherDData->v8object.value()); + thisObject->defineDefaultProperty(thisObject->engine(), QStringLiteral("autoTestStrongRef"), otherDData->jsWrapper.value()); } void setEngine(QQmlEngine* declarativeEngine) -- 2.7.4