Revert "Delete JS-owned QML objects right away in the engine dtor."

This reverts commit ecc432a5b7ae269220f86c6f0b3dd364f8643191
to fix a crash on exit.

Task-number: QTBUG-20377

Change-Id: I6606ff194f2c16e06bdbbfca94e55821cf055f75
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
Reviewed-by: Thomas McGuire <thomas.mcguire@kdab.com>

11 files changed, 24 insertions, 119 deletions

diff --git a/src/declarative/qml/qdeclarativeengine.cpp b/src/declarative/qml/qdeclarativeengine.cpp
index 91390cd..92a7391 100644
--- a/src/declarative/qml/qdeclarativeengine.cpp
+++ b/src/declarative/qml/qdeclarativeengine.cpp
@@ -350,9 +350,9 @@ QDeclarativeEnginePrivate::QDeclarativeEnginePrivate(QDeclarativeEngine *e)
 : captureProperties(false), rootContext(0), isDebugging(false),
   outputWarningsToStdErr(true), contextClass(0), sharedContext(0), sharedScope(0),
   objectClass(0), valueTypeClass(0), globalClass(0), cleanup(0), erroredBindings(0),
-  inProgressCreations(0), scriptEngine(new QDeclarativeScriptEngine(this)),
-  workerScriptEngine(0), componentAttached(0), inBeginCreate(false), networkAccessManager(0),
-  networkAccessManagerFactory(0), typeLoader(e), importDatabase(e), uniqueId(1)
+  inProgressCreations(0), scriptEngine(this), workerScriptEngine(0), componentAttached(0),
+  inBeginCreate(false), networkAccessManager(0), networkAccessManagerFactory(0),
+  typeLoader(e), importDatabase(e), uniqueId(1)
 {
     if (!qt_QmlQtModule_registered) {
         qt_QmlQtModule_registered = true;
@@ -365,7 +365,7 @@ QDeclarativeEnginePrivate::QDeclarativeEnginePrivate(QDeclarativeEngine *e)
         }
         QDeclarativeUtilModule::defineModule(appType);
     }
-    globalClass = new QDeclarativeGlobalScriptClass(scriptEngine);
+    globalClass = new QDeclarativeGlobalScriptClass(&scriptEngine);
 }
 
 /*!
@@ -500,12 +500,6 @@ QDeclarativeEnginePrivate::~QDeclarativeEnginePrivate()
         c->clear();
     }
 
-    // Destroy the script engine now, before object class is destroyed.
-    // This is needed here since destroying the script engine causes the
-    // JSC heap to be cleaned up, and that needs the object class intact
-    // to work.
-    delete scriptEngine;
-
     delete rootContext;
     rootContext = 0;
     delete contextClass;
@@ -595,7 +589,7 @@ void QDeclarativeEnginePrivate::init()
     rootContext = new QDeclarativeContext(q,true);
 
     QScriptValue applicationObject = objectClass->newQObject(new QDeclarativeApplication(q));
-    scriptEngine->globalObject().property(QLatin1String("Qt")).setProperty(QLatin1String("application"), applicationObject);
+    scriptEngine.globalObject().property(QLatin1String("Qt")).setProperty(QLatin1String("application"), applicationObject);
 
     if (QCoreApplication::instance()->thread() == q->thread() &&
         QDeclarativeEngineDebugService::isDebuggingEnabled()) {
@@ -2117,11 +2111,11 @@ QScriptValue QDeclarativeEnginePrivate::scriptValueFromVariant(const QVariant &v
         if (p->object) {
             return listClass->newList(p->property, p->propertyType);
         } else {
-            return scriptEngine->nullValue();
+            return scriptEngine.nullValue();
         }
     } else if (val.userType() == qMetaTypeId<QList<QObject *> >()) {
         const QList<QObject *> &list = *(QList<QObject *>*)val.constData();
-        QScriptValue rv = scriptEngine->newArray(list.count());
+        QScriptValue rv = scriptEngine.newArray(list.count());
         for (int ii = 0; ii < list.count(); ++ii) {
             QObject *object = list.at(ii);
             rv.setProperty(ii, objectClass->newQObject(object));
@@ -2136,7 +2130,7 @@ QScriptValue QDeclarativeEnginePrivate::scriptValueFromVariant(const QVariant &v
     if (objOk) {
         return objectClass->newQObject(obj);
     } else {
-        return scriptEngine->toScriptValue(val);
+        return scriptEngine.toScriptValue(val);
     }
 }
 
@@ -2303,13 +2297,13 @@ bool QDeclarativeEngine::importPlugin(const QString &filePath, const QString &ur
 void QDeclarativeEngine::setOfflineStoragePath(const QString& dir)
 {
     Q_D(QDeclarativeEngine);
-    d->scriptEngine->offlineStoragePath = dir;
+    d->scriptEngine.offlineStoragePath = dir;
 }
 
 QString QDeclarativeEngine::offlineStoragePath() const
 {
     Q_D(const QDeclarativeEngine);
-    return d->scriptEngine->offlineStoragePath;
+    return d->scriptEngine.offlineStoragePath;
 }
 
 static void voidptr_destructor(void *v)
diff --git a/src/declarative/qml/qdeclarativeengine_p.h b/src/declarative/qml/qdeclarativeengine_p.h
index ee95120..c324f7a 100644
--- a/src/declarative/qml/qdeclarativeengine_p.h
+++ b/src/declarative/qml/qdeclarativeengine_p.h
@@ -181,7 +181,7 @@ public:
     QDeclarativeDelayedError *erroredBindings;
     int inProgressCreations;
 
-    QDeclarativeScriptEngine *scriptEngine;
+    QDeclarativeScriptEngine scriptEngine;
 
     QDeclarativeWorkerScriptEngine *getWorkerScriptEngine();
     QDeclarativeWorkerScriptEngine *workerScriptEngine;
@@ -311,7 +311,7 @@ public:
     static QScriptValue formatTime(QScriptContext*, QScriptEngine*);
     static QScriptValue formatDateTime(QScriptContext*, QScriptEngine*);
 #endif
-    static QScriptEngine *getScriptEngine(QDeclarativeEngine *e) { return e->d_func()->scriptEngine; }
+    static QScriptEngine *getScriptEngine(QDeclarativeEngine *e) { return &e->d_func()->scriptEngine; }
     static QDeclarativeEngine *getEngine(QScriptEngine *e) { return static_cast<QDeclarativeScriptEngine*>(e)->p->q_func(); }
     static QDeclarativeEnginePrivate *get(QDeclarativeEngine *e) { return e->d_func(); }
     static QDeclarativeEnginePrivate *get(QDeclarativeContext *c) { return (c && c->engine()) ? QDeclarativeEnginePrivate::get(c->engine()) : 0; }
diff --git a/src/declarative/qml/qdeclarativeexpression.cpp b/src/declarative/qml/qdeclarativeexpression.cpp
index e3dba93..f457286 100644
--- a/src/declarative/qml/qdeclarativeexpression.cpp
+++ b/src/declarative/qml/qdeclarativeexpression.cpp
@@ -176,7 +176,7 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex
                                                               int lineNumber, QScriptValue *contextObject)
 {
     QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(context->engine);
-    QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(ep->scriptEngine);
+    QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(&ep->scriptEngine);
     if (contextObject) {
         *contextObject = ep->contextClass->newContext(context, object);
         scriptContext->pushScope(*contextObject);
@@ -184,8 +184,8 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex
         scriptContext->pushScope(ep->contextClass->newContext(context, object));
     }
     scriptContext->pushScope(ep->globalClass->staticGlobalObject());
-    QScriptValue rv = ep->scriptEngine->evaluate(program, fileName, lineNumber);
-    ep->scriptEngine->popContext();
+    QScriptValue rv = ep->scriptEngine.evaluate(program, fileName, lineNumber);
+    ep->scriptEngine.popContext();
     return rv;
 }
 
@@ -194,7 +194,7 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex
                                                               QScriptValue *contextObject)
 {
     QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(context->engine);
-    QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(ep->scriptEngine);
+    QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(&ep->scriptEngine);
     if (contextObject) {
         *contextObject = ep->contextClass->newContext(context, object);
         scriptContext->pushScope(*contextObject);
@@ -202,8 +202,8 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex
         scriptContext->pushScope(ep->contextClass->newContext(context, object));
     }
     scriptContext->pushScope(ep->globalClass->staticGlobalObject());
-    QScriptValue rv = ep->scriptEngine->evaluate(program);
-    ep->scriptEngine->popContext();
+    QScriptValue rv = ep->scriptEngine.evaluate(program);
+    ep->scriptEngine.popContext();
     return rv;
 }
 
diff --git a/src/declarative/qml/qdeclarativeobjectscriptclass.cpp b/src/declarative/qml/qdeclarativeobjectscriptclass.cpp
index f71a64d..8f8557f 100644
--- a/src/declarative/qml/qdeclarativeobjectscriptclass.cpp
+++ b/src/declarative/qml/qdeclarativeobjectscriptclass.cpp
@@ -74,28 +74,14 @@ struct ObjectData : public QScriptDeclarativeClass::Object {
         }
     }
 
-    enum Disposal { Immediate, Deferred };
-
-    inline void disposeObject(Disposal disposal) {
+    virtual ~ObjectData() {
         if (object && !object->parent()) {
             QDeclarativeData *ddata = QDeclarativeData::get(object, false);
-            if (ddata && !ddata->indestructible && 0 == --ddata->objectDataRefCount) {
-                if (disposal == Immediate)
-                    delete object;
-                else
-                    object->deleteLater();
-            }
+            if (ddata && !ddata->indestructible && 0 == --ddata->objectDataRefCount) 
+                object->deleteLater();
         }
     }
 
-    virtual void disposeNow() {
-        disposeObject(Immediate);
-    }
-
-    virtual ~ObjectData() {
-        disposeObject(Deferred);
-    }
-
     QDeclarativeGuard<QObject> object;
     int type;
 };
diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp
index cebbe0e..d821084 100644
--- a/src/script/api/qscriptengine.cpp
+++ b/src/script/api/qscriptengine.cpp
@@ -962,7 +962,7 @@ QScriptEnginePrivate::QScriptEnginePrivate()
       qobjectPrototype(0), qmetaobjectPrototype(0), variantPrototype(0),
       activeAgent(0), agentLineNumber(-1),
       registeredScriptValues(0), freeScriptValues(0), freeScriptValuesCount(0),
-      registeredScriptStrings(0), processEventsInterval(-1), inEval(false), inDestructor(false)
+      registeredScriptStrings(0), processEventsInterval(-1), inEval(false)
 {
     qMetaTypeId<QScriptValue>();
     qMetaTypeId<QList<int> >();
@@ -1015,7 +1015,6 @@ QScriptEnginePrivate::QScriptEnginePrivate()
 
 QScriptEnginePrivate::~QScriptEnginePrivate()
 {
-    inDestructor = true;
     QScript::APIShim shim(this);
 
     //disconnect all loadedScripts and generate all jsc::debugger::scriptUnload events
diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h
index 8a7037b..6a023d7 100644
--- a/src/script/api/qscriptengine_p.h
+++ b/src/script/api/qscriptengine_p.h
@@ -382,7 +382,6 @@ public:
     int processEventsInterval;
     QScriptValue abortResult;
     bool inEval;
-    bool inDestructor;
 
     JSC::UString cachedTranslationUrl;
     JSC::UString cachedTranslationContext;
diff --git a/src/script/bridge/qscriptdeclarativeclass_p.h b/src/script/bridge/qscriptdeclarativeclass_p.h
index c67d23d..c8c35bf 100644
--- a/src/script/bridge/qscriptdeclarativeclass_p.h
+++ b/src/script/bridge/qscriptdeclarativeclass_p.h
@@ -77,14 +77,7 @@ public:
 
     typedef void* Identifier;
 
-    struct Object {
-        // Like the destructor, frees resources associated with this object. The difference is that
-        // the destructor might delay resource freeing with deleteLater(), while disposeNow() always
-        // deletes the resource straigt away.
-        virtual void disposeNow() {}
-
-        virtual ~Object() {}
-    };
+    struct Object { virtual ~Object() {} };
 
     static QScriptValue newObject(QScriptEngine *, QScriptDeclarativeClass *, Object *);
     static Value newObjectValue(QScriptEngine *, QScriptDeclarativeClass *, Object *);
diff --git a/src/script/bridge/qscriptdeclarativeobject.cpp b/src/script/bridge/qscriptdeclarativeobject.cpp
index a9cd87d..927309a 100644
--- a/src/script/bridge/qscriptdeclarativeobject.cpp
+++ b/src/script/bridge/qscriptdeclarativeobject.cpp
@@ -53,11 +53,6 @@ DeclarativeObjectDelegate::DeclarativeObjectDelegate(QScriptDeclarativeClass *c,
 
 DeclarativeObjectDelegate::~DeclarativeObjectDelegate()
 {
-    // When the engine is being destructed, delete the object now, instead of using deleteLater(),
-    // to not have memory leaks on exit.
-    if (m_class->engine() && QScriptEnginePrivate::get(m_class->engine())->inDestructor)
-        m_object->disposeNow();
-
     delete m_object;
 }
 
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml b/tests/auto/declarative/qdeclarativeecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml
deleted file mode 100644
index da1b682..0000000
--- a/tests/auto/declarative/qdeclarativeecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml
+++ /dev/null
@@ -1,6 +0,0 @@
-import QtQuick 1.0
-
-Item {
-    property variant jsOwnedObject1: deleteObject.object1()
-    property variant jsOwnedObject2: deleteObject.object2
-}
diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
index 3d0d37f..19505e1 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
+++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
@@ -903,27 +903,6 @@ protected:
     qreal m_p4;
 };
 
-class MyDeleteObject : public QObject
-{
-    Q_OBJECT
-    Q_PROPERTY(QObject *object2 READ object2 NOTIFY object2Changed)
-
-public:
-    MyDeleteObject() : m_object1(0), m_object2(0) {}
-
-    Q_INVOKABLE QObject *object1() const { return m_object1; }
-    Q_INVOKABLE QObject *object2() const { return m_object2; }
-    void setObject1(QObject *object) { m_object1 = object; }
-    void setObject2(QObject *object) { m_object2 = object; emit object2Changed(); }
-
-signals:
-    void object2Changed();
-
-private:
-    QObject *m_object1;
-    QObject *m_object2;
-};
-
 QML_DECLARE_TYPE(MyRevisionedBaseClassRegistered)
 QML_DECLARE_TYPE(MyRevisionedBaseClassUnregistered)
 QML_DECLARE_TYPE(MyRevisionedClass)
diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index 188cf1b..1c54494 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -48,7 +48,6 @@
 #include <QtDeclarative/private/qdeclarativeguard_p.h>
 #include <QtCore/qdir.h>
 #include <QtCore/qnumeric.h>
-#include <QtTest/qsignalspy.h>
 #include <private/qdeclarativeengine_p.h>
 #include <private/qdeclarativeglobalscriptclass_p.h>
 #include <private/qscriptdeclarativeclass_p.h>
@@ -179,7 +178,6 @@ private slots:
     void pushCleanContext();
     void realToInt();
     void qtbug_20648();
-    void jsOwnedObjectsDeletedOnEngineDestroy();
 
     void include();
 
@@ -1390,7 +1388,7 @@ void tst_qdeclarativeecmascript::callQtInvokables()
 
     QDeclarativeEngine qmlengine;
     QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(&qmlengine);
-    QScriptEngine *engine = ep->scriptEngine;
+    QScriptEngine *engine = &ep->scriptEngine;
 
     QStringList names; QList<QScriptValue> values;
     names << QLatin1String("object"); values << ep->objectClass->newQObject(&o);
@@ -3102,38 +3100,6 @@ void tst_qdeclarativeecmascript::qtbug_20648()
     delete o;
 }
 
-void tst_qdeclarativeecmascript::jsOwnedObjectsDeletedOnEngineDestroy()
-{
-    QDeclarativeEngine *myEngine = new QDeclarativeEngine;
-
-    MyDeleteObject deleteObject;
-    deleteObject.setObjectName("deleteObject");
-    QObject * const object1 = new QObject;
-    QObject * const object2 = new QObject;
-    object1->setObjectName("object1");
-    object2->setObjectName("object2");
-    deleteObject.setObject1(object1);
-    deleteObject.setObject2(object2);
-
-    // Objects returned by function calls get marked as destructible, but objects returned by
-    // property getters do not - therefore we explicitly set the object as destructible.
-    QDeclarativeEngine::setObjectOwnership(object2, QDeclarativeEngine::JavaScriptOwnership);
-
-    myEngine->rootContext()->setContextProperty("deleteObject", &deleteObject);
-    QDeclarativeComponent component(myEngine, TEST_FILE("jsOwnedObjectsDeletedOnEngineDestroy.qml"));
-    QObject *object = component.create();
-    QVERIFY(object);
-
-    // Destroying the engine should delete all JS owned QObjects
-    QSignalSpy spy1(object1, SIGNAL(destroyed()));
-    QSignalSpy spy2(object2, SIGNAL(destroyed()));
-    delete myEngine;
-    QCOMPARE(spy1.count(), 1);
-    QCOMPARE(spy2.count(), 1);
-
-    delete object;
-}
-
 QTEST_MAIN(tst_qdeclarativeecmascript)
 
 #include "tst_qdeclarativeecmascript.moc"