From: Andrew den Exter Date: Fri, 25 May 2012 02:32:28 +0000 (+1000) Subject: Don't parent (QObject) delegate items to views. X-Git-Url: http://git.silmor.de/gitweb/?a=commitdiff_plain;h=d4dad3a0b83c921ed4d746eb9694be8b008bcc1a;p=konrad%2Fqtdeclarative.git 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 --- diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 88a8a30..da661af 100644 --- 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 --- 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 --- 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 --- 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(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(cacheItem->object())) + if (QQuickPackage *package = qmlobject_cast(cacheItem->object)) emitCreatedPackage(cacheItem, package); - else if (QQuickItem *item = qobject_cast(cacheItem->object())) + else if (QQuickItem *item = qobject_cast(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(cacheItem->object())) + if (QQuickPackage *package = qmlobject_cast(cacheItem->object)) emitInitPackage(cacheItem, package); - else if (QQuickItem *item = qmlobject_cast(cacheItem->object())) + else if (QQuickItem *item = qmlobject_cast(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(object)) emitDestroyingPackage(package); else if (QQuickItem *item = qmlobject_cast(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(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::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 --- 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 +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 indexHandle; diff --git a/src/quick/items/qquickvisualitemmodel.cpp b/src/quick/items/qquickvisualitemmodel.cpp index df274e1..1cefcb4 100644 --- a/src/quick/items/qquickvisualitemmodel.cpp +++ b/src/quick/items/qquickvisualitemmodel.cpp @@ -65,7 +65,6 @@ public: QQuickVisualItemModelPrivate() : QObjectPrivate() {} static void children_append(QQmlListProperty *prop, QQuickItem *item) { - QQml_setParent_noEvent(item, prop->object); static_cast(prop->data)->children.append(Item(item)); static_cast(prop->data)->itemAppended(); static_cast(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 --- 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 QQuickPackage::data() diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index 828a102..04cc79d 100644 --- 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("listItem")); + QVERIFY(item = findItem(rootItem, "listItem")); QCOMPARE(item->property("text").toString(), QString("Hello!")); QVERIFY(item = rootItem->findChild("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 --- 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("listItem")); + QVERIFY(item = findItem(rootItem, "listItem")); QCOMPARE(item->property("text").toString(), QString("Hello!")); QVERIFY(item = rootItem->findChild("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 --- 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("one"); + QQuickRectangle *one = findItem(canvas->rootItem(), "one"); QVERIFY(one != 0); - QQuickRectangle *two = canvas->rootObject()->findChild("two"); + QQuickRectangle *two = findItem(canvas->rootItem(), "two"); QVERIFY(two != 0); - QQuickRectangle *three = canvas->rootObject()->findChild("three"); + QQuickRectangle *three = findItem(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 --- 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()); QVERIFY(!property.value()); - QQuickItem *item = object->findChild("delegate"); + QQuickItem *item = findItem(static_cast(object.data()), "delegate"); QVERIFY(item); property = item->property("validVdm");