Ensure that dynamic property storing QObject ptr notifies on delete

Previously, when a QObject ptr was stored in a dynamic variant
property, the value of the property could change (to a zero ptr)
if the QObject was deleted without a notify signal being emitted
by the QDeclarativeVMEMetaObject which stores the property.

This commit ensures that such a notify signal is emitted correctly.

Task-number: QTBUG-23451
Change-Id: I5689abd984b177737f8d5f18950838b73ebde328
Reviewed-by: Martin Jones <martin.jones@nokia.com>

diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp
index 7ea89a4..ecfde20 100644 (file)
--- a/src/qml/qml/qqmlvmemetaobject.cpp
+++ b/src/qml/qml/qqmlvmemetaobject.cpp
@@ -56,6 +56,28 @@ Q_DECLARE_METATYPE(QJSValue);
 
 QT_BEGIN_NAMESPACE
 
+QQmlVMEVariantQObjectPtr::QQmlVMEVariantQObjectPtr()
+    : QQmlGuard<QObject>(0), m_target(0), m_index(-1)
+{
+}
+
+QQmlVMEVariantQObjectPtr::~QQmlVMEVariantQObjectPtr()
+{
+}
+
+void QQmlVMEVariantQObjectPtr::objectDestroyed(QObject *)
+{
+    if (m_target && m_index >= 0)
+        m_target->activate(m_target->object, m_target->methodOffset + m_index, 0);
+}
+
+void QQmlVMEVariantQObjectPtr::setGuardedValue(QObject *obj, QQmlVMEMetaObject *target, int index)
+{
+    m_target = target;
+    m_index = index;
+    setObject(obj);
+}
+
 class QQmlVMEVariant
 {
 public:
@@ -79,7 +101,7 @@ public:
     inline const QDateTime &asQDateTime();
     inline const QJSValue &asQJSValue();
 
-    inline void setValue(QObject *);
+    inline void setValue(QObject *v, QQmlVMEMetaObject *target, int index);
     inline void setValue(const QVariant &);
     inline void setValue(int);
     inline void setValue(bool);
@@ -93,7 +115,7 @@ public:
     inline void setValue(const QJSValue &);
 private:
     int type;
-    void *data[4]; // Large enough to hold all types
+    void *data[6]; // Large enough to hold all types
 
     inline void cleanup();
 };
@@ -127,7 +149,7 @@ void QQmlVMEVariant::cleanup()
                type == QMetaType::Double) {
         type = QVariant::Invalid;
     } else if (type == QMetaType::QObjectStar) {
-        ((QQmlGuard<QObject>*)dataPtr())->~QQmlGuard<QObject>();
+        ((QQmlVMEVariantQObjectPtr*)dataPtr())->~QQmlVMEVariantQObjectPtr();
         type = QVariant::Invalid;
     } else if (type == QMetaType::QString) {
         ((QString *)dataPtr())->~QString();
@@ -174,8 +196,8 @@ void *QQmlVMEVariant::dataPtr()
 
 QObject *QQmlVMEVariant::asQObject() 
 {
-    if (type != QMetaType::QObjectStar) 
-        setValue((QObject *)0);
+    if (type != QMetaType::QObjectStar)
+        setValue((QObject *)0, 0, -1);
 
     return *(QQmlGuard<QObject> *)(dataPtr());
 }
@@ -268,14 +290,14 @@ const QJSValue &QQmlVMEVariant::asQJSValue()
     return *(QJSValue *)(dataPtr());
 }
 
-void QQmlVMEVariant::setValue(QObject *v)
+void QQmlVMEVariant::setValue(QObject *v, QQmlVMEMetaObject *target, int index)
 {
     if (type != QMetaType::QObjectStar) {
         cleanup();
         type = QMetaType::QObjectStar;
-        new (dataPtr()) QQmlGuard<QObject>();
+        new (dataPtr()) QQmlVMEVariantQObjectPtr;
     }
-    *(QQmlGuard<QObject>*)(dataPtr()) = v;
+    reinterpret_cast<QQmlVMEVariantQObjectPtr*>(dataPtr())->setGuardedValue(v, target, index);
 }
 
 void QQmlVMEVariant::setValue(const QVariant &v)
@@ -629,7 +651,7 @@ int QQmlVMEMetaObject::metaCall(QMetaObject::Call c, int _id, void **a)
                             break;
                         case QMetaType::QObjectStar:
                             needActivate = *reinterpret_cast<QObject **>(a[0]) != data[id].asQObject();
-                            data[id].setValue(*reinterpret_cast<QObject **>(a[0]));
+                            data[id].setValue(*reinterpret_cast<QObject **>(a[0]), this, id);
                             break;
                         case QMetaType::QVariant:
                             writeProperty(id, *reinterpret_cast<QVariant *>(a[0]));
@@ -892,7 +914,7 @@ void QQmlVMEMetaObject::writeProperty(int id, const QVariant &value)
         if (value.userType() == QMetaType::QObjectStar) {
             QObject *o = qvariant_cast<QObject *>(value);
             needActivate = (data[id].dataType() != QMetaType::QObjectStar || data[id].asQObject() != o);
-            data[id].setValue(qvariant_cast<QObject *>(value));
+            data[id].setValue(qvariant_cast<QObject *>(value), this, id);
         } else {
             needActivate = (data[id].dataType() != qMetaTypeId<QVariant>() ||
                             data[id].asQVariant().userType() != value.userType() ||

diff --git a/src/qml/qml/qqmlvmemetaobject_p.h b/src/qml/qml/qqmlvmemetaobject_p.h
index deee989..84c88fd 100644 (file)
--- a/src/qml/qml/qqmlvmemetaobject_p.h
+++ b/src/qml/qml/qqmlvmemetaobject_p.h
@@ -136,6 +136,19 @@ struct QQmlVMEMetaData
     }
 };
 
+class QQmlVMEMetaObject;
+class QQmlVMEVariantQObjectPtr : public QQmlGuard<QObject>
+{
+public:
+    inline QQmlVMEVariantQObjectPtr();
+    inline ~QQmlVMEVariantQObjectPtr();
+    inline void objectDestroyed(QObject *);
+    inline void setGuardedValue(QObject *obj, QQmlVMEMetaObject *target, int index);
+
+    QQmlVMEMetaObject *m_target;
+    int m_index;
+};
+
 class QV8QObjectWrapper;
 class QQmlVMEVariant;
 class QQmlRefCount;
@@ -163,6 +176,7 @@ protected:
 
 private:
     friend class QQmlVMEMetaObjectEndpoint;
+    friend class QQmlVMEVariantQObjectPtr;
 
     QObject *object;
     QQmlCompiledData *compiledData;

diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp
index 78b2cb7..5842e29 100644 (file)
--- a/src/qml/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp
@@ -1043,7 +1043,7 @@ v8::Handle<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
         return v8::Null();
 
     if (QObjectPrivate::get(object)->wasDeleted)
-       return v8::Undefined();
+       return v8::Null();
     
     QQmlData *ddata = QQmlData::get(object, true);
 

diff --git a/tests/auto/qml/qqmlecmascript/data/dynamicDeletion.3.qml b/tests/auto/qml/qqmlecmascript/data/dynamicDeletion.3.qml
new file mode 100644 (file)
index 0000000..3739150
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/dynamicDeletion.3.qml
@@ -0,0 +1,30 @@
+import QtQuick 2.0
+import Qt.test 1.0
+
+Item {
+    id: root
+    property bool test: false
+    property QtObject objectProperty
+
+    onObjectPropertyChanged: {
+        root.test = true;
+    }
+
+    property Component c: Component {
+        id: dynamicComponent
+        QtObject {
+            id: dynamicObject
+        }
+    }
+
+    function create() {
+        root.objectProperty = root.c.createObject(root);
+    }
+
+    function destroy() {
+        root.test = false; // reset test
+        root.objectProperty.destroy(100);
+        // in cpp, wait objectProperty deletion, inspect "test" and "objectProperty"
+        // test should be true and value of objectProperty should be zero.
+    }
+}

diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 8fea635..e3ad1e7 100644 (file)
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -1288,6 +1288,28 @@ void tst_qqmlecmascript::dynamicDestruction()
 
     delete o;
     }
+
+    {
+    // QTBUG-23451
+    QQmlGuard<QObject> createdQmlObject = 0;
+    QQmlComponent component(&engine, testFileUrl("dynamicDeletion.3.qml"));
+    QObject *o = component.create();
+    QVERIFY(o != 0);
+    QVERIFY(qvariant_cast<QObject*>(o->property("objectProperty")) == 0);
+    QMetaObject::invokeMethod(o, "create");
+    createdQmlObject = qvariant_cast<QObject*>(o->property("objectProperty"));
+    QVERIFY(createdQmlObject);
+    QMetaObject::invokeMethod(o, "destroy");
+    QVERIFY(qvariant_cast<bool>(o->property("test")) == false);
+    for (int ii = 0; createdQmlObject && ii < 50; ++ii) { // After 5 seconds we should give up
+        QTest::qWait(100);
+        QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
+        QCoreApplication::processEvents();
+    }
+    QVERIFY(qvariant_cast<QObject*>(o->property("objectProperty")) == 0);
+    QVERIFY(qvariant_cast<bool>(o->property("test")) == true);
+    delete o;
+    }
 }
 
 /*