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 <bea.lam@nokia.com>

diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp
index dd7a3fa..1fab4a8 100644 (file)
--- 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<QQuickGridViewAttached*>(qmlAttachedPropertiesObject<QQuickGridView>(item));
         if (attached)
             static_cast<QQuickGridViewAttached*>(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 5089d23..50a3216 100644 (file)
--- 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 dfc0a8b..7516761 100644 (file)
--- 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 @@ public:
     int index;
     bool ownItem;
     bool releaseAfterTransition;
+    bool trackGeom;
 };
 
 
@@ -124,6 +125,8 @@ public:
     QQuickItemViewPrivate();
     ~QQuickItemViewPrivate();
 
+    static inline QQuickItemViewPrivate *get(QQuickItemView *o) { return o->d_func(); }
+
     struct ChangeResult {
         QQmlNullableValue<qreal> visiblePos;
         bool changedFirstItem;
@@ -191,7 +194,7 @@ public:
     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 9d9e3ae..6f33545 100644 (file)
--- 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<QQuickListViewAttached*>(qmlAttachedPropertiesObject<QQuickListView>(item));
         if (attached)
             static_cast<QQuickListViewAttached*>(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<QQuickListViewAttached*>(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<FxListItemSG*>(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 (file)
index 0000000..322cfed
--- /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 a282d55..88ed94d 100644 (file)
--- 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<QQuickGridView*>(canvas->rootObject());
+        QTRY_VERIFY(gridview != 0);
+
+        QQuickItem *contentItem = gridview->contentItem();
+        QTRY_VERIFY(contentItem != 0);
+
+        QQuickItem *header = findItem<QQuickItem>(contentItem, "header");
+        QVERIFY(header);
+        QCOMPARE(header->y(), -header->height());
+
+        QQuickItem *footer = findItem<QQuickItem>(contentItem, "footer");
+        QVERIFY(footer);
+        QCOMPARE(footer->y(), 0.);
+
+        QCOMPARE(static_cast<GVAccessor*>(gridview)->minY(), header->height());
+        QCOMPARE(static_cast<GVAccessor*>(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<QQuickGridView*>(canvas->rootObject());
+        QTRY_VERIFY(gridview != 0);
+
+        QQuickItem *contentItem = gridview->contentItem();
+        QTRY_VERIFY(contentItem != 0);
+
+        QQuickItem *header = findItem<QQuickItem>(contentItem, "header");
+        QVERIFY(header);
+        QCOMPARE(header->x(), -header->width());
+
+        QQuickItem *footer = findItem<QQuickItem>(contentItem, "footer");
+        QVERIFY(footer);
+        QCOMPARE(footer->x(), 0.);
+
+        QCOMPARE(static_cast<GVAccessor*>(gridview)->minX(), header->width());
+        QCOMPARE(static_cast<GVAccessor*>(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<QQuickGridView*>(canvas->rootObject());
+        QTRY_VERIFY(gridview != 0);
+
+        QQuickItem *contentItem = gridview->contentItem();
+        QTRY_VERIFY(contentItem != 0);
+
+        QQuickItem *header = findItem<QQuickItem>(contentItem, "header");
+        QVERIFY(header);
+        QCOMPARE(header->x(), 0.);
+
+        QQuickItem *footer = findItem<QQuickItem>(contentItem, "footer");
+        QVERIFY(footer);
+        QCOMPARE(footer->x(), -footer->width());
+
+        QCOMPARE(static_cast<GVAccessor*>(gridview)->minX(), 240. - header->width());
+        QCOMPARE(static_cast<GVAccessor*>(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<QQuickGridView*>(canvas->rootObject());
+        QTRY_VERIFY(gridview != 0);
+
+        QQuickItem *contentItem = gridview->contentItem();
+        QTRY_VERIFY(contentItem != 0);
+
+        QQuickItem *header = findItem<QQuickItem>(contentItem, "header");
+        QVERIFY(header);
+        QCOMPARE(header->y(), -header->height());
+
+        QQuickItem *footer = findItem<QQuickItem>(contentItem, "footer");
+        QVERIFY(footer);
+        QCOMPARE(footer->y(), 80.*2);
+
+        model.reset();
+
+        header = findItem<QQuickItem>(contentItem, "header");
+        QVERIFY(header);
+        QCOMPARE(header->y(), -header->height());
+
+        footer = findItem<QQuickItem>(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 8e8463d..4c3eeca 100644 (file)
--- 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 335fc3c..202f516 100644 (file)
--- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
@@ -3529,6 +3529,45 @@ void tst_QQuickListView::headerFooter()
 
         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<QQuickListView*>(canvas->rootObject());
+        QTRY_VERIFY(listview != 0);
+
+        QQuickItem *contentItem = listview->contentItem();
+        QTRY_VERIFY(contentItem != 0);
+
+        QQuickItem *header = findItem<QQuickItem>(contentItem, "header");
+        QVERIFY(header);
+        QCOMPARE(header->y(), -header->height());
+
+        QQuickItem *footer = findItem<QQuickItem>(contentItem, "footer");
+        QVERIFY(footer);
+        QCOMPARE(footer->y(), 30.*4);
+
+        model.reset();
+
+        header = findItem<QQuickItem>(contentItem, "header");
+        QVERIFY(header);
+        QCOMPARE(header->y(), -header->height());
+
+        footer = findItem<QQuickItem>(contentItem, "footer");
+        QVERIFY(footer);
+        QCOMPARE(footer->y(), 30.*4);
+
+        delete canvas;
+    }
 }
 
 void tst_QQuickListView::resizeView()