Avoid calling gc in QQmlEngine destructor.

GC may be expensive. GC call in QQmlEngine doesn't have much sense
because V8 will destroy all objects living in a current context.
The only problem is that V8 may decide to not invoke weak callbacks
which may cause a memory leak. To avoid that we track all QObjects that
have JavaScript ownership set and we delete them explicitly.

The change reduce time of destroying QQmlEngine by 75%, which is really
visible in qquicklistmodel test.

Change-Id: I2a3668fd23630669114baee8c241a7ecc4100e33
Reviewed-by: Chris Adams <christopher.adams@nokia.com>

3 files changed, 62 insertions, 8 deletions

diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
index e34304b258..5ebc42a453 100644
--- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp
+++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
@@ -120,6 +120,7 @@ private slots:
 #endif
     void newQObject_promoteNonObject();
     void newQObject_promoteNonQScriptObject();
+    void newQObject_deletedEngine();
 #if 0 // ### FIXME: No QScript Metaobject support right now
     void newQMetaObject();
     void newActivationObject();
@@ -1120,6 +1121,19 @@ void tst_QJSEngine::newQObject_promoteNonQScriptObject()
 #endif
 }
 
+void tst_QJSEngine::newQObject_deletedEngine()
+{
+    QJSValue object;
+    QObject *ptr = new QObject();
+    QSignalSpy spy(ptr, SIGNAL(destroyed()));
+    {
+        QJSEngine engine;
+        object = engine.newQObject(ptr);
+        engine.globalObject().setProperty("obj", object);
+    }
+    QTRY_VERIFY(spy.count());
+}
+
 #if 0 // ### FIXME: No QScript Metaobject support right now
 QT_BEGIN_NAMESPACE
 Q_SCRIPT_DECLARE_QMETAOBJECT(QObject, QObject*)
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 4b913ef697..cb56948b6e 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -4904,18 +4904,20 @@ void tst_qqmlecmascript::handleReferenceManagement()
         QQmlEngine::setObjectOwnership(second1, QQmlEngine::JavaScriptOwnership);
         QQmlEngine::setObjectOwnership(first2, QQmlEngine::JavaScriptOwnership);
         QQmlEngine::setObjectOwnership(second2, QQmlEngine::JavaScriptOwnership);
-        gc(engine);
-        QCOMPARE(dtorCount, 0);
-        delete hrmEngine2;
-        gc(engine);
+        gc(*hrmEngine1);
+        gc(*hrmEngine2);
         QCOMPARE(dtorCount, 0);
+        delete hrmEngine2; // should trigger deletion of objects with JS ownership tracked by this engine
+        gc(*hrmEngine1);
+        QCOMPARE(dtorCount, 2); // first2 and second2 should have been deleted.
         delete object1;
         delete object2;
-        hrmEngine1->collectGarbage();
+        gc(*hrmEngine1);
+        QCOMPARE(dtorCount, 6); // deleting object1 and object2 should trigger deletion of first1 and first2.
+        delete hrmEngine1;
         QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
         QCoreApplication::processEvents();
-        QCOMPARE(dtorCount, 6);
-        delete hrmEngine1;
+        QCOMPARE(dtorCount, 6); // all objects should have been cleaned up prior to deleting hrmEngine1.
     }
 }
 
diff --git a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp
index c039429f48..ab18cdd050 100644
--- a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp
+++ b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp
@@ -46,6 +46,7 @@
 #include <QPointer>
 #include <QDir>
 #include <QStandardPaths>
+#include <QSignalSpy>
 #include <QDebug>
 #include <QQmlComponent>
 #include <QQmlNetworkAccessManagerFactory>
@@ -67,6 +68,13 @@ private slots:
     void outputWarningsToStandardError();
     void objectOwnership();
     void multipleEngines();
+
+public slots:
+    QObject *createAQObjectForOwnershipTest ()
+    {
+        static QObject *ptr = new QObject();
+        return ptr;
+    }
 };
 
 void tst_qqmlengine::rootContext()
@@ -326,7 +334,37 @@ void tst_qqmlengine::objectOwnership()
 
     delete o;
     }
-
+    {
+        QObject *ptr = createAQObjectForOwnershipTest();
+        QSignalSpy spy(ptr, SIGNAL(destroyed()));
+        {
+            QQmlEngine engine;
+            QQmlComponent c(&engine);
+            engine.rootContext()->setContextProperty("test", this);
+            QQmlEngine::setObjectOwnership(ptr, QQmlEngine::JavaScriptOwnership);
+            c.setData("import QtQuick 2.0; Item { property int data: test.createAQObjectForOwnershipTest() ? 0 : 1 }", QUrl());
+            QVERIFY(c.isReady());
+            QObject *o = c.create();
+            QVERIFY(o != 0);
+        }
+        QTRY_VERIFY(spy.count());
+    }
+    {
+        QObject *ptr = new QObject();
+        QSignalSpy spy(ptr, SIGNAL(destroyed()));
+        {
+            QQmlEngine engine;
+            QQmlComponent c(&engine);
+            engine.rootContext()->setContextProperty("test", ptr);
+            QQmlEngine::setObjectOwnership(ptr, QQmlEngine::JavaScriptOwnership);
+            c.setData("import QtQuick 2.0; QtObject { property var object: { var i = test; test ? 0 : 1 }  }", QUrl());
+            QVERIFY(c.isReady());
+            QObject *o = c.create();
+            QVERIFY(o != 0);
+            engine.rootContext()->setContextProperty("test", 0);
+        }
+        QTRY_VERIFY(spy.count());
+    }
 }
 
 // Test an object can be accessed by multiple engines