V4: Avoid copying WeakValues with wrapped QObjects

Such WeakValues are kept alive until the respective QObject is deleted.
Therefore they are quite expensive. In this case we don't actually need
a copy as on retrieval we only want a ReturnValue and on inserting we
just want to replace the value in the map.

Fixes: QTBUG-71817
Change-Id: I385c55140337d468289046243941077ba1ff61a3
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>

3 files changed, 49 insertions, 4 deletions

diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 8cdec2f6ee..9344a231ff 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -2226,9 +2226,7 @@ void QmlSignalHandler::initProto(ExecutionEngine *engine)
 
 void MultiplyWrappedQObjectMap::insert(QObject *key, Heap::Object *value)
 {
-    QV4::WeakValue v;
-    v.set(value->internalClass->engine, value);
-    QHash<QObject*, QV4::WeakValue>::insert(key, v);
+    QHash<QObject*, QV4::WeakValue>::operator[](key).set(value->internalClass->engine, value);
     connect(key, SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*)));
 }
 
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index be46245d5a..6465ee0fa6 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -290,7 +290,14 @@ public:
     Iterator end() { return QHash<QObject*, QV4::WeakValue>::end(); }
 
     void insert(QObject *key, Heap::Object *value);
-    ReturnedValue value(QObject *key) const { return QHash<QObject*, QV4::WeakValue>::value(key).value(); }
+    ReturnedValue value(QObject *key) const
+    {
+        ConstIterator it = find(key);
+        return it == end()
+                ? QV4::WeakValue().value()
+                : it->value();
+    }
+
     Iterator erase(Iterator it);
     void remove(QObject *key);
     void mark(QObject *key, MarkStack *markStack);
diff --git a/tests/auto/qml/qv4mm/tst_qv4mm.cpp b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
index 07f8e9f1d1..d8f4ed12e8 100644
--- a/tests/auto/qml/qv4mm/tst_qv4mm.cpp
+++ b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
@@ -30,6 +30,7 @@
 #include <QQmlEngine>
 #include <QLoggingCategory>
 #include <private/qv4mm_p.h>
+#include <private/qv4qobjectwrapper_p.h>
 
 class tst_qv4mm : public QObject
 {
@@ -37,6 +38,7 @@ class tst_qv4mm : public QObject
 
 private slots:
     void gcStats();
+    void multiWrappedQObjects();
 };
 
 void tst_qv4mm::gcStats()
@@ -46,6 +48,44 @@ void tst_qv4mm::gcStats()
     engine.collectGarbage();
 }
 
+void tst_qv4mm::multiWrappedQObjects()
+{
+    QV4::ExecutionEngine engine1;
+    QV4::ExecutionEngine engine2;
+    {
+        QObject object;
+        for (int i = 0; i < 10; ++i)
+            QV4::QObjectWrapper::wrap(i % 2 ? &engine1 : &engine2, &object);
+
+        QCOMPARE(engine1.memoryManager->m_pendingFreedObjectWrapperValue.size(), 0);
+        QCOMPARE(engine2.memoryManager->m_pendingFreedObjectWrapperValue.size(), 0);
+        {
+            QV4::WeakValue value;
+            value.set(&engine1, QV4::QObjectWrapper::wrap(&engine1, &object));
+        }
+
+        QCOMPARE(engine1.memoryManager->m_pendingFreedObjectWrapperValue.size(), 1);
+        QCOMPARE(engine2.memoryManager->m_pendingFreedObjectWrapperValue.size(), 0);
+
+        // Moves the additional WeakValue from m_multiplyWrappedQObjects to
+        // m_pendingFreedObjectWrapperValue. It's still alive after all.
+        engine1.memoryManager->runGC();
+        QCOMPARE(engine1.memoryManager->m_pendingFreedObjectWrapperValue.size(), 2);
+
+        // engine2 doesn't own the object as engine1 was the first to wrap it above.
+        // Therefore, no effect here.
+        engine2.memoryManager->runGC();
+        QCOMPARE(engine2.memoryManager->m_pendingFreedObjectWrapperValue.size(), 0);
+    }
+
+    // Clears m_pendingFreedObjectWrapperValue. Now it's really dead.
+    engine1.memoryManager->runGC();
+    QCOMPARE(engine1.memoryManager->m_pendingFreedObjectWrapperValue.size(), 0);
+
+    engine2.memoryManager->runGC();
+    QCOMPARE(engine2.memoryManager->m_pendingFreedObjectWrapperValue.size(), 0);
+}
+
 QTEST_MAIN(tst_qv4mm)
 
 #include "tst_qv4mm.moc"