Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Chris Adams authored and Qt by Nokia committed Mar 14, 2012
1 parent a27fd58 commit b061083
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 11 deletions.
42 changes: 32 additions & 10 deletions src/qml/qml/qqmlvmemetaobject.cpp
Expand Up @@ -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:
Expand All @@ -79,7 +101,7 @@ class QQmlVMEVariant
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);
Expand All @@ -93,7 +115,7 @@ class QQmlVMEVariant
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();
};
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]));
Expand Down Expand Up @@ -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() ||
Expand Down
14 changes: 14 additions & 0 deletions src/qml/qml/qqmlvmemetaobject_p.h
Expand Up @@ -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;
Expand Down Expand Up @@ -163,6 +176,7 @@ class Q_AUTOTEST_EXPORT QQmlVMEMetaObject : public QAbstractDynamicMetaObject,

private:
friend class QQmlVMEMetaObjectEndpoint;
friend class QQmlVMEVariantQObjectPtr;

QObject *object;
QQmlCompiledData *compiledData;
Expand Down
2 changes: 1 addition & 1 deletion src/qml/qml/v8/qv8qobjectwrapper.cpp
Expand Up @@ -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);

Expand Down
30 changes: 30 additions & 0 deletions 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.
}
}
22 changes: 22 additions & 0 deletions tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
Expand Up @@ -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;
}
}

/*
Expand Down

0 comments on commit b061083

Please sign in to comment.