ListView can freeze if flicked beyond its bounds.

If the delegate's size changes in componentComplete and all
items are flicked out of view, an incorrect jump calculation
in addVisibleItems() resulted in a new delegate being created
in the wrong position, and retriggering the jump calculation,
which resulted in a new delegate being created in the wrong
position, and retriggering the jump...

Also fixed currentItem visibility.

Change-Id: Iad5f211c4fc5eed9c009d51a0ce3b58181a7b36e
Reviewed-by: Bea Lam <bea.lam@nokia.com>

diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp
index 45fbc9f..a78ab4c 100644 (file)
--- a/src/quick/items/qquickgridview.cpp
+++ b/src/quick/items/qquickgridview.cpp
@@ -1884,6 +1884,10 @@ void QQuickGridView::viewportMoved()
         FxGridItemSG *item = static_cast<FxGridItemSG*>(d->visibleItems.at(i));
         item->item->setVisible(item->rowPos() + d->rowSize() >= from && item->rowPos() <= to);
     }
+    if (d->currentItem) {
+        FxGridItemSG *item = static_cast<FxGridItemSG*>(d->currentItem);
+        item->item->setVisible(item->rowPos() + d->rowSize() >= from && item->rowPos() <= to);
+    }
 
     if (d->hData.flicking || d->vData.flicking || d->hData.moving || d->vData.moving)
         d->moveReason = QQuickGridViewPrivate::Mouse;

diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp
index 5e9a685..a2f687c 100644 (file)
--- a/src/quick/items/qquicklistview.cpp
+++ b/src/quick/items/qquicklistview.cpp
@@ -615,20 +615,17 @@ bool QQuickListViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d
         // We've jumped more than a page.  Estimate which items are now
         // visible and fill from there.
         int count = (fillFrom - itemEnd) / (averageSize + spacing);
-        for (int i = 0; i < visibleItems.count(); ++i)
-            releaseItem(visibleItems.at(i));
-        visibleItems.clear();
-        modelIndex += count;
-        if (modelIndex >= model->count()) {
-            count -= modelIndex - model->count() + 1;
-            modelIndex = model->count() - 1;
-        } else if (modelIndex < 0) {
-            count -= modelIndex;
-            modelIndex = 0;
-        }
-        visibleIndex = modelIndex;
-        visiblePos = itemEnd + count * (averageSize + spacing);
-        itemEnd = visiblePos;
+        int newModelIdx = qBound(0, modelIndex + count, model->count());
+        count = newModelIdx - modelIndex;
+        if (count) {
+            for (int i = 0; i < visibleItems.count(); ++i)
+                releaseItem(visibleItems.at(i));
+            visibleItems.clear();
+            modelIndex = newModelIdx;
+            visibleIndex = modelIndex;
+            visiblePos = itemEnd + count * (averageSize + spacing);
+            itemEnd = visiblePos;
+        }
     }
 
     bool changed = false;
@@ -2553,6 +2550,8 @@ void QQuickListView::viewportMoved()
         FxViewItem *item = static_cast<FxListItemSG*>(d->visibleItems.at(i));
         item->item->setVisible(item->endPosition() >= from && item->position() <= to);
     }
+    if (d->currentItem)
+        d->currentItem->item->setVisible(d->currentItem->endPosition() >= from && d->currentItem->position() <= to);
 
     if (d->hData.flicking || d->vData.flicking || d->hData.moving || d->vData.moving)
         d->moveReason = QQuickListViewPrivate::Mouse;

diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
index 0171872..66c98cd 100644 (file)
--- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
+++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
@@ -1773,6 +1773,12 @@ void tst_QQuickGridView::currentIndex()
     QVERIFY(!gridview->highlightItem());
     QVERIFY(!gridview->currentItem());
 
+    // moving currentItem out of view should make it invisible
+    gridview->setCurrentIndex(0);
+    QTRY_VERIFY(gridview->currentItem()->isVisible());
+    gridview->setContentY(200);
+    QTRY_VERIFY(!gridview->currentItem()->isVisible());
+
     delete canvas;
 }
 

diff --git a/tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml b/tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml
new file mode 100644 (file)
index 0000000..0a1b1a1
--- /dev/null
+++ b/tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml
@@ -0,0 +1,43 @@
+import QtQuick 2.0
+
+Rectangle {
+    id: root
+    width: 240
+    height: 320
+    color: "#ffffff"
+
+    Component {
+        id: myDelegate
+        Rectangle {
+            id: wrapper
+            objectName: "wrapper"
+            height: column.height
+            Column {
+                id: column
+                Text {
+                    text: "index: " + index + ", delegate A"
+                    Component.onCompleted: height = index % 2 ? 30 : 20
+                }
+                Text {
+                    x: 200
+                    text: wrapper.y
+                    height: 25
+                }
+            }
+            color: ListView.isCurrentItem ? "lightsteelblue" : "#EEEEEE"
+        }
+    }
+    ListView {
+        id: list
+        objectName: "list"
+        focus: true
+        width: 240
+        height: 320
+        model: 2
+        delegate: myDelegate
+        highlightMoveSpeed: 1000
+        highlightResizeSpeed: 1000
+        cacheBuffer: 400
+    }
+    Text { anchors.bottom: parent.bottom; text: list.contentY }
+}

diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
index 32f329f..dcc2e9d 100644 (file)
--- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
@@ -184,6 +184,8 @@ private slots:
     void multipleTransitions();
     void multipleTransitions_data();
 
+    void flickBeyondBounds();
+
 private:
     template <class T> void items(const QUrl &source, bool forceLayout);
     template <class T> void changed(const QUrl &source, bool forceLayout);
@@ -2351,6 +2353,11 @@ void tst_QQuickListView::currentIndex()
     QVERIFY(!listview->highlightItem());
     QVERIFY(!listview->currentItem());
 
+    listview->setCurrentIndex(0);
+    QTRY_VERIFY(listview->currentItem()->isVisible());
+    listview->setContentY(200);
+    QTRY_VERIFY(!listview->currentItem()->isVisible());
+
     delete canvas;
 }
 
@@ -5994,6 +6001,38 @@ void tst_QQuickListView::matchItemLists(const QVariantList &itemLists, const QLi
     }
 }
 
+void tst_QQuickListView::flickBeyondBounds()
+{
+    QQuickView *canvas = createView();
+
+    canvas->setSource(testFileUrl("flickBeyondBoundsBug.qml"));
+    canvas->show();
+    qApp->processEvents();
+
+    QQuickListView *listview = findItem<QQuickListView>(canvas->rootObject(), "list");
+    QTRY_VERIFY(listview != 0);
+
+    QQuickItem *contentItem = listview->contentItem();
+    QTRY_VERIFY(contentItem != 0);
+    QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
+
+    // Flick view up beyond bounds
+    flick(canvas, QPoint(10, 10), QPoint(10, -1000), 180);
+    QTRY_VERIFY(findItems<QQuickItem>(contentItem, "wrapper").count() == 0);
+
+    // We're really testing that we don't get stuck in a loop,
+    // but also confirm items positioned correctly.
+    QTRY_COMPARE(findItems<QQuickItem>(contentItem, "wrapper").count(), 2);
+    for (int i = 0; i < 2; ++i) {
+        QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", i);
+        if (!item) qWarning() << "Item" << i << "not found";
+        QTRY_VERIFY(item);
+        QTRY_VERIFY(item->y() == i*45);
+    }
+
+    delete canvas;
+}
+
 
 QTEST_MAIN(tst_QQuickListView)