Move handling of multiply wrapped QObjects from QV8QObjectWrapper into QV4::Execution...

The bookkeeping is annoying to do, but it should be a rare case. The common case of a
QObject being wrapped only once is still lightweight as-is.

Change-Id: I12e9b67270ca2afbd77b4395246eef0dfc324f8f
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/src/qml/qml/v4/qv4engine.cpp b/src/qml/qml/v4/qv4engine.cpp
index fbe55d1..c96d60a 100644 (file)
--- a/src/qml/qml/v4/qv4engine.cpp
+++ b/src/qml/qml/v4/qv4engine.cpp
@@ -63,6 +63,7 @@
 #include "qv4debugging_p.h"
 #include "qv4executableallocator_p.h"
 #include "qv4sequenceobject_p.h"
+#include <private/qv8qobjectwrapper_p.h>
 
 #if defined(Q_OS_LINUX) || defined(Q_OS_MAC)
 #include <execinfo.h>
@@ -93,6 +94,7 @@ ExecutionEngine::ExecutionEngine(QQmlJS::EvalISelFactory *factory)
     , functionsNeedSort(false)
     , m_engineId(engineSerial.fetchAndAddOrdered(1))
     , regExpCache(0)
+    , m_multiplyWrappedQObjects(0)
 {
     MemoryManager::GCBlocker gcBlocker(memoryManager);
 
@@ -271,6 +273,8 @@ ExecutionEngine::ExecutionEngine(QQmlJS::EvalISelFactory *factory)
 
 ExecutionEngine::~ExecutionEngine()
 {
+    delete m_multiplyWrappedQObjects;
+    m_multiplyWrappedQObjects = 0;
     delete memoryManager;
     emptyClass->destroy();
     delete identifierCache;

diff --git a/src/qml/qml/v4/qv4engine_p.h b/src/qml/qml/v4/qv4engine_p.h
index 6db8ebc..60e3eee 100644 (file)
--- a/src/qml/qml/v4/qv4engine_p.h
+++ b/src/qml/qml/v4/qv4engine_p.h
@@ -103,7 +103,7 @@ struct SequencePrototype;
 struct EvalFunction;
 struct Identifiers;
 struct InternalClass;
-
+class MultiplyWrappedQObjectMap;
 class RegExp;
 class RegExpCache;
 
@@ -221,6 +221,11 @@ struct Q_QML_EXPORT ExecutionEngine
     };
     QIntrusiveList<ScarceResourceData, &ScarceResourceData::node> scarceResources;
 
+    // Normally the JS wrappers for QObjects are stored in the QQmlData/QObjectPrivate,
+    // but any time a QObject is wrapped a second time in another engine, we have to do
+    // bookkeeping.
+    MultiplyWrappedQObjectMap *m_multiplyWrappedQObjects;
+
     ExecutionEngine(QQmlJS::EvalISelFactory *iselFactory = 0);
     ~ExecutionEngine();
 

diff --git a/src/qml/qml/v4/qv4mm.cpp b/src/qml/qml/v4/qv4mm.cpp
index 4afef68..e278637 100644 (file)
--- a/src/qml/qml/v4/qv4mm.cpp
+++ b/src/qml/qml/v4/qv4mm.cpp
@@ -342,6 +342,15 @@ std::size_t MemoryManager::sweep()
         weak = weak->next;
     }
 
+    if (MultiplyWrappedQObjectMap *multiplyWrappedQObjects = m_d->engine->m_multiplyWrappedQObjects) {
+        for (MultiplyWrappedQObjectMap::Iterator it = multiplyWrappedQObjects->begin(); it != multiplyWrappedQObjects->end();) {
+            if (!it.value()->markBit)
+                it = multiplyWrappedQObjects->erase(it);
+            else
+                ++it;
+        }
+    }
+
     std::size_t freedCount = 0;
 
     for (QVector<Data::Chunk>::iterator i = m_d->heapChunks.begin(), ei = m_d->heapChunks.end(); i != ei; ++i)

diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp
index 90c903b..091a952 100644 (file)
--- a/src/qml/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp
@@ -268,29 +268,6 @@ DEFINE_MANAGED_VTABLE(QObjectWrapper);
 // XXX TODO: Need to review all calls to QQmlEngine *engine() to confirm QObjects work
 // correctly in a worker thread
 
-class QV8QObjectInstance : public QQmlGuard<QObject>
-{
-public:
-    QV8QObjectInstance(QObject *o, QV8QObjectWrapper *w)
-    : QQmlGuard<QObject>(o), wrapper(w)
-    {
-    }
-
-    ~QV8QObjectInstance()
-    {
-    }
-
-    virtual void objectDestroyed(QObject *o)
-    {
-        if (wrapper)
-            wrapper->m_taintedObjects.remove(o);
-        delete this;
-    }
-
-    QV4::WeakValue jsWrapper;
-    QV8QObjectWrapper *wrapper;
-};
-
 namespace {
 
 template<typename A, typename B, typename C, typename D, typename E,
@@ -359,12 +336,6 @@ QV8QObjectWrapper::QV8QObjectWrapper()
 
 QV8QObjectWrapper::~QV8QObjectWrapper()
 {
-    for (TaintedHash::Iterator iter = m_taintedObjects.begin(); 
-         iter != m_taintedObjects.end();
-         ++iter) {
-        (*iter)->wrapper = 0;
-    }
-    m_taintedObjects.clear();
 }
 
 void QV8QObjectWrapper::destroy()
@@ -833,34 +804,28 @@ v8::Handle<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
     } else {
         // 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();
+        Object *alternateWrapper = 0;
+        if (v4->m_multiplyWrappedQObjects && ddata->hasTaintedV8Object)
+            alternateWrapper = v4->m_multiplyWrappedQObjects->value(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)->jsWrapper.isEmpty()) && ddata->jsWrapper.isEmpty()) {
-            QV4::Value rv = QV4::QObjectWrapper::wrap(v4, ddata, object);
-            ddata->jsWrapper = rv;
+        if (ddata->jsWrapper.isEmpty() && !alternateWrapper) {
+            QV4::Value result = QV4::QObjectWrapper::wrap(v4, ddata, object);
+            ddata->jsWrapper = result;
             ddata->jsEngineId = v4->m_engineId;
-
-            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;
+            return result;
         }
 
-        if ((*iter)->jsWrapper.isEmpty()) {
-            (*iter)->jsWrapper = QV4::QObjectWrapper::wrap(v4, ddata, object);
+        if (!alternateWrapper) {
+            alternateWrapper = QV4::QObjectWrapper::wrap(v4, ddata, object).asObject();
+            if (!v4->m_multiplyWrappedQObjects)
+                v4->m_multiplyWrappedQObjects = new MultiplyWrappedQObjectMap;
+            v4->m_multiplyWrappedQObjects->insert(object, alternateWrapper);
+            ddata->hasTaintedV8Object = true;
         }
 
-        return (*iter)->jsWrapper.value();
+        return QV4::Value::fromObject(alternateWrapper);
     }
 }
 
@@ -1957,5 +1922,30 @@ QmlSignalHandler::QmlSignalHandler(ExecutionEngine *engine, QObject *object, int
 
 DEFINE_MANAGED_VTABLE(QmlSignalHandler);
 
+void MultiplyWrappedQObjectMap::insert(QObject *key, Object *value)
+{
+    QHash<QObject*, Object*>::insert(key, value);
+    connect(key, SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*)));
+}
+
+MultiplyWrappedQObjectMap::Iterator MultiplyWrappedQObjectMap::erase(MultiplyWrappedQObjectMap::Iterator it)
+{
+    disconnect(it.key(), SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*)));
+    return QHash<QObject*, Object*>::erase(it);
+}
+
+void MultiplyWrappedQObjectMap::remove(QObject *key)
+{
+    Iterator it = find(key);
+    if (it == end())
+        return;
+    erase(it);
+}
+
+void MultiplyWrappedQObjectMap::removeDestroyedObject(QObject *object)
+{
+    QHash<QObject*, Object*>::remove(object);
+}
+
 QT_END_NAMESPACE
 

diff --git a/src/qml/qml/v8/qv8qobjectwrapper_p.h b/src/qml/qml/v8/qv8qobjectwrapper_p.h
index 78db659..72bf750 100644 (file)
--- a/src/qml/qml/v8/qv8qobjectwrapper_p.h
+++ b/src/qml/qml/v8/qv8qobjectwrapper_p.h
@@ -73,7 +73,6 @@ class QObject;
 class QV8Engine;
 class QQmlData;
 class QV8ObjectResource;
-class QV8QObjectInstance;
 class QV8QObjectConnectionList;
 class QQmlPropertyCache;
 
@@ -167,6 +166,28 @@ private:
     }
 };
 
+class MultiplyWrappedQObjectMap : public QObject,
+                                  private QHash<QObject*, Object*>
+{
+    Q_OBJECT
+public:
+    typedef QHash<QObject*, Object*>::ConstIterator ConstIterator;
+    typedef QHash<QObject*, Object*>::Iterator Iterator;
+
+    ConstIterator begin() const { return QHash<QObject*, Object*>::constBegin(); }
+    Iterator begin() { return QHash<QObject*, Object*>::begin(); }
+    ConstIterator end() const { return QHash<QObject*, Object*>::constEnd(); }
+    Iterator end() { return QHash<QObject*, Object*>::end(); }
+
+    void insert(QObject *key, Object *value);
+    Object *value(QObject *key) const { return QHash<QObject*, Object*>::value(key, 0); }
+    Iterator erase(Iterator it);
+    void remove(QObject *key);
+
+private slots:
+    void removeDestroyedObject(QObject*);
+};
+
 }
 
 class Q_QML_PRIVATE_EXPORT QV8QObjectWrapper
@@ -186,7 +207,6 @@ public:
 private:
     friend class QQmlPropertyCache;
     friend class QV8QObjectConnectionList;
-    friend class QV8QObjectInstance;
     friend struct QV4::QObjectWrapper;
 
     static QV4::Value GetProperty(QV8Engine *, QObject *,
@@ -200,8 +220,6 @@ private:
 
     QV8Engine *m_engine;
     QHash<QObject *, QV8QObjectConnectionList *> m_connections;
-    typedef QHash<QObject *, QV8QObjectInstance *> TaintedHash;
-    TaintedHash m_taintedObjects;
 };
 
 v8::Handle<v8::Value> QV8QObjectWrapper::getProperty(QObject *object, const QHashedV4String &string,