Don't parent (QObject) delegate items to views.

This keeps object ownership within the context the items were created
in and simplifies lifetime management as the VisualDataModel has sole
license to delete objects and doesn't have to keep guards against a
view and all it's children being deleted.

Delegates are still reparented in the item heirarchy.

Change-Id: Ife5afdfe294a5a8ca1ca3638a086f72452e4915c
Reviewed-by: Martin Jones <martin.jones@nokia.com>

diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp
index 88a8a30..da661af 100644 (file)
--- a/src/quick/items/qquickitemview.cpp
+++ b/src/quick/items/qquickitemview.cpp
@@ -2181,7 +2181,6 @@ FxViewItem *QQuickItemViewPrivate::createItem(int modelIndex, bool asynchronous)
 
     if (QQuickItem *item = model->item(modelIndex, asynchronous)) {
         item->setParentItem(q->contentItem());
-        QQml_setParent_noEvent(item, q->contentItem());
         requestedIndex = -1;
         FxViewItem *viewItem = requestedItem;
         if (!viewItem)
@@ -2228,7 +2227,6 @@ void QQuickItemView::initItem(int index, QQuickItem *item)
         if (d->requestedAsync)
             item->setVisible(false);
         item->setParentItem(contentItem());
-        QQml_setParent_noEvent(item, contentItem());
         d->requestedItem = d->newViewItem(index, item);
     }
 }

diff --git a/src/quick/items/qquickpathview.cpp b/src/quick/items/qquickpathview.cpp
index fa281cb..1561dbc 100644 (file)
--- a/src/quick/items/qquickpathview.cpp
+++ b/src/quick/items/qquickpathview.cpp
@@ -154,7 +154,6 @@ QQuickItem *QQuickPathViewPrivate::getItem(int modelIndex, qreal z, bool onPath)
     inRequest = true;
     QQuickItem *item = model->item(modelIndex, false);
     if (item) {
-        QQml_setParent_noEvent(item, q);
         item->setParentItem(q);
         requestedIndex = -1;
         qPathViewAttachedType = attType;
@@ -181,7 +180,6 @@ void QQuickPathView::createdItem(int index, QQuickItem *item)
             att->setOnPath(false);
         }
         item->setParentItem(this);
-        QQml_setParent_noEvent(item, this);
         d->updateItem(item, index < d->firstIndex ? 0.0 : 1.0);
     } else {
         d->requestedIndex = -1;

diff --git a/src/quick/items/qquickrepeater.cpp b/src/quick/items/qquickrepeater.cpp
index ec8dc9d..db58fba 100644 (file)
--- a/src/quick/items/qquickrepeater.cpp
+++ b/src/quick/items/qquickrepeater.cpp
@@ -386,7 +386,6 @@ void QQuickRepeaterPrivate::createItems()
                 break;
             }
             deletables[ii] = item;
-            QQml_setParent_noEvent(item, q->parentItem());
             item->setParentItem(q->parentItem());
             if (ii > 0 && deletables.at(ii-1)) {
                 item->stackAfter(deletables.at(ii-1));
@@ -415,7 +414,6 @@ void QQuickRepeater::createdItem(int, QQuickItem *)
 
 void QQuickRepeater::initItem(int, QQuickItem *item)
 {
-    QQml_setParent_noEvent(item, parentItem());
     item->setParentItem(parentItem());
 }
 

diff --git a/src/quick/items/qquickvisualdatamodel.cpp b/src/quick/items/qquickvisualdatamodel.cpp
index dfbf3a3..4d295b3 100644 (file)
--- a/src/quick/items/qquickvisualdatamodel.cpp
+++ b/src/quick/items/qquickvisualdatamodel.cpp
@@ -194,12 +194,10 @@ QQuickVisualDataModel::~QQuickVisualDataModel()
     Q_D(QQuickVisualDataModel);
 
     foreach (QQuickVisualDataModelItem *cacheItem, d->m_cache) {
-        if (QObject *object = cacheItem->object()) {
-            // Clear the guard before deleting the object so it doesn't decrement scriptRef and
-            // potentially delete the cacheItem itself.
-            cacheItem->setObject(0);
-            delete object;
+        if (cacheItem->object) {
+            delete cacheItem->object;
 
+            cacheItem->object = 0;
             cacheItem->contextData->destroy();
             cacheItem->contextData = 0;
             cacheItem->scriptRef -= 1;
@@ -517,8 +515,8 @@ void QQuickVisualDataModel::cancel(int index)
             d->releaseIncubator(cacheItem->incubationTask);
             cacheItem->incubationTask = 0;
         }
-        if (cacheItem->object() && !cacheItem->isObjectReferenced()) {
-            QObject *object = cacheItem->object();
+        if (cacheItem->object && !cacheItem->isObjectReferenced()) {
+            QObject *object = cacheItem->object;
             cacheItem->destroyObject();
             if (QQuickPackage *package = qmlobject_cast<QQuickPackage *>(object))
                 d->emitDestroyingPackage(package);
@@ -784,9 +782,9 @@ void QQuickVisualDataModelPrivate::incubatorStatusChanged(QVDMIncubationTask *in
     if (status == QQmlIncubator::Ready) {
         incubationTask->incubating = 0;
         releaseIncubator(incubationTask);
-        if (QQuickPackage *package = qmlobject_cast<QQuickPackage *>(cacheItem->object()))
+        if (QQuickPackage *package = qmlobject_cast<QQuickPackage *>(cacheItem->object))
             emitCreatedPackage(cacheItem, package);
-        else if (QQuickItem *item = qobject_cast<QQuickItem *>(cacheItem->object()))
+        else if (QQuickItem *item = qobject_cast<QQuickItem *>(cacheItem->object))
             emitCreatedItem(cacheItem, item);
     } else if (status == QQmlIncubator::Error) {
         cacheItem->scriptRef -= 1;
@@ -814,11 +812,11 @@ void QVDMIncubationTask::setInitialState(QObject *o)
 void QQuickVisualDataModelPrivate::setInitialState(QVDMIncubationTask *incubationTask, QObject *o)
 {
     QQuickVisualDataModelItem *cacheItem = incubationTask->incubating;
-    cacheItem->setObject(o);
+    cacheItem->object = o;
 
-    if (QQuickPackage *package = qmlobject_cast<QQuickPackage *>(cacheItem->object()))
+    if (QQuickPackage *package = qmlobject_cast<QQuickPackage *>(cacheItem->object))
         emitInitPackage(cacheItem, package);
-    else if (QQuickItem *item = qmlobject_cast<QQuickItem *>(cacheItem->object()))
+    else if (QQuickItem *item = qmlobject_cast<QQuickItem *>(cacheItem->object))
         emitInitItem(cacheItem, item);
 }
 
@@ -854,7 +852,7 @@ QObject *QQuickVisualDataModelPrivate::object(Compositor::Group group, int index
             // previously requested async - now needed immediately
             cacheItem->incubationTask->forceCompletion();
         }
-    } else if (!cacheItem->object()) {
+    } else if (!cacheItem->object) {
         QQmlContext *creationContext = m_delegate->creationContext();
 
         cacheItem->scriptRef += 1;
@@ -886,9 +884,9 @@ QObject *QQuickVisualDataModelPrivate::object(Compositor::Group group, int index
 
     if (index == m_compositor.count(group) - 1 && m_adaptorModel.canFetchMore())
         QCoreApplication::postEvent(q, new QEvent(QEvent::UpdateRequest));
-    if (cacheItem->object() && reference)
+    if (cacheItem->object && reference)
         cacheItem->referenceObject();
-    return cacheItem->object();
+    return cacheItem->object;
 }
 
 /*
@@ -1157,14 +1155,13 @@ void QQuickVisualDataModelPrivate::itemsRemoved(
         } else {
             for (; cacheIndex < remove.cacheIndex + remove.count - removedCache; ++cacheIndex) {
                 QQuickVisualDataModelItem *cacheItem = m_cache.at(cacheIndex);
-                if (remove.inGroup(Compositor::Persisted) && cacheItem->objectRef == 0 && cacheItem->object()) {
-                    QObject *object = cacheItem->object();
+                if (remove.inGroup(Compositor::Persisted) && cacheItem->objectRef == 0 && cacheItem->object) {
+                    QObject *object = cacheItem->object;
                     cacheItem->destroyObject();
                     if (QQuickPackage *package = qmlobject_cast<QQuickPackage *>(object))
                         emitDestroyingPackage(package);
                     else if (QQuickItem *item = qmlobject_cast<QQuickItem *>(object))
                         emitDestroyingItem(item);
-                    cacheItem->setObject(0);
                     cacheItem->scriptRef -= 1;
                 }
                 if (!cacheItem->isReferenced()) {
@@ -1321,7 +1318,7 @@ void QQuickVisualDataModelPrivate::emitChanges()
         QQuickVisualDataGroupPrivate::get(m_groups[i])->emitModelUpdated(reset);
 
     foreach (QQuickVisualDataModelItem *cacheItem, m_cache) {
-        if (cacheItem->object() && cacheItem->attached)
+        if (cacheItem->attached)
             cacheItem->attached->emitChanges();
     }
 }
@@ -1406,7 +1403,7 @@ void QQuickVisualDataModel::_q_layoutChanged()
 QQuickVisualDataModelAttached *QQuickVisualDataModel::qmlAttachedProperties(QObject *obj)
 {
     if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(obj)) {
-        if (cacheItem->object() == obj) { // Don't create attached item for child objects.
+        if (cacheItem->object == obj) { // Don't create attached item for child objects.
             cacheItem->attached = new QQuickVisualDataModelAttached(cacheItem, obj);
             return cacheItem->attached;
         }
@@ -1678,6 +1675,7 @@ QQuickVisualDataModelItem::QQuickVisualDataModelItem(
     : QV8ObjectResource(metaType->v8Engine)
     , metaType(metaType)
     , contextData(0)
+    , object(0)
     , attached(0)
     , incubationTask(0)
     , objectRef(0)
@@ -1692,7 +1690,7 @@ QQuickVisualDataModelItem::~QQuickVisualDataModelItem()
 {
     Q_ASSERT(scriptRef == 0);
     Q_ASSERT(objectRef == 0);
-    Q_ASSERT(!object());
+    Q_ASSERT(!object);
     Q_ASSERT(indexHandle.IsEmpty());
     Q_ASSERT(modelHandle.IsEmpty());
 
@@ -1749,18 +1747,15 @@ void QQuickVisualDataModelItem::incubateObject(
 
 void QQuickVisualDataModelItem::destroyObject()
 {
-    QObject * const obj = object();
-    setObject(0);
-
-    Q_ASSERT(obj);
+    Q_ASSERT(object);
     Q_ASSERT(contextData);
 
-    QObjectPrivate *p = QObjectPrivate::get(obj);
+    QObjectPrivate *p = QObjectPrivate::get(object);
     Q_ASSERT(p->declarativeData);
     QQmlData *data = static_cast<QQmlData*>(p->declarativeData);
     if (data->ownContext && data->context)
         data->context->clearContext();
-    obj->deleteLater();
+    object->deleteLater();
 
     if (attached) {
         attached->m_cacheItem = 0;
@@ -1769,6 +1764,7 @@ void QQuickVisualDataModelItem::destroyObject()
 
     contextData->destroy();
     contextData = 0;
+    object = 0;
 }
 
 QQuickVisualDataModelItem *QQuickVisualDataModelItem::dataForObject(QObject *object)
@@ -1786,25 +1782,6 @@ QQuickVisualDataModelItem *QQuickVisualDataModelItem::dataForObject(QObject *obj
     return 0;
 }
 
-void QQuickVisualDataModelItem::objectDestroyed(QObject *)
-{
-    const bool contextValid = contextData->isValid();
-    contextData->destroy();
-    contextData = 0;
-    attached = 0;
-    objectRef = 0;
-
-    if (contextValid) {
-        Dispose();
-    } else {
-        // The parent context was invalidated, meaning the visual data model is about to be
-        // destroyed.  So we'll just decrement the script ref here and let the vdm destructor
-        // destroy the object if necessary, rather than Dispose of the object and unnecessarily
-        // remove each item from the soon to be destroyed cache list individually.
-        scriptRef -= 1;
-    }
-}
-
 //---------------------------------------------------------------------------
 
 QQuickVisualDataModelAttachedMetaObject::QQuickVisualDataModelAttachedMetaObject(
@@ -2841,7 +2818,6 @@ QQuickVisualModel::ReleaseFlags QQuickVisualPartsModel::release(QQuickItem *item
     QHash<QObject *, QQuickPackage *>::iterator it = m_packaged.find(item);
     if (it != m_packaged.end()) {
         QQuickPackage *package = *it;
-        QQml_setParent_noEvent(item, package);
         QQuickVisualDataModelPrivate *model = QQuickVisualDataModelPrivate::get(m_model);
         flags = model->release(package);
         m_packaged.erase(it);
@@ -2893,7 +2869,6 @@ void QQuickVisualPartsModel::destroyingPackage(QQuickPackage *package)
         Q_ASSERT(!m_packaged.contains(item));
         emit destroyingItem(item);
         item->setParentItem(0);
-        QQml_setParent_noEvent(item, package);
     }
 }
 

diff --git a/src/quick/items/qquickvisualdatamodel_p_p.h b/src/quick/items/qquickvisualdatamodel_p_p.h
index e2887e7..ad5aa5e 100644 (file)
--- a/src/quick/items/qquickvisualdatamodel_p_p.h
+++ b/src/quick/items/qquickvisualdatamodel_p_p.h
@@ -103,7 +103,7 @@ public:
 class QQuickVisualAdaptorModel;
 class QVDMIncubationTask;
 
-class QQuickVisualDataModelItem : public QObject, public QV8ObjectResource, public QQmlGuard<QObject>
+class QQuickVisualDataModelItem : public QObject, public QV8ObjectResource
 {
     Q_OBJECT
     Q_PROPERTY(int index READ modelIndex NOTIFY modelIndexChanged)
@@ -146,6 +146,7 @@ public:
 
     QQuickVisualDataModelItemMetaType * const metaType;
     QQmlContextData *contextData;
+    QObject *object;
     QQuickVisualDataModelAttached *attached;
     QVDMIncubationTask *incubationTask;
     v8::Persistent<v8::Object> indexHandle;

diff --git a/src/quick/items/qquickvisualitemmodel.cpp b/src/quick/items/qquickvisualitemmodel.cpp
index df274e1..1cefcb4 100644 (file)
--- a/src/quick/items/qquickvisualitemmodel.cpp
+++ b/src/quick/items/qquickvisualitemmodel.cpp
@@ -65,7 +65,6 @@ public:
     QQuickVisualItemModelPrivate() : QObjectPrivate() {}
 
     static void children_append(QQmlListProperty<QQuickItem> *prop, QQuickItem *item) {
-        QQml_setParent_noEvent(item, prop->object);
         static_cast<QQuickVisualItemModelPrivate *>(prop->data)->children.append(Item(item));
         static_cast<QQuickVisualItemModelPrivate *>(prop->data)->itemAppended();
         static_cast<QQuickVisualItemModelPrivate *>(prop->data)->emitChildrenChanged();
@@ -206,7 +205,6 @@ QQuickVisualModel::ReleaseFlags QQuickVisualItemModel::release(QQuickItem *item)
         if (d->children[idx].deref()) {
             // XXX todo - the original did item->scene()->removeItem().  Why?
             item->setParentItem(0);
-            QQml_setParent_noEvent(item, this);
         }
     }
     return 0;

diff --git a/src/quick/util/qquickpackage.cpp b/src/quick/util/qquickpackage.cpp
index 94e4fe5..6c95989 100644 (file)
--- a/src/quick/util/qquickpackage.cpp
+++ b/src/quick/util/qquickpackage.cpp
@@ -145,11 +145,6 @@ QQuickPackage::QQuickPackage(QObject *parent)
 
 QQuickPackage::~QQuickPackage()
 {
-    Q_D(QQuickPackage);
-    for (int ii = 0; ii < d->dataList.count(); ++ii) {
-        QObject *obj = d->dataList.at(ii);
-        obj->setParent(this);
-    }
 }
 
 QQmlListProperty<QObject> QQuickPackage::data()

diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
index 828a102..04cc79d 100644 (file)
--- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
+++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
@@ -4264,7 +4264,7 @@ void tst_QQuickGridView::creationContext()
     QVERIFY(rootItem->property("count").toInt() > 0);
 
     QQuickItem *item;
-    QVERIFY(item = rootItem->findChild<QQuickItem *>("listItem"));
+    QVERIFY(item = findItem<QQuickItem>(rootItem, "listItem"));
     QCOMPARE(item->property("text").toString(), QString("Hello!"));
     QVERIFY(item = rootItem->findChild<QQuickItem *>("header"));
     QCOMPARE(item->property("text").toString(), QString("Hello!"));

diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
index f24f91c..2a67405 100644 (file)
--- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
@@ -5002,7 +5002,7 @@ void tst_QQuickListView::creationContext()
     QVERIFY(rootItem->property("count").toInt() > 0);
 
     QQuickItem *item;
-    QVERIFY(item = rootItem->findChild<QQuickItem *>("listItem"));
+    QVERIFY(item = findItem<QQuickItem>(rootItem, "listItem"));
     QCOMPARE(item->property("text").toString(), QString("Hello!"));
     QVERIFY(item = rootItem->findChild<QQuickItem *>("header"));
     QCOMPARE(item->property("text").toString(), QString("Hello!"));

diff --git a/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp b/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp
index ee0988f..920db00 100644 (file)
--- a/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp
+++ b/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp
@@ -1480,13 +1480,13 @@ void tst_qquickpositioners::test_repeater()
 {
     QQuickView *canvas = createView(testFile("repeatertest.qml"));
 
-    QQuickRectangle *one = canvas->rootObject()->findChild<QQuickRectangle*>("one");
+    QQuickRectangle *one = findItem<QQuickRectangle>(canvas->rootItem(), "one");
     QVERIFY(one != 0);
 
-    QQuickRectangle *two = canvas->rootObject()->findChild<QQuickRectangle*>("two");
+    QQuickRectangle *two = findItem<QQuickRectangle>(canvas->rootItem(), "two");
     QVERIFY(two != 0);
 
-    QQuickRectangle *three = canvas->rootObject()->findChild<QQuickRectangle*>("three");
+    QQuickRectangle *three = findItem<QQuickRectangle>(canvas->rootItem(), "three");
     QVERIFY(three != 0);
 
     QCOMPARE(one->x(), 0.0);

diff --git a/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp b/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
index 02b1618..50c1e35 100644 (file)
--- a/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
+++ b/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
@@ -3452,7 +3452,7 @@ void tst_qquickvisualdatamodel::invalidAttachment()
     QCOMPARE(property.userType(), qMetaTypeId<QQuickVisualDataModel *>());
     QVERIFY(!property.value<QQuickVisualDataModel *>());
 
-    QQuickItem *item = object->findChild<QQuickItem *>("delegate");
+    QQuickItem *item = findItem<QQuickItem>(static_cast<QQuickItem *>(object.data()), "delegate");
     QVERIFY(item);
 
     property = item->property("validVdm");