Ensure that we don't attempt to dispose handle twice

If a weak ref callback causes disposal of a v8object associated with
a qobject, the later qqmldata::destroyed() handler could cause a
double dispose, due to 753d9f4be5960be8b11ad067b29fc87c168ee663.

Change-Id: I07c1c8e2e7b444a7e873da26bc4d0c19bcfe57b5
Reviewed-by: Matthew Vogt <matthew.vogt@nokia.com>

10 files changed, 159 insertions, 0 deletions

diff --git a/tests/auto/qml/qqmlecmascript/data/DeleteRootObjectInCreationComponentBase.qml b/tests/auto/qml/qqmlecmascript/data/DeleteRootObjectInCreationComponentBase.qml
new file mode 100644
index 0000000000..ce542d3dbd
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/DeleteRootObjectInCreationComponentBase.qml
@@ -0,0 +1,24 @@
+import QtQuick 2.0
+import Qt.test.qobjectApi 1.0 as ModApi
+
+Rectangle {
+    id: base
+    color: "red"
+
+    function flipOwnership() {
+        ModApi.trackObject(base);
+        ModApi.trackedObject(); // flip the ownership.
+        if (!ModApi.trackedObjectHasJsOwnership())
+            derived.testConditionsMet = false;
+        else
+            derived.testConditionsMet = true;
+    }
+
+    onColorChanged: {
+        // will be triggered during beginCreate of derived
+        flipOwnership();
+        gc();
+        gc();
+        gc();
+    }
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/DeleteRootObjectInCreationComponentDerived.qml b/tests/auto/qml/qqmlecmascript/data/DeleteRootObjectInCreationComponentDerived.qml
new file mode 100644
index 0000000000..c6273465d7
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/DeleteRootObjectInCreationComponentDerived.qml
@@ -0,0 +1,11 @@
+import QtQuick 2.0
+
+DeleteRootObjectInCreationComponentBase {
+    id: derived
+    color: "black" // will trigger change signal during beginCreate.
+
+    property bool testConditionsMet: false // will be set by base
+    function setTestConditionsMet(obj) {
+        obj.testConditionsMet = derived.testConditionsMet;
+    }
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent.qml b/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent.qml
new file mode 100644
index 0000000000..f5c0ce6b18
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent.qml
@@ -0,0 +1,5 @@
+import QtQuick 2.0
+
+Item {
+    property int a: 50
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent2Base.qml b/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent2Base.qml
new file mode 100644
index 0000000000..486e88a9e3
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent2Base.qml
@@ -0,0 +1,26 @@
+import QtQuick 2.0
+import Qt.test.qobjectApi 1.0 as ModApi
+
+Rectangle {
+    id: base
+    x: 1
+    color: "red"
+    property bool testConditionsMet: false
+
+    onXChanged: {
+        ModApi.trackObject(base);
+        ModApi.trackedObject(); // flip the ownership.
+        if (!ModApi.trackedObjectHasJsOwnership())
+            testConditionsMet = false;
+        else
+            testConditionsMet = true;
+   }
+
+    onColorChanged: {
+        // will be triggered during beginCreate of derived
+        test.testConditionsMet = testConditionsMet;
+        gc();
+        gc();
+        gc();
+    }
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent2Derived.qml b/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent2Derived.qml
new file mode 100644
index 0000000000..b736e70dea
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/QQmlDataDestroyedComponent2Derived.qml
@@ -0,0 +1,7 @@
+import QtQuick 2.0
+
+QQmlDataDestroyedComponent2Base {
+    id: derived
+    color: "black" // will trigger change signal during beginCreate.
+    x: 2 // flip ownership of base
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/deleteRootObjectInCreation.2.qml b/tests/auto/qml/qqmlecmascript/data/deleteRootObjectInCreation.2.qml
new file mode 100644
index 0000000000..b67e8bb7d8
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/deleteRootObjectInCreation.2.qml
@@ -0,0 +1,10 @@
+import QtQuick 2.0
+
+Item {
+    id: test
+    property bool testConditionsMet: false
+    Component.onCompleted: {
+        var c = Qt.createComponent("DeleteRootObjectInCreationComponentDerived.qml")
+        c.createObject(null).setTestConditionsMet(test); // JS ownership, but it will be a RootObjectInCreation until finished beginCreate.
+    }
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/qqmldataDestroyed.2.qml b/tests/auto/qml/qqmlecmascript/data/qqmldataDestroyed.2.qml
new file mode 100644
index 0000000000..19222712cd
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/qqmldataDestroyed.2.qml
@@ -0,0 +1,10 @@
+import QtQuick 2.0
+
+Item {
+    id: test
+    property bool testConditionsMet: false
+    Component.onCompleted: {
+        var c = Qt.createComponent("QQmlDataDestroyedComponent2Derived.qml")
+        c.createObject(test); // Cpp ownership, but it will be a RootObjectInCreation until finished beginCreate.
+    }
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/qqmldataDestroyed.qml b/tests/auto/qml/qqmlecmascript/data/qqmldataDestroyed.qml
new file mode 100644
index 0000000000..84308ec46e
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/qqmldataDestroyed.qml
@@ -0,0 +1,8 @@
+import QtQuick 2.0
+
+Item {
+    Component.onCompleted: {
+        var c = Qt.createComponent("QQmlDataDestroyedComponent.qml")
+        var o = c.createObject(null); // JS ownership
+    }
+}
diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h
index 5fb78e81b6..4740c47caa 100644
--- a/tests/auto/qml/qqmlecmascript/testtypes.h
+++ b/tests/auto/qml/qqmlecmascript/testtypes.h
@@ -1033,6 +1033,19 @@ public:
     int qobjectTestWritableFinalProperty() const { return m_testWritableFinalProperty; }
     void setQObjectTestWritableFinalProperty(int tp) { m_testWritableFinalProperty = tp; emit qobjectTestWritableFinalPropertyChanged(); }
 
+    Q_INVOKABLE bool trackedObjectHasJsOwnership() {
+        QObject * object = m_trackedObject;
+
+        if (!object)
+            return false;
+
+        QQmlData *ddata = QQmlData::get(object, false);
+        if (!ddata)
+            return false;
+        else
+            return ddata->indestructible?false:true;
+    }
+
 signals:
     void qobjectTestPropertyChanged(int testProperty);
     void qobjectTestWritablePropertyChanged(int testWritableProperty);
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index b910ec7b83..0ea4e2a413 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -263,6 +263,7 @@ private slots:
     void bindingSuppression();
     void signalEmitted();
     void threadSignal();
+    void qqmldataDestroyed();
 
 private:
     static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
@@ -6648,6 +6649,7 @@ void tst_qqmlecmascript::replaceBinding()
 
 void tst_qqmlecmascript::deleteRootObjectInCreation()
 {
+    {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("deleteRootObjectInCreation.qml"));
     QObject *obj = c.create();
@@ -6657,6 +6659,15 @@ void tst_qqmlecmascript::deleteRootObjectInCreation()
     QTest::qWait(1);
     QVERIFY(obj->property("childDestructible").toBool());
     delete obj;
+    }
+
+    {
+    QQmlComponent c(&engine, testFileUrl("deleteRootObjectInCreation.2.qml"));
+    QObject *object = c.create();
+    QVERIFY(object != 0);
+    QVERIFY(object->property("testConditionsMet").toBool());
+    delete object;
+    }
 }
 
 void tst_qqmlecmascript::onDestruction()
@@ -6768,6 +6779,40 @@ void tst_qqmlecmascript::threadSignal()
     delete object;
 }
 
+// ensure that the qqmldata::destroyed() handler doesn't cause problems
+void tst_qqmlecmascript::qqmldataDestroyed()
+{
+    // gc cleans up a qobject, later the qqmldata destroyed handler will run.
+    {
+        QQmlComponent c(&engine, testFileUrl("qqmldataDestroyed.qml"));
+        QObject *object = c.create();
+        QVERIFY(object != 0);
+        // now gc causing the collection of the dynamically constructed object.
+        engine.collectGarbage();
+        engine.collectGarbage();
+        // now process events to allow deletion (calling qqmldata::destroyed())
+        QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
+        QCoreApplication::processEvents();
+        // shouldn't crash.
+        delete object;
+    }
+
+    // in this case, the object has CPP ownership, and the gc will
+    // be triggered during its beginCreate stage.
+    {
+        QQmlComponent c(&engine, testFileUrl("qqmldataDestroyed.2.qml"));
+        QObject *object = c.create();
+        QVERIFY(object != 0);
+        QVERIFY(object->property("testConditionsMet").toBool());
+        // the gc() within the handler will have triggered the weak
+        // qobject reference callback.  If that incorrectly disposes
+        // the handle, when the qqmldata::destroyed() handler is
+        // called due to object deletion we will see a crash.
+        delete object;
+        // shouldn't have crashed.
+    }
+}
+
 QTEST_MAIN(tst_qqmlecmascript)
 
 #include "tst_qqmlecmascript.moc"