Fix index handling on insert in QQmlDelegateModel

QQmlListCompositor can manipulate the "before" index when inserting. In
particular it can extend an existing group. QQmlDelegateModel's cache,
however, always wants the original index.

Fixes: QTBUG-100161
Fixes: QTBUG-103203
Change-Id: I35c0a14ae45c9f66fdd079894dd0de2f80e2c781
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
(cherry picked from commit e8ba9a3caa362f05657c4cb83893fe429ebaf4ab)
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

4 files changed, 154 insertions, 1 deletions

diff --git a/src/qmlmodels/qqmldelegatemodel.cpp b/src/qmlmodels/qqmldelegatemodel.cpp
index ebe165c644..416efbf9cd 100644
--- a/src/qmlmodels/qqmldelegatemodel.cpp
+++ b/src/qmlmodels/qqmldelegatemodel.cpp
@@ -2107,8 +2107,8 @@ bool QQmlDelegateModelPrivate::insert(Compositor::insert_iterator &before, const
     // Must be before the new object is inserted into the cache or its indexes will be adjusted too.
     itemsInserted(QVector<Compositor::Insert>(1, Compositor::Insert(before, 1, cacheItem->groups & ~Compositor::CacheFlag)));
 
-    before = m_compositor.insert(before, nullptr, 0, 1, cacheItem->groups);
     m_cache.insert(before.cacheIndex, cacheItem);
+    m_compositor.insert(before, nullptr, 0, 1, cacheItem->groups);
 
     return true;
 }
diff --git a/tests/auto/quick/qquicklistview2/data/delegateModelRefresh.qml b/tests/auto/quick/qquicklistview2/data/delegateModelRefresh.qml
new file mode 100644
index 0000000000..8c16ed64ce
--- /dev/null
+++ b/tests/auto/quick/qquicklistview2/data/delegateModelRefresh.qml
@@ -0,0 +1,78 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick
+import QtQuick.Controls
+import QtQml.Models
+
+ApplicationWindow {
+    id: window
+    visible: true
+    width: 400
+    height: 800
+    property bool done: false
+
+    ListView {
+        model: delegateModel
+        anchors.fill: parent
+    }
+
+    DelegateModel {
+        id: delegateModel
+        model: ListModel {
+            ListElement {
+                available: true
+            }
+            ListElement {
+                available: true
+            }
+            ListElement {
+                available: true
+            }
+        }
+
+        Component.onCompleted: {
+            delegateModel.refresh()
+            done = true;
+        }
+        function refresh() {
+            var rowCount = delegateModel.model.count;
+            const flatItemsList = []
+            for (var i = 0; i < rowCount; i++) {
+                var entry = delegateModel.model.get(i);
+                flatItemsList.push(entry)
+            }
+
+            for (i = 0; i < flatItemsList.length; ++i) {
+                var item = flatItemsList[i]
+                if (item !== null)
+                    items.insert(item)
+            }
+        }
+    }
+}
diff --git a/tests/auto/quick/qquicklistview2/data/noCrashOnIndexChange.qml b/tests/auto/quick/qquicklistview2/data/noCrashOnIndexChange.qml
new file mode 100644
index 0000000000..6065d09981
--- /dev/null
+++ b/tests/auto/quick/qquicklistview2/data/noCrashOnIndexChange.qml
@@ -0,0 +1,48 @@
+import QtQuick
+import QtQml.Models
+
+Item {
+    ListModel {
+        id: myModel
+        ListElement { role_display: "One"; role_value: 0; }
+        ListElement { role_display: "One"; role_value: 2; }
+        ListElement { role_display: "One"; role_value: 3; }
+        ListElement { role_display: "One"; role_value: 4; }
+        ListElement { role_details: "Two"; role_value: 5; }
+        ListElement { role_details: "Three"; role_value: 6; }
+        ListElement { role_details: "Four"; role_value: 7; }
+        ListElement { role_details: "Five"; role_value: 8; }
+        ListElement { role_details: "Six"; role_value: 9; }
+        ListElement { role_keyID: "Seven"; role_value: 10; }
+        ListElement { role_keyID: "Eight"; role_value: 11; }
+        ListElement { role_keyID: "hello"; role_value: 12; }
+    }
+
+    DelegateModel {
+        id: displayDelegateModel
+        delegate: Text { text: role_display }
+        model: myModel
+        groups: [
+            DelegateModelGroup {
+                includeByDefault: false
+                name: "displayField"
+            }
+        ]
+        filterOnGroup: "displayField"
+        Component.onCompleted: {
+            var rowCount = myModel.count;
+            items.remove(0, rowCount);
+            for (var i = 0; i < rowCount; i++) {
+                var entry = myModel.get(i);
+                if (entry.role_display) {
+                    items.insert(entry, "displayField");
+                }
+            }
+        }
+    }
+
+    ListView {
+        model: displayDelegateModel
+    }
+}
+
diff --git a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
index f7bbbb22e8..27bf389891 100644
--- a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
+++ b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
@@ -55,9 +55,11 @@ private slots:
     void QTBUG_92809();
     void footerUpdate();
     void singletonModelLifetime();
+    void delegateModelRefresh();
 
     void sectionsNoOverlap();
     void metaSequenceAsModel();
+    void noCrashOnIndexChange();
 };
 
 tst_QQuickListView2::tst_QQuickListView2()
@@ -289,6 +291,22 @@ void tst_QQuickListView2::metaSequenceAsModel()
     QCOMPARE(strings[1], QStringLiteral("5/6"));
 }
 
+void tst_QQuickListView2::noCrashOnIndexChange()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("noCrashOnIndexChange.qml"));
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+
+    QObject *delegateModel = qmlContext(o.data())->objectForName("displayDelegateModel");
+    QVERIFY(delegateModel);
+
+    QObject *items = qvariant_cast<QObject *>(delegateModel->property("items"));
+    QCOMPARE(items->property("name").toString(), QStringLiteral("items"));
+    QCOMPARE(items->property("count").toInt(), 4);
+}
+
 class SingletonModel : public QStringListModel
 {
     Q_OBJECT
@@ -308,6 +326,15 @@ void tst_QQuickListView2::singletonModelLifetime()
     QTRY_VERIFY(engine.rootObjects().first()->property("alive").toBool());
 }
 
+void tst_QQuickListView2::delegateModelRefresh()
+{
+    // Test case originates from QTBUG-100161
+    QQmlApplicationEngine engine(testFile("delegateModelRefresh.qml"));
+    QVERIFY(!engine.rootObjects().isEmpty());
+    // needs event loop iteration for callLater to execute
+    QTRY_VERIFY(engine.rootObjects().first()->property("done").toBool());
+}
+
 QTEST_MAIN(tst_QQuickListView2)
 
 #include "tst_qquicklistview2.moc"