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 <lars.knoll@digia.com>

diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index 568a43e..7db822b 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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<Function *> 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 (file)
--- 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 (file)
--- 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<v8::Value> 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<v8::Object> 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<v8::Value> 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<v8::Object> 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<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
             ddata->hasTaintedV8Object = true;
         }
 
-        if ((*iter)->v8object.isEmpty()) {
+        if ((*iter)->jsWrapper.isEmpty()) {
             v8::Handle<v8::Object> 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 (file)
--- a/src/qml/qml/v8/qv8qobjectwrapper_p.h
+++ b/src/qml/qml/v8/qv8qobjectwrapper_p.h
@@ -196,7 +196,6 @@ private:
     static QPair<QObject *, int> ExtractQtSignal(QV8Engine *, const QV4::Value &value);
 
     QV8Engine *m_engine;
-    quint32 m_id;
     QHash<QObject *, QV8QObjectConnectionList *> m_connections;
     typedef QHash<QObject *, QV8QObjectInstance *> TaintedHash;
     TaintedHash m_taintedObjects;

diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h
index 3752dd8..7205155 100644 (file)
--- 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>();
+        QV4::QObjectWrapper *thisObject = ddata->jsWrapper.value().as<QV4::QObjectWrapper>();
         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)