From 6deb3ceffa209d710570ffeb361e95c988e6e7cd Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Fri, 16 Mar 2012 18:45:39 +1000 Subject: [PATCH] Resetting a model can cause a crash in views with header/footer. Geometry listeners were called for deleted header/footer. Change-Id: I47854178232f8a4ab5e19a931901b49741fec388 Reviewed-by: Bea Lam --- src/quick/items/qquickgridview.cpp | 25 ++- src/quick/items/qquickitemview.cpp | 11 +- src/quick/items/qquickitemview_p_p.h | 7 +- src/quick/items/qquicklistview.cpp | 25 ++- .../qquickgridview/data/headerfooter.qml | 31 ++++ .../qquickgridview/tst_qquickgridview.cpp | 146 ++++++++++++++++++ .../qquicklistview/data/headerfooter.qml | 2 + .../qquicklistview/tst_qquicklistview.cpp | 39 +++++ 8 files changed, 261 insertions(+), 25 deletions(-) create mode 100644 tests/auto/quick/qquickgridview/data/headerfooter.qml diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index dd7a3faa6f..1fab4a8c99 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -66,13 +66,22 @@ QT_BEGIN_NAMESPACE class FxGridItemSG : public FxViewItem { public: - FxGridItemSG(QQuickItem *i, QQuickGridView *v, bool own) : FxViewItem(i, own), view(v) { + FxGridItemSG(QQuickItem *i, QQuickGridView *v, bool own, bool trackGeometry) : FxViewItem(i, own, trackGeometry), view(v) { attached = static_cast(qmlAttachedPropertiesObject(item)); if (attached) static_cast(attached)->setView(view); + if (trackGeometry) { + QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); + itemPrivate->addItemChangeListener(QQuickItemViewPrivate::get(view), QQuickItemPrivate::Geometry); + } } - ~FxGridItemSG() {} + ~FxGridItemSG() { + if (trackGeom) { + QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); + itemPrivate->removeItemChangeListener(QQuickItemViewPrivate::get(view), QQuickItemPrivate::Geometry); + } + } qreal position() const { return rowPos(); @@ -423,7 +432,7 @@ FxViewItem *QQuickGridViewPrivate::newViewItem(int modelIndex, QQuickItem *item) { Q_Q(QQuickGridView); Q_UNUSED(modelIndex); - return new FxGridItemSG(item, q, false); + return new FxGridItemSG(item, q, false, false); } void QQuickGridViewPrivate::initializeViewItem(FxViewItem *item) @@ -685,7 +694,7 @@ void QQuickGridViewPrivate::createHighlight() if (currentItem) { QQuickItem *item = createHighlightItem(); if (item) { - FxGridItemSG *newHighlight = new FxGridItemSG(item, q, true); + FxGridItemSG *newHighlight = new FxGridItemSG(item, q, true, true); if (autoHighlight) resetHighlightPosition(); highlightXAnimator = new QSmoothedAnimation; @@ -760,11 +769,11 @@ void QQuickGridViewPrivate::updateFooter() Q_Q(QQuickGridView); bool created = false; if (!footer) { - QQuickItem *item = createComponentItem(footerComponent, true); + QQuickItem *item = createComponentItem(footerComponent); if (!item) return; item->setZ(1); - footer = new FxGridItemSG(item, q, true); + footer = new FxGridItemSG(item, q, true, true); created = true; } @@ -799,11 +808,11 @@ void QQuickGridViewPrivate::updateHeader() Q_Q(QQuickGridView); bool created = false; if (!header) { - QQuickItem *item = createComponentItem(headerComponent, true); + QQuickItem *item = createComponentItem(headerComponent); if (!item) return; item->setZ(1); - header = new FxGridItemSG(item, q, true); + header = new FxGridItemSG(item, q, true, true); created = true; } diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 5089d230d7..50a3216bf0 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -45,11 +45,12 @@ QT_BEGIN_NAMESPACE -FxViewItem::FxViewItem(QQuickItem *i, bool own) +FxViewItem::FxViewItem(QQuickItem *i, bool own, bool trackGeometry) : item(i) , transitionableItem(0) , ownItem(own) , releaseAfterTransition(false) + , trackGeom(trackGeometry) { } @@ -2195,10 +2196,10 @@ bool QQuickItemViewPrivate::releaseItem(FxViewItem *item) QQuickItem *QQuickItemViewPrivate::createHighlightItem() { - return createComponentItem(highlightComponent, true, true); + return createComponentItem(highlightComponent, true); } -QQuickItem *QQuickItemViewPrivate::createComponentItem(QQmlComponent *component, bool receiveItemGeometryChanges, bool createDefault) +QQuickItem *QQuickItemViewPrivate::createComponentItem(QQmlComponent *component, bool createDefault) { Q_Q(QQuickItemView); @@ -2222,10 +2223,6 @@ QQuickItem *QQuickItemViewPrivate::createComponentItem(QQmlComponent *component, if (item) { QQml_setParent_noEvent(item, q->contentItem()); item->setParentItem(q->contentItem()); - if (receiveItemGeometryChanges) { - QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); - itemPrivate->addItemChangeListener(this, QQuickItemPrivate::Geometry); - } } return item; } diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h index dfc0a8bc7e..7516761ee8 100644 --- a/src/quick/items/qquickitemview_p_p.h +++ b/src/quick/items/qquickitemview_p_p.h @@ -60,7 +60,7 @@ QT_MODULE(Quick) class FxViewItem { public: - FxViewItem(QQuickItem *, bool own); + FxViewItem(QQuickItem *, bool own, bool trackGeometry); virtual ~FxViewItem(); qreal itemX() const; @@ -92,6 +92,7 @@ class FxViewItem int index; bool ownItem; bool releaseAfterTransition; + bool trackGeom; }; @@ -124,6 +125,8 @@ class QQuickItemViewPrivate : public QQuickFlickablePrivate, public QQuickItemVi QQuickItemViewPrivate(); ~QQuickItemViewPrivate(); + static inline QQuickItemViewPrivate *get(QQuickItemView *o) { return o->d_func(); } + struct ChangeResult { QQmlNullableValue visiblePos; bool changedFirstItem; @@ -191,7 +194,7 @@ class QQuickItemViewPrivate : public QQuickFlickablePrivate, public QQuickItemVi virtual bool releaseItem(FxViewItem *item); QQuickItem *createHighlightItem(); - QQuickItem *createComponentItem(QQmlComponent *component, bool receiveItemGeometryChanges, bool createDefault = false); + QQuickItem *createComponentItem(QQmlComponent *component, bool createDefault = false); void updateCurrent(int modelIndex); void updateTrackedItem(); diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp index 9d9e3ae186..6f33545185 100644 --- a/src/quick/items/qquicklistview.cpp +++ b/src/quick/items/qquicklistview.cpp @@ -236,13 +236,22 @@ void QQuickViewSection::setLabelPositioning(int l) class FxListItemSG : public FxViewItem { public: - FxListItemSG(QQuickItem *i, QQuickListView *v, bool own) : FxViewItem(i, own), view(v) { + FxListItemSG(QQuickItem *i, QQuickListView *v, bool own, bool trackGeometry) : FxViewItem(i, own, trackGeometry), view(v) { attached = static_cast(qmlAttachedPropertiesObject(item)); if (attached) static_cast(attached)->setView(view); + if (trackGeometry) { + QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); + itemPrivate->addItemChangeListener(QQuickItemViewPrivate::get(view), QQuickItemPrivate::Geometry); + } } - ~FxListItemSG() {} + ~FxListItemSG() { + if (trackGeom) { + QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); + itemPrivate->removeItemChangeListener(QQuickItemViewPrivate::get(view), QQuickItemPrivate::Geometry); + } + } inline QQuickItem *section() const { return attached ? static_cast(attached)->m_sectionItem : 0; @@ -536,7 +545,7 @@ FxViewItem *QQuickListViewPrivate::newViewItem(int modelIndex, QQuickItem *item) { Q_Q(QQuickListView); - FxListItemSG *listItem = new FxListItemSG(item, q, false); + FxListItemSG *listItem = new FxListItemSG(item, q, false, false); listItem->index = modelIndex; // initialise attached properties @@ -835,7 +844,7 @@ void QQuickListViewPrivate::createHighlight() if (currentItem) { QQuickItem *item = createHighlightItem(); if (item) { - FxListItemSG *newHighlight = new FxListItemSG(item, q, true); + FxListItemSG *newHighlight = new FxListItemSG(item, q, true, true); if (autoHighlight) { newHighlight->setSize(static_cast(currentItem)->itemSize()); @@ -1238,11 +1247,11 @@ void QQuickListViewPrivate::updateFooter() Q_Q(QQuickListView); bool created = false; if (!footer) { - QQuickItem *item = createComponentItem(footerComponent, true); + QQuickItem *item = createComponentItem(footerComponent); if (!item) return; item->setZ(1); - footer = new FxListItemSG(item, q, true); + footer = new FxListItemSG(item, q, true, true); created = true; } @@ -1269,11 +1278,11 @@ void QQuickListViewPrivate::updateHeader() Q_Q(QQuickListView); bool created = false; if (!header) { - QQuickItem *item = createComponentItem(headerComponent, true); + QQuickItem *item = createComponentItem(headerComponent); if (!item) return; item->setZ(1); - header = new FxListItemSG(item, q, true); + header = new FxListItemSG(item, q, true, true); created = true; } diff --git a/tests/auto/quick/qquickgridview/data/headerfooter.qml b/tests/auto/quick/qquickgridview/data/headerfooter.qml new file mode 100644 index 0000000000..322cfed388 --- /dev/null +++ b/tests/auto/quick/qquickgridview/data/headerfooter.qml @@ -0,0 +1,31 @@ +import QtQuick 2.0 + +GridView { + id: view + property bool horizontal: false + property bool rtl: false + width: 240 + height: 320 + + model: testModel + + flow: horizontal ? GridView.TopToBottom : GridView.LeftToRight + header: Rectangle { + objectName: "header" + width: horizontal ? 20 : view.width + height: horizontal ? view.height : 20 + color: "red" + } + footer: Rectangle { + objectName: "footer" + width: horizontal ? 30 : view.width + height: horizontal ? view.height : 30 + color: "blue" + } + + cellWidth: 80; + cellHeight: 80; + + delegate: Text { width: 80; height: 80; text: index + "(" + x + ")" } + layoutDirection: rtl ? Qt.RightToLeft : Qt.LeftToRight +} diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index a282d55ac8..88ed94d8bc 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -113,6 +113,7 @@ private slots: void footer_data(); void header(); void header_data(); + void headerFooter(); void resizeViewAndRepaint(); void changeColumnCount(); void indexAt_itemAt_data(); @@ -3144,6 +3145,151 @@ void tst_QQuickGridView::header_data() << QPointF(-(240 - 40), 0); } +class GVAccessor : public QQuickGridView +{ +public: + qreal minY() const { return minYExtent(); } + qreal maxY() const { return maxYExtent(); } + qreal minX() const { return minXExtent(); } + qreal maxX() const { return maxXExtent(); } +}; + +void tst_QQuickGridView::headerFooter() +{ + { + // Vertical + QQuickView *canvas = createView(); + + QmlListModel model; + QQmlContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + canvas->setSource(testFileUrl("headerfooter.qml")); + qApp->processEvents(); + + QQuickGridView *gridview = qobject_cast(canvas->rootObject()); + QTRY_VERIFY(gridview != 0); + + QQuickItem *contentItem = gridview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + QQuickItem *header = findItem(contentItem, "header"); + QVERIFY(header); + QCOMPARE(header->y(), -header->height()); + + QQuickItem *footer = findItem(contentItem, "footer"); + QVERIFY(footer); + QCOMPARE(footer->y(), 0.); + + QCOMPARE(static_cast(gridview)->minY(), header->height()); + QCOMPARE(static_cast(gridview)->maxY(), header->height()); + + delete canvas; + } + { + // Horizontal + QQuickView *canvas = createView(); + + QmlListModel model; + QQmlContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + canvas->setSource(testFileUrl("headerfooter.qml")); + canvas->rootObject()->setProperty("horizontal", true); + qApp->processEvents(); + + QQuickGridView *gridview = qobject_cast(canvas->rootObject()); + QTRY_VERIFY(gridview != 0); + + QQuickItem *contentItem = gridview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + QQuickItem *header = findItem(contentItem, "header"); + QVERIFY(header); + QCOMPARE(header->x(), -header->width()); + + QQuickItem *footer = findItem(contentItem, "footer"); + QVERIFY(footer); + QCOMPARE(footer->x(), 0.); + + QCOMPARE(static_cast(gridview)->minX(), header->width()); + QCOMPARE(static_cast(gridview)->maxX(), header->width()); + + delete canvas; + } + { + // Horizontal RTL + QQuickView *canvas = createView(); + + QmlListModel model; + QQmlContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + canvas->setSource(testFileUrl("headerfooter.qml")); + canvas->rootObject()->setProperty("horizontal", true); + canvas->rootObject()->setProperty("rtl", true); + qApp->processEvents(); + + QQuickGridView *gridview = qobject_cast(canvas->rootObject()); + QTRY_VERIFY(gridview != 0); + + QQuickItem *contentItem = gridview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + QQuickItem *header = findItem(contentItem, "header"); + QVERIFY(header); + QCOMPARE(header->x(), 0.); + + QQuickItem *footer = findItem(contentItem, "footer"); + QVERIFY(footer); + QCOMPARE(footer->x(), -footer->width()); + + QCOMPARE(static_cast(gridview)->minX(), 240. - header->width()); + QCOMPARE(static_cast(gridview)->maxX(), 240. - header->width()); + + delete canvas; + } + { + // Reset model + QQuickView *canvas = createView(); + + QaimModel model; + for (int i = 0; i < 6; i++) + model.addItem("Item" + QString::number(i), ""); + QQmlContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + canvas->setSource(testFileUrl("headerfooter.qml")); + qApp->processEvents(); + + QQuickGridView *gridview = qobject_cast(canvas->rootObject()); + QTRY_VERIFY(gridview != 0); + + QQuickItem *contentItem = gridview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + QQuickItem *header = findItem(contentItem, "header"); + QVERIFY(header); + QCOMPARE(header->y(), -header->height()); + + QQuickItem *footer = findItem(contentItem, "footer"); + QVERIFY(footer); + QCOMPARE(footer->y(), 80.*2); + + model.reset(); + + header = findItem(contentItem, "header"); + QVERIFY(header); + QCOMPARE(header->y(), -header->height()); + + footer = findItem(contentItem, "footer"); + QVERIFY(footer); + QCOMPARE(footer->y(), 80.*2); + + delete canvas; + } +} + void tst_QQuickGridView::resizeViewAndRepaint() { QQuickView *canvas = createView(); diff --git a/tests/auto/quick/qquicklistview/data/headerfooter.qml b/tests/auto/quick/qquicklistview/data/headerfooter.qml index 8e8463d645..4c3eeca328 100644 --- a/tests/auto/quick/qquicklistview/data/headerfooter.qml +++ b/tests/auto/quick/qquicklistview/data/headerfooter.qml @@ -6,6 +6,8 @@ ListView { property bool rtl: false width: 240 height: 320 + + model: testModel orientation: horizontal ? ListView.Horizontal : ListView.Vertical header: Rectangle { diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp index 335fc3c6e2..202f5164af 100644 --- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp @@ -3527,6 +3527,45 @@ void tst_QQuickListView::headerFooter() QCOMPARE(static_cast(listview)->minX(), 240. - header->width()); QCOMPARE(static_cast(listview)->maxX(), 240. - header->width()); + delete canvas; + } + { + // Reset model + QQuickView *canvas = createView(); + + QaimModel model; + for (int i = 0; i < 4; i++) + model.addItem("Item" + QString::number(i), ""); + QQmlContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + canvas->setSource(testFileUrl("headerfooter.qml")); + qApp->processEvents(); + + QQuickListView *listview = qobject_cast(canvas->rootObject()); + QTRY_VERIFY(listview != 0); + + QQuickItem *contentItem = listview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + QQuickItem *header = findItem(contentItem, "header"); + QVERIFY(header); + QCOMPARE(header->y(), -header->height()); + + QQuickItem *footer = findItem(contentItem, "footer"); + QVERIFY(footer); + QCOMPARE(footer->y(), 30.*4); + + model.reset(); + + header = findItem(contentItem, "header"); + QVERIFY(header); + QCOMPARE(header->y(), -header->height()); + + footer = findItem(contentItem, "footer"); + QVERIFY(footer); + QCOMPARE(footer->y(), 30.*4); + delete canvas; } }