Allow the same QObject to be used in multiple QDeclarativeEngines

diff --git a/src/declarative/qml/qdeclarativedata_p.h b/src/declarative/qml/qdeclarativedata_p.h
index ea8f423..65f4926 100644 (file)
--- a/src/declarative/qml/qdeclarativedata_p.h
+++ b/src/declarative/qml/qdeclarativedata_p.h
@@ -77,9 +77,10 @@ class Q_AUTOTEST_EXPORT QDeclarativeData : public QAbstractDeclarativeData
 public:
     QDeclarativeData()
         : ownMemory(true), ownContext(false), indestructible(true), explicitIndestructibleSet(false), 
-          context(0), outerContext(0), bindings(0), nextContextObject(0), prevContextObject(0), bindingBitsSize(0), 
-          bindingBits(0), lineNumber(0), columnNumber(0), deferredComponent(0), deferredIdx(0), propertyCache(0), 
-          guards(0), extendedData(0) {
+          hasTaintedV8Object(false), context(0), outerContext(0), bindings(0), nextContextObject(0), 
+          prevContextObject(0), bindingBitsSize(0), bindingBits(0), lineNumber(0), columnNumber(0), 
+          deferredComponent(0), deferredIdx(0), v8objectid(0), propertyCache(0), guards(0), 
+          extendedData(0) {
           init(); 
       }
 
@@ -105,7 +106,8 @@ public:
     quint32 ownContext:1;
     quint32 indestructible:1;
     quint32 explicitIndestructibleSet:1;
-    quint32 dummy:28;
+    quint32 hasTaintedV8Object:1;
+    quint32 dummy:27;
 
     // The context that created the C++ object
     QDeclarativeContextData *context; 
@@ -130,7 +132,9 @@ public:
     QDeclarativeCompiledData *deferredComponent; // Can't this be found from the context?
     unsigned int deferredIdx;
 
+    quint32 v8objectid;
     v8::Persistent<v8::Object> v8object;
+
     QDeclarativePropertyCache *propertyCache;
 
     QDeclarativeGuard<QObject> *guards;

diff --git a/src/declarative/qml/qdeclarativeengine.cpp b/src/declarative/qml/qdeclarativeengine.cpp
index f3c4204..fa606ab 100644 (file)
--- a/src/declarative/qml/qdeclarativeengine.cpp
+++ b/src/declarative/qml/qdeclarativeengine.cpp
@@ -1080,7 +1080,7 @@ void QDeclarativeData::destroyed(QObject *object)
     if (extendedData)
         delete extendedData;
 
-    v8object.Dispose();
+    v8object.Clear(); // The WeakReference handler will clean the actual handle
 
     if (ownMemory)
         delete this;

diff --git a/src/declarative/qml/v8/qv8qobjectwrapper.cpp b/src/declarative/qml/v8/qv8qobjectwrapper.cpp
index 99cede2..2647197 100644 (file)
--- a/src/declarative/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/declarative/qml/v8/qv8qobjectwrapper.cpp
@@ -52,6 +52,7 @@
 #include <QtScript/qscriptvalue.h>
 #include <QtCore/qvarlengtharray.h>
 #include <QtCore/qtimer.h>
+#include <QtCore/qatomic.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -79,6 +80,31 @@ public:
     QDeclarativeGuard<QObject> object;
 };
 
+class QV8QObjectInstance : public QDeclarativeGuard<QObject>
+{
+public:
+    QV8QObjectInstance(QObject *o, QV8QObjectWrapper *w)
+    : QDeclarativeGuard<QObject>(o), wrapper(w)
+    {
+    }
+
+    ~QV8QObjectInstance()
+    {
+        v8object.Dispose();
+        v8object.Clear();
+    }
+
+    virtual void objectDestroyed(QObject *o)
+    {
+        if (wrapper)
+            wrapper->m_taintedObjects.remove(o);
+        delete this;
+    }
+
+    v8::Persistent<v8::Object> v8object;
+    QV8QObjectWrapper *wrapper;
+};
+
 namespace {
 struct MetaCallArgument {
     inline MetaCallArgument();
@@ -105,13 +131,21 @@ QV8QObjectResource::QV8QObjectResource(QV8Engine *engine, QObject *object)
 {
 }
 
+static QAtomicInt objectIdCounter(1);
+
 QV8QObjectWrapper::QV8QObjectWrapper()
-: m_engine(0)
+: m_engine(0), m_id(objectIdCounter.fetchAndAddOrdered(1))
 {
 }
 
 QV8QObjectWrapper::~QV8QObjectWrapper()
 {
+    for (TaintedHash::Iterator iter = m_taintedObjects.begin(); 
+         iter != m_taintedObjects.end();
+         ++iter) {
+        (*iter)->wrapper = 0;
+    }
+    m_taintedObjects.clear();
 }
 
 void QV8QObjectWrapper::destroy()
@@ -619,8 +653,13 @@ static void WeakQObjectReferenceCallback(v8::Persistent<v8::Value> handle, void
         }
     }
 
-    // XXX do we want to use the objectDataRefCount to support multiple concurrent engines?
+    handle.Dispose();
+}
 
+static void WeakQObjectInstanceCallback(v8::Persistent<v8::Value> handle, void *data)
+{
+    QV8QObjectInstance *instance = (QV8QObjectInstance *)data;
+    instance->v8object.Clear();
     handle.Dispose();
 }
 
@@ -628,10 +667,8 @@ v8::Local<v8::Object> QDeclarativePropertyCache::newQObject(QObject *object, QV8
 {
     Q_ASSERT(object);
 
-    QDeclarativeData *ddata = QDeclarativeData::get(object, false);
-
-    Q_ASSERT(ddata && ddata->propertyCache && ddata->propertyCache == this);
-    Q_ASSERT(ddata->v8object.IsEmpty());
+    Q_ASSERT(QDeclarativeData::get(object, false));
+    Q_ASSERT(QDeclarativeData::get(object, false)->propertyCache == this);
 
     // Setup constructor
     if (constructor.IsEmpty()) {
@@ -709,16 +746,42 @@ v8::Local<v8::Object> QDeclarativePropertyCache::newQObject(QObject *object, QV8
     v8::Local<v8::Object> result = constructor->NewInstance();
     QV8QObjectResource *r = new QV8QObjectResource(engine, object);
     result->SetExternalResource(r);
-
-    ddata->v8object = v8::Persistent<v8::Object>::New(result);
-    ddata->v8object.MakeWeak(0, WeakQObjectReferenceCallback);
     return result;
 }
 
-v8::Handle<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
+v8::Local<v8::Object> QV8QObjectWrapper::newQObject(QObject *object, QDeclarativeData *ddata, QV8Engine *engine)
 {
-    // XXX aakenned QDeclarativeObjectScriptClass::newQObject()  does a lot more
+    v8::Local<v8::Object> rv;
 
+    if (ddata->propertyCache) {
+        rv = ddata->propertyCache->newQObject(object, engine);
+    } else {
+        // XXX aakenned - NewInstance() is slow for our case
+        rv = m_constructor->NewInstance(); 
+        QV8QObjectResource *r = new QV8QObjectResource(engine, object);
+        rv->SetExternalResource(r);
+    }
+
+    return rv;
+}
+
+/*
+As V8 doesn't support an equality callback, for QObject's we have to return exactly the same
+V8 handle for subsequent calls to newQObject for the same QObject.  To do this we have a two
+pronged strategy:
+   1. If there is no current outstanding V8 handle to the QObject, we create one and store a 
+      persistent handle in QDeclarativeData::v8object.  We mark the QV8QObjectWrapper that 
+      "owns" this handle by setting the QDeclarativeData::v8objectid to the id of this 
+      QV8QObjectWrapper.
+   2. If another QV8QObjectWrapper has create the handle in QDeclarativeData::v8object we create 
+      an entry in the m_taintedObject hash where we store the handle and mark the object as 
+      "tainted" in the QDeclarativeData::hasTaintedV8Object flag.
+We have to mark the object as tainted to ensure that we search our m_taintedObject hash even
+in the case that the original QV8QObjectWrapper owner of QDeclarativeData::v8object has 
+released the handle.
+*/
+v8::Handle<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
+{
     if (!object)
         return v8::Null();
 
@@ -730,23 +793,56 @@ v8::Handle<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
     if (!ddata) 
         return v8::Undefined();
 
-    if (ddata->v8object.IsEmpty()) {
-
-        if (ddata->propertyCache) {
-            return ddata->propertyCache->newQObject(object, m_engine);
-        }
+    if (ddata->v8objectid == m_id && !ddata->v8object.IsEmpty()) {
+        // We own the v8object 
+        return v8::Local<v8::Object>::New(ddata->v8object);
+    } else if (ddata->v8object.IsEmpty() && 
+               (ddata->v8objectid == m_id || // We own the QObject
+                ddata->v8objectid == 0 ||    // No one owns the QObject
+                !ddata->hasTaintedV8Object)) { // Someone else has used the QObject, but it isn't tainted
 
-        // XXX aakenned - NewInstance() is slow for our case
-        v8::Local<v8::Object> rv = m_constructor->NewInstance(); 
-        QV8QObjectResource *r = new QV8QObjectResource(m_engine, object);
-        rv->SetExternalResource(r);
+        v8::Local<v8::Object> rv = newQObject(object, ddata, m_engine);
         ddata->v8object = v8::Persistent<v8::Object>::New(rv);
-    }
+        ddata->v8object.MakeWeak(0, WeakQObjectReferenceCallback);
+        ddata->v8objectid = m_id;
+        return rv;
+
+    } else {
 
-    // XXX do we have to check that the v8object isn't "owned" by another engine?
+        // If this object is tainted, we have to check to see if it is in our
+        // tainted object list
+        TaintedHash::Iterator iter =
+            ddata->hasTaintedV8Object?m_taintedObjects.find(object):m_taintedObjects.end();
+        bool found = iter != m_taintedObjects.end();
+
+        // 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()) {
+            v8::Local<v8::Object> rv = newQObject(object, ddata, m_engine);
+            ddata->v8object = v8::Persistent<v8::Object>::New(rv);
+            ddata->v8object.MakeWeak(0, WeakQObjectReferenceCallback);
+            ddata->v8objectid = m_id;
+
+            if (found) {
+                delete (*iter);
+                m_taintedObjects.erase(iter);
+            }
+
+            return rv;
+        } else if (!found) {
+            QV8QObjectInstance *instance = new QV8QObjectInstance(object, this);
+            iter = m_taintedObjects.insert(object, instance);
+            ddata->hasTaintedV8Object = true;
+        }
 
-    ddata->v8object.MakeWeak(0, WeakQObjectReferenceCallback);
-    return v8::Local<v8::Object>::New(ddata->v8object);
+        if ((*iter)->v8object.IsEmpty()) {
+            v8::Local<v8::Object> rv = newQObject(object, ddata, m_engine);
+            (*iter)->v8object = v8::Persistent<v8::Object>::New(rv);
+            (*iter)->v8object.MakeWeak((*iter), WeakQObjectInstanceCallback);
+        }
+
+        return v8::Local<v8::Object>::New((*iter)->v8object);
+    }
 }
 
 QPair<QObject *, int> QV8QObjectWrapper::ExtractQtMethod(QV8Engine *engine, v8::Handle<v8::Function> function)

diff --git a/src/declarative/qml/v8/qv8qobjectwrapper_p.h b/src/declarative/qml/v8/qv8qobjectwrapper_p.h
index f633969..5100c3f 100644 (file)
--- a/src/declarative/qml/v8/qv8qobjectwrapper_p.h
+++ b/src/declarative/qml/v8/qv8qobjectwrapper_p.h
@@ -63,9 +63,11 @@ QT_BEGIN_NAMESPACE
 
 class QObject;
 class QV8Engine;
+class QDeclarativeData;
 class QV8ObjectResource;
-class QDeclarativePropertyCache;
+class QV8QObjectInstance;
 class QV8QObjectConnectionList;
+class QDeclarativePropertyCache;
 class Q_AUTOTEST_EXPORT QV8QObjectWrapper 
 {
 public:
@@ -87,7 +89,9 @@ public:
 private:
     friend class QDeclarativePropertyCache;
     friend class QV8QObjectConnectionList;
+    friend class QV8QObjectInstance;
 
+    v8::Local<v8::Object> newQObject(QObject *, QDeclarativeData *, QV8Engine *);
     static v8::Handle<v8::Value> GetProperty(QV8Engine *, QObject *, v8::Handle<v8::Value> *, 
                                              v8::Handle<v8::String>, QV8QObjectWrapper::RevisionMode);
     static bool SetProperty(QV8Engine *, QObject *, v8::Handle<v8::String>,
@@ -106,12 +110,15 @@ private:
     static QPair<QObject *, int> ExtractQtMethod(QV8Engine *, v8::Handle<v8::Function>);
 
     QV8Engine *m_engine;
+    quint32 m_id;
     v8::Persistent<v8::Function> m_constructor;
     v8::Persistent<v8::Function> m_methodConstructor;
     v8::Persistent<v8::String> m_toStringSymbol;
     v8::Persistent<v8::String> m_destroySymbol;
     v8::Persistent<v8::Object> m_hiddenObject;
     QHash<QObject *, QV8QObjectConnectionList *> m_connections;
+    typedef QHash<QObject *, QV8QObjectInstance *> TaintedHash;
+    TaintedHash m_taintedObjects;
 };
 
 QT_END_NAMESPACE