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 <liang.qi@digia.com>
Reviewed-by: Shawn Rutledge <shawn.rutledge@digia.com>

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index b4732c3..21d5058 100644 (file)
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -2744,10 +2744,10 @@ void QQuickItemPrivate::data_append(QQmlListProperty<QObject> *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<QObject> *property)
 
 QObject *QQuickItemPrivate::resources_at(QQmlListProperty<QObject> *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<QQuickItem *>(prop->object));
+    return quickItemPrivate->extra.isAllocated() ? quickItemPrivate->extra->resourcesList.value(index) : 0;
 }
 
-void QQuickItemPrivate::resources_append(QQmlListProperty<QObject> *prop, QObject *o)
+void QQuickItemPrivate::resources_append(QQmlListProperty<QObject> *prop, QObject *object)
 {
-    // XXX todo - do we really want this behavior?
-    o->setParent(prop->object);
+    QQuickItem *quickItem = static_cast<QQuickItem *>(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<QObject> *prop)
 {
-    return prop->object->children().count();
+    QQuickItemPrivate *quickItemPrivate = QQuickItemPrivate::get(static_cast<QQuickItem *>(prop->object));
+    return  quickItemPrivate->extra.isAllocated() ? quickItemPrivate->extra->resourcesList.count() : 0;
 }
 
 void QQuickItemPrivate::resources_clear(QQmlListProperty<QObject> *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<QQuickItem *>(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<QQuickItem> *prop, int index)
@@ -2995,6 +3003,12 @@ void QQuickItemPrivate::transform_clear(QQmlListProperty<QQuickTransform> *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 (file)
--- 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 (file)
--- a/src/quick/items/qquickitem_p.h
+++ b/src/quick/items/qquickitem_p.h
@@ -293,6 +293,8 @@ public:
     static QQuickTransform *transform_at(QQmlListProperty<QQuickTransform> *list, int);
     static void transform_clear(QQmlListProperty<QQuickTransform> *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<ExtraData> extra;
 

diff --git a/tests/auto/quick/qquickitem2/data/resourcesProperty.qml b/tests/auto/quick/qquickitem2/data/resourcesProperty.qml
index b8f18bb..2c4c0aa 100644 (file)
--- 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 (file)
--- 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<QQmlListReference>(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<QObject *>("subObject");
+
+    QVERIFY(subObject);
+
+    QCOMPARE(object, subObject->parent());
+
+    delete subObject;
+
+    QCOMPARE(list.count(), 3);
+
+    delete object;
 }
 
 void tst_QQuickItem::propertyChanges()