From 95fd73b86cddf806c2065cbdad2e0f7641379ce6 Mon Sep 17 00:00:00 2001 From: Thomas Hartmann Date: Wed, 8 May 2013 11:18:04 +0200 Subject: [PATCH] The resources property should be independent from QObject Resources was a direct mapping of the QObject children. This led to problems since the QObject children can also contain other objects from other sources like attached properties. This patch decouples resources from QObject properties and also does not call setParent() anymore. The special case for QQuickWindow in data_append does rely on the fact that QObject::setParent() is called and the inner window becomes a QObject::child of the content item. So we keep the setParent for this special case. The children property does not take QObject ownership either. QObject ownership is handled by the VME. None of the documented QML use cases should be touched by this change. This is a cleaner solution then the ad hoc fix provided by https://codereview.qt-project.org/#change,54677 I changed also the test. The list count now has to be exactly 4. Change-Id: I5c119e333ee82e888aaac1da559fd63a875d08ee Reviewed-by: Liang Qi Reviewed-by: Shawn Rutledge --- src/quick/items/qquickitem.cpp | 44 +++++++++++++------- src/quick/items/qquickitem.h | 2 + src/quick/items/qquickitem_p.h | 4 ++ .../quick/qquickitem2/data/resourcesProperty.qml | 23 ++++++++-- tests/auto/quick/qquickitem2/tst_qquickitem.cpp | 36 ++++++++++++---- 5 files changed, 81 insertions(+), 28 deletions(-) diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index b4732c3..21d5058 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -2744,10 +2744,10 @@ void QQuickItemPrivate::data_append(QQmlListProperty *prop, QObject *o) QObject::connect(item, SIGNAL(windowChanged(QQuickWindow*)), thisWindow, SLOT(setTransientParent_helper(QQuickWindow*))); } + o->setParent(that); } - // XXX todo - do we really want this behavior? - o->setParent(that); + resources_append(prop, o); } } @@ -2825,30 +2825,38 @@ void QQuickItemPrivate::data_clear(QQmlListProperty *property) QObject *QQuickItemPrivate::resources_at(QQmlListProperty *prop, int index) { - const QObjectList children = prop->object->children(); - if (index < children.count()) - return children.at(index); - else - return 0; + QQuickItemPrivate *quickItemPrivate = QQuickItemPrivate::get(static_cast(prop->object)); + return quickItemPrivate->extra.isAllocated() ? quickItemPrivate->extra->resourcesList.value(index) : 0; } -void QQuickItemPrivate::resources_append(QQmlListProperty *prop, QObject *o) +void QQuickItemPrivate::resources_append(QQmlListProperty *prop, QObject *object) { - // XXX todo - do we really want this behavior? - o->setParent(prop->object); + QQuickItem *quickItem = static_cast(prop->object); + QQuickItemPrivate *quickItemPrivate = QQuickItemPrivate::get(quickItem); + if (!quickItemPrivate->extra.value().resourcesList.contains(object)) { + quickItemPrivate->extra.value().resourcesList.append(object); + qmlobject_connect(object, QObject, SIGNAL(destroyed(QObject*)), + quickItem, QQuickItem, SLOT(_q_resourceObjectDeleted(QObject*))); + } } int QQuickItemPrivate::resources_count(QQmlListProperty *prop) { - return prop->object->children().count(); + QQuickItemPrivate *quickItemPrivate = QQuickItemPrivate::get(static_cast(prop->object)); + return quickItemPrivate->extra.isAllocated() ? quickItemPrivate->extra->resourcesList.count() : 0; } void QQuickItemPrivate::resources_clear(QQmlListProperty *prop) { - // XXX todo - do we really want this behavior? - const QObjectList children = prop->object->children(); - for (int index = 0; index < children.count(); index++) - children.at(index)->setParent(0); + QQuickItem *quickItem = static_cast(prop->object); + QQuickItemPrivate *quickItemPrivate = QQuickItemPrivate::get(quickItem); + if (quickItemPrivate->extra.isAllocated()) {//If extra is not allocated resources is empty. + foreach (QObject *object, quickItemPrivate->extra->resourcesList) { + qmlobject_disconnect(object, QObject, SIGNAL(destroyed(QObject*)), + quickItem, QQuickItem, SLOT(_q_resourceObjectDeleted(QObject*))); + } + quickItemPrivate->extra->resourcesList.clear(); + } } QQuickItem *QQuickItemPrivate::children_at(QQmlListProperty *prop, int index) @@ -2995,6 +3003,12 @@ void QQuickItemPrivate::transform_clear(QQmlListProperty *prop) p->dirty(QQuickItemPrivate::Transform); } +void QQuickItemPrivate::_q_resourceObjectDeleted(QObject *object) +{ + if (extra.isAllocated() && extra->resourcesList.contains(object)) + extra->resourcesList.removeAll(object); +} + /*! \qmlproperty AnchorLine QtQuick2::Item::anchors.top \qmlproperty AnchorLine QtQuick2::Item::anchors.bottom diff --git a/src/quick/items/qquickitem.h b/src/quick/items/qquickitem.h index 078df2e..84eafec 100644 --- a/src/quick/items/qquickitem.h +++ b/src/quick/items/qquickitem.h @@ -432,6 +432,8 @@ protected: QQuickItem(QQuickItemPrivate &dd, QQuickItem *parent = 0); private: + Q_PRIVATE_SLOT(d_func(), void _q_resourceObjectDeleted(QObject *)) + friend class QQuickWindow; friend class QQuickWindowPrivate; friend class QSGRenderer; diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h index 4c2df99..c71da3c 100644 --- a/src/quick/items/qquickitem_p.h +++ b/src/quick/items/qquickitem_p.h @@ -293,6 +293,8 @@ public: static QQuickTransform *transform_at(QQmlListProperty *list, int); static void transform_clear(QQmlListProperty *list); + void _q_resourceObjectDeleted(QObject *); + enum ChangeType { Geometry = 0x01, SiblingOrder = 0x02, @@ -363,6 +365,8 @@ public: Qt::MouseButtons acceptedMouseButtons; QQuickItem::TransformOrigin origin:5; + + QObjectList resourcesList; }; QLazilyAllocated extra; diff --git a/tests/auto/quick/qquickitem2/data/resourcesProperty.qml b/tests/auto/quick/qquickitem2/data/resourcesProperty.qml index b8f18bb..2c4c0aa 100644 --- a/tests/auto/quick/qquickitem2/data/resourcesProperty.qml +++ b/tests/auto/quick/qquickitem2/data/resourcesProperty.qml @@ -8,14 +8,27 @@ Item { property bool test3 property bool test4 property bool test5 + property bool test6 Component.onCompleted: { - test1 = (root.resources.length >= 3) - test2 = root.resources[0] == item1 - test3 = root.resources[1] == item2 - test4 = root.resources[2] == item3 - test5 = root.resources[10] == null + test1 = (root.resources.length === 4) + test2 = root.resources[0] === item1 + test3 = root.resources[1] === item2 + test4 = root.resources[2] === item3 + test5 = root.resources[3] === otherObject + test6 = root.resources[10] == null } + //Resources can be used explicitly resources: [ Item { id: item1 }, Item { id: item2 }, Item { id: item3 } ] + + Item { + //Item in Data go the children property. + } + + QtObject { + //Objects in Data which are not items are put in resources. + id: otherObject + objectName: "subObject"; + } } diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp index 18ab581..9a6bed6 100644 --- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp +++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp @@ -1840,15 +1840,35 @@ void tst_QQuickItem::resourcesProperty() { QQmlComponent component(&engine, testFileUrl("resourcesProperty.qml")); - QObject *o = component.create(); - QVERIFY(o != 0); + QObject *object = component.create(); + QVERIFY(object != 0); - QCOMPARE(o->property("test1").toBool(), true); - QCOMPARE(o->property("test2").toBool(), true); - QCOMPARE(o->property("test3").toBool(), true); - QCOMPARE(o->property("test4").toBool(), true); - QCOMPARE(o->property("test5").toBool(), true); - delete o; + QQmlProperty property(object, "resources", component.creationContext()); + + QVERIFY(property.isValid()); + QQmlListReference list = qvariant_cast(property.read()); + QVERIFY(list.isValid()); + + QCOMPARE(list.count(), 4); + + QCOMPARE(object->property("test1").toBool(), true); + QCOMPARE(object->property("test2").toBool(), true); + QCOMPARE(object->property("test3").toBool(), true); + QCOMPARE(object->property("test4").toBool(), true); + QCOMPARE(object->property("test5").toBool(), true); + QCOMPARE(object->property("test6").toBool(), true); + + QObject *subObject = object->findChild("subObject"); + + QVERIFY(subObject); + + QCOMPARE(object, subObject->parent()); + + delete subObject; + + QCOMPARE(list.count(), 3); + + delete object; } void tst_QQuickItem::propertyChanges() -- 1.7.2.5