aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnton Kudryavtsev <a.kudryavtsev@netris.ru>2016-05-26 14:45:31 +0300
committerAnton Kudryavtsev <a.kudryavtsev@netris.ru>2016-08-12 18:50:22 +0000
commit6f15de1d2da9c83d7fca1d5c8243c0d69a1390ee (patch)
treecbd01a5cf15c8618eca94a0d6949717798a28aff
parent9010dd0e767c407f107aacfe6be145229e3ba793 (diff)
PathView: fix infinite construction/destruction loop
... when all (or almost all) items are in the cache. When all items are in cache, check lower bound is equal to upper_bound. In rare cases, especially when almost all items are in cache, the inserting code was used (not only appending and prepending). In this code there was not bound check before creation of item and there was such situation: 1. Create item by inserting code (without bound check) 2. At the next call of refill() remove this item by life cycle because this item does not meet the conditions. And go to step 1. In other words at the first call we create some item, at the second remove this item. And again. So we had infinite construction/destruction loop. To break it we should check position of new item before creation in inserting code too (like we do in appending and prepending code). Task-number: QTBUG-37815 Change-Id: I015cdeb67ca5fcd06c34b3145b49cbd3e38d4078 Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
-rw-r--r--src/quick/items/qquickpathview.cpp44
-rw-r--r--tests/auto/quick/qquickpathview/data/qtbug37815.qml77
-rw-r--r--tests/auto/quick/qquickpathview/tst_qquickpathview.cpp26
3 files changed, 129 insertions, 18 deletions
diff --git a/src/quick/items/qquickpathview.cpp b/src/quick/items/qquickpathview.cpp
index 1dbdd10303..f858b18c84 100644
--- a/src/quick/items/qquickpathview.cpp
+++ b/src/quick/items/qquickpathview.cpp
@@ -289,6 +289,8 @@ qreal QQuickPathViewPrivate::positionOfIndex(qreal index) const
// account the circular space.
bool QQuickPathViewPrivate::isInBound(qreal position, qreal lower, qreal upper) const
{
+ if (lower == upper)
+ return true;
if (lower > upper) {
if (position > upper && position > lower)
position -= mappedRange;
@@ -2047,27 +2049,33 @@ void QQuickPathView::refill()
idx = startIdx;
QQuickItem *lastItem = d->items[0];
while (idx != endIdx) {
- //This gets the reference from the delegate model, and will not re-create
- QQuickItem *item = d->getItem(idx, idx+1, nextPos >= 1.0);
- if (!item) {
- waiting = true;
- break;
- }
- if (!d->items.contains(item)) { //We found a hole
- nextPos = d->positionOfIndex(idx);
- qCDebug(lcItemViewDelegateLifecycle) << "middle insert" << idx << "@" << nextPos << (d->currentIndex == idx ? "current" : "") << "items count was" << d->items.count();
- if (d->currentIndex == idx) {
- currentVisible = true;
- d->currentItemOffset = nextPos;
+ nextPos = d->positionOfIndex(idx);
+ if (d->isInBound(nextPos, d->mappedRange - d->mappedCache, 1.0 + d->mappedCache)) {
+ //This gets the reference from the delegate model, and will not re-create
+ QQuickItem *item = d->getItem(idx, idx+1, nextPos >= 1.0);
+ if (!item) {
+ waiting = true;
+ break;
}
- int lastListIdx = d->items.indexOf(lastItem);
- d->items.insert(lastListIdx + 1, item);
- d->updateItem(item, nextPos);
- } else {
- d->releaseItem(item);
+
+ if (!d->items.contains(item)) { //We found a hole
+ qCDebug(lcItemViewDelegateLifecycle) << "middle insert" << idx << "@" << nextPos
+ << (d->currentIndex == idx ? "current" : "")
+ << "items count was" << d->items.count();
+ if (d->currentIndex == idx) {
+ currentVisible = true;
+ d->currentItemOffset = nextPos;
+ }
+ int lastListIdx = d->items.indexOf(lastItem);
+ d->items.insert(lastListIdx + 1, item);
+ d->updateItem(item, nextPos);
+ } else {
+ d->releaseItem(item);
+ }
+
+ lastItem = item;
}
- lastItem = item;
++idx;
if (idx >= d->modelCount)
idx = 0;
diff --git a/tests/auto/quick/qquickpathview/data/qtbug37815.qml b/tests/auto/quick/qquickpathview/data/qtbug37815.qml
new file mode 100644
index 0000000000..3fd4daca63
--- /dev/null
+++ b/tests/auto/quick/qquickpathview/data/qtbug37815.qml
@@ -0,0 +1,77 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 Netris
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:BSD$
+** You may use this file under the terms of the BSD license as follows:
+**
+** "Redistribution and use in source and binary forms, with or without
+** modification, are permitted provided that the following conditions are
+** met:
+** * Redistributions of source code must retain the above copyright
+** notice, this list of conditions and the following disclaimer.
+** * Redistributions in binary form must reproduce the above copyright
+** notice, this list of conditions and the following disclaimer in
+** the documentation and/or other materials provided with the
+** distribution.
+** * Neither the name of The Qt Company Ltd nor the names of its
+** contributors may be used to endorse or promote products derived
+** from this software without specific prior written permission.
+**
+**
+** THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+** "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+** LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+** A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+** OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+** SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+** LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+** DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+** THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+** (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+** OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE."
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick 2.0
+
+Rectangle {
+ width: 600
+ height: 400
+ PathView {
+ objectName: "pathView"
+ model: 10
+ anchors.fill: parent
+ pathItemCount: 5
+ cacheItemCount: 5
+ highlightRangeMode: PathView.StrictlyEnforceRange
+ preferredHighlightBegin: 0.5
+ preferredHighlightEnd: 0.5
+
+ path: Path {
+ startX: 0
+ startY: 50
+ PathLine {
+ x: 600
+ y: 50
+ }
+ }
+
+ delegate: Component {
+ Text {
+ width: 50
+ height: 50
+ text: index
+ objectName: "delegate" + index
+ font.pixelSize: 24
+ color: PathView.isCurrentItem ? "green" : "black"
+ }
+ }
+ }
+}
+
diff --git a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
index 2046391d02..493af97521 100644
--- a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
+++ b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
@@ -140,6 +140,7 @@ private slots:
void nestedinFlickable();
void flickableDelegate();
void jsArrayChange();
+ void qtbug37815();
void qtbug42716();
void qtbug53464();
void addCustomAttribute();
@@ -2334,6 +2335,31 @@ void tst_QQuickPathView::jsArrayChange()
QCOMPARE(spy.count(), 1);
}
+void tst_QQuickPathView::qtbug37815()
+{
+ QScopedPointer<QQuickView> window(createView());
+
+ window->setSource(testFileUrl("qtbug37815.qml"));
+ window->show();
+ window->requestActivate();
+ QVERIFY(QTest::qWaitForWindowActive(window.data()));
+
+ // cache items will be created async. Let's wait...
+ QTest::qWait(1000);
+
+ QQuickPathView *pathView = findItem<QQuickPathView>(window->rootObject(), "pathView");
+ QVERIFY(pathView != Q_NULLPTR);
+
+ const int pathItemCount = pathView->pathItemCount();
+ const int cacheItemCount = pathView->cacheItemCount();
+ int totalCount = 0;
+ foreach (QQuickItem *item, pathView->childItems()) {
+ if (item->objectName().startsWith(QLatin1String("delegate")))
+ ++totalCount;
+ }
+ QCOMPARE(pathItemCount + cacheItemCount, totalCount);
+}
+
/* This bug was one where if you jump the list such that the sole missing item needed to be
* added in the middle of the list, it would instead move an item somewhere else in the list
* to the middle (messing it up very badly).