Fix crash when items are moved in model.

When erasing an item we need to backtrack to the previous item so the
next iteration doesn't skip an item. In the worst case the next item
is the last and a failure to backtrack will cause the loop to wrap
around and run over the list again.

Task-number: QTBUG-23107
Change-Id: I82156f6fc1f7973ba11f09a4694230c77c293757
Reviewed-by: Bea Lam <bea.lam@nokia.com>

diff --git a/src/quick/util/qdeclarativelistcompositor.cpp b/src/quick/util/qdeclarativelistcompositor.cpp
index d73d76e..e43a8a7 100644 (file)
--- a/src/quick/util/qdeclarativelistcompositor.cpp
+++ b/src/quick/util/qdeclarativelistcompositor.cpp
@@ -872,9 +872,7 @@ void QDeclarativeListCompositor::listItemsRemoved(
 {
     QT_DECLARATIVE_TRACE_LISTCOMPOSITOR(<< list << *removals)
 
-    for (iterator it(m_ranges.next, 0, Default, m_groupCount);
-            *it != &m_ranges && !removals->isEmpty();
-            *it = it->next) {
+    for (iterator it(m_ranges.next, 0, Default, m_groupCount); *it != &m_ranges; *it = it->next) {
         if (it->list != list || it->flags == CacheFlag) {
             it.incrementIndexes(it->count);
             continue;
@@ -920,21 +918,20 @@ void QDeclarativeListCompositor::listItemsRemoved(
                         translatedRemoval.moveId = ++moveId;
                         movedFlags->append(MovedFlags(moveId, it->flags & ~AppendFlag));
 
-                        removal = removals->insert(removal, QDeclarativeChangeSet::Remove(
-                                removal->index, removeCount, translatedRemoval.moveId));
-                        ++removal;
-                        insertion = insertions->insert(insertion, QDeclarativeChangeSet::Insert(
-                                insertion->index, removeCount, translatedRemoval.moveId));
-                        ++insertion;
-
-                        removal->count -= removeCount;
-                        insertion->index += removeCount;
-                        insertion->count -= removeCount;
-                        if (removal->count == 0) {
-                            removal = removals->erase(removal);
-                            insertion = insertions->erase(insertion);
-                            --removal;
-                            --insertion;
+                        if (removeCount < removal->count) {
+                            removal = removals->insert(removal, QDeclarativeChangeSet::Remove(
+                                    removal->index, removeCount, translatedRemoval.moveId));
+                            ++removal;
+                            insertion = insertions->insert(insertion, QDeclarativeChangeSet::Insert(
+                                    insertion->index, removeCount, translatedRemoval.moveId));
+                            ++insertion;
+
+                            removal->count -= removeCount;
+                            insertion->index += removeCount;
+                            insertion->count -= removeCount;
+                        } else {
+                            removal->moveId = translatedRemoval.moveId;
+                            insertion->moveId = translatedRemoval.moveId;
                         }
                     } else {
                         if (offset > 0) {
@@ -989,7 +986,7 @@ void QDeclarativeListCompositor::listItemsRemoved(
                     it->previous->count += it->count;
                     it->previous->flags = it->flags;
                     it.incrementIndexes(it->count);
-                    *it = erase(*it);
+                    *it = erase(*it)->previous;
                     removed = true;
                 }
             }

diff --git a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp
index ed786eb..43ae2f6 100644 (file)
--- a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp
+++ b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp
@@ -1490,6 +1490,23 @@ void tst_qdeclarativelistcompositor::listItemsMoved_data()
                 << IndexArray(defaultIndexes)
                 << IndexArray()
                 << IndexArray();
+    } { static const int cacheIndexes[] = {/*A*/0,1,2,3};
+        static const int defaultIndexes[] = {/*A*/0,1,2,3};
+        static const int selectionIndexes[] = {/*A*/3};
+        QTest::newRow("Move selection")
+                << (RangeList()
+                    << Range(a, 0, 2, C::PrependFlag | C::DefaultFlag | C::CacheFlag)
+                    << Range(a, 2, 1, C::PrependFlag | SelectionFlag | C::DefaultFlag | C::CacheFlag)
+                    << Range(a, 3, 1, C::AppendFlag | C::PrependFlag | C::DefaultFlag | C::CacheFlag))
+                << a << 2 << 3 << 1
+                << (RemoveList()
+                    << Remove(0, 0, 2, 2, 1, C::PrependFlag | SelectionFlag | C::DefaultFlag | C::CacheFlag, 0))
+                << (InsertList()
+                    << Insert(0, 0, 3, 3, 1, C::PrependFlag | SelectionFlag | C::DefaultFlag | C::CacheFlag, 0))
+                << IndexArray(cacheIndexes)
+                << IndexArray(defaultIndexes)
+                << IndexArray()
+                << IndexArray(selectionIndexes);
     }
 }