Fix ListView.isCurrentItem for DelegateModel, take 2

The attached ListView.isCurrentItem property is updated when
QQuickItemViewPrivate::updateCurrent() method is called.
During the initialization it could be potentially called twice:
* from the QQuickItemView::setCurrentIndex() method, but that does
  not happen, because the isComponentCompleted() condition is not
  fulfilled at that time.
* from QQuickItemView::componentComplete() method, but that does
  not happen, because the d->isValid() condition is not fulfilled.
  The latter means that the model is not yet initialized.

This patch attempts to fix it by adding the updateCurrent() call
into layout() method. This method is called each time the
component needs to be redrawn, so it creates the proper
currentItem during the first call. Most of the subsequent calls
will do nothing, because the currentItem will be non-null, and
the currentIndex will not change.

The unit-test is taken from the reverted commit
d9f9d773e92940786f159897623618f3bf6bcf0f.

Another unit-test is meant to check that there is no regression
like in QTBUG-98315.

Done-with: Joni Poikelin <joni.poikelin@qt.io>
Fixes: QTBUG-86744
Pick-to: 6.5 6.4 6.2
Change-Id: Iab86a9c0e22660126f152727e8bd393ac17f5d15
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

4 files changed, 167 insertions, 0 deletions

diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp
index 4b3b90b805..1f3ec79190 100644
--- a/src/quick/items/qquickitemview.cpp
+++ b/src/quick/items/qquickitemview.cpp
@@ -1932,6 +1932,9 @@ void QQuickItemViewPrivate::layout()
         transitioner->resetTargetLists();
     }
 
+    if (!currentItem)
+        updateCurrent(currentIndex);
+
     runDelayedRemoveTransition = false;
     inLayout = false;
 }
diff --git a/tests/auto/quick/qquicklistview2/data/qtbug86744.qml b/tests/auto/quick/qquicklistview2/data/qtbug86744.qml
new file mode 100644
index 0000000000..c69a4f7ec6
--- /dev/null
+++ b/tests/auto/quick/qquicklistview2/data/qtbug86744.qml
@@ -0,0 +1,25 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
+
+import QtQuick
+import QtQml.Models
+
+Item {
+    height: 200
+    width: 100
+    DelegateModel {
+        id: dm
+        model: 2
+        delegate: Item {
+            width: 100
+            height: 20
+            property bool isCurrent: ListView.isCurrentItem
+        }
+    }
+    ListView {
+        objectName: "listView"
+        model: dm
+        currentIndex: 1
+        anchors.fill: parent
+    }
+}
diff --git a/tests/auto/quick/qquicklistview2/data/qtbug98315.qml b/tests/auto/quick/qquicklistview2/data/qtbug98315.qml
new file mode 100644
index 0000000000..bf2ed857b1
--- /dev/null
+++ b/tests/auto/quick/qquicklistview2/data/qtbug98315.qml
@@ -0,0 +1,98 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
+
+import QtQuick
+import QtQml.Models
+
+Item {
+    width: 500
+    height: 200
+
+    property list<QtObject> myModel: [
+        QtObject {
+            objectName: "Item 0"
+            property bool selected: true
+        },
+        QtObject {
+            objectName: "Item 1"
+            property bool selected: false
+        },
+        QtObject {
+            objectName: "Item 2"
+            property bool selected: false
+        },
+        QtObject {
+            objectName: "Item 3"
+            property bool selected: true
+        },
+        QtObject {
+            objectName: "Item 4"
+            property bool selected: true
+        },
+        QtObject {
+            objectName: "Item 5"
+            property bool selected: true
+        },
+        QtObject {
+            objectName: "Item 6"
+            property bool selected: false
+        }
+    ]
+
+    ListView {
+        objectName: "listView"
+        id: listview
+        width: 500
+        height: 200
+
+        focus: true
+        clip: true
+        spacing: 2
+        orientation: ListView.Horizontal
+        highlightMoveDuration: 300
+        highlightMoveVelocity: -1
+        preferredHighlightBegin: (500 - 100) / 2
+        preferredHighlightEnd: (500 + 100) / 2
+        highlightRangeMode: ListView.StrictlyEnforceRange
+        cacheBuffer: 500
+        currentIndex: 1
+
+        model: DelegateModel {
+            id: delegateModel
+            filterOnGroup: "visible"
+            model: myModel
+            groups: [
+                DelegateModelGroup {
+                    name: "visible"
+                    includeByDefault: true
+                }
+            ]
+            delegate: Rectangle {
+                id: tile
+                objectName: model.modelData.objectName
+
+                width: 100
+                height: 100
+                border.width: 0
+                anchors.verticalCenter: parent.verticalCenter
+
+                visible: model.modelData.selected
+                Component.onCompleted: {
+                    DelegateModel.inPersistedItems = true
+                    DelegateModel.inVisible = Qt.binding(function () {
+                        return model.modelData.selected
+                    })
+                }
+
+                property bool isCurrent: ListView.isCurrentItem
+                color: isCurrent ? "red" : "green"
+
+                Text {
+                    id: valueText
+                    anchors.centerIn: parent
+                    text: model.modelData.objectName
+                }
+            }
+        }
+    }
+}
diff --git a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
index 91f5397769..8152e74da3 100644
--- a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
+++ b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
@@ -47,6 +47,8 @@ private slots:
     void flickDuringFlicking();
     void maxExtent_data();
     void maxExtent();
+    void isCurrentItem_DelegateModel();
+    void isCurrentItem_NoRegressionWithDelegateModelGroups();
 
 private:
     void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to);
@@ -868,6 +870,45 @@ void tst_QQuickListView2::maxExtent()
         QCOMPARE(viewAccessor.maxYExtent(), 0);
 }
 
+void tst_QQuickListView2::isCurrentItem_DelegateModel()
+{
+    QScopedPointer<QQuickView> window(createView());
+    window->setSource(testFileUrl("qtbug86744.qml"));
+    window->resize(640, 480);
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window.data()));
+    QQuickListView* listView = window->rootObject()->findChild<QQuickListView*>("listView");
+    QVERIFY(listView);
+    QVariant value = listView->itemAtIndex(1)->property("isCurrent");
+    QVERIFY(value.toBool() == true);
+}
+
+void tst_QQuickListView2::isCurrentItem_NoRegressionWithDelegateModelGroups()
+{
+    QScopedPointer<QQuickView> window(createView());
+    window->setSource(testFileUrl("qtbug98315.qml"));
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window.data()));
+    QQuickListView* listView = window->rootObject()->findChild<QQuickListView*>("listView");
+    QVERIFY(listView);
+
+    QQuickItem *item3 = listView->itemAtIndex(1);
+    QVERIFY(item3);
+    QCOMPARE(item3->property("isCurrent").toBool(), true);
+
+    QObject *item0 = listView->itemAtIndex(0);
+    QVERIFY(item0);
+    QCOMPARE(item0->property("isCurrent").toBool(), false);
+
+    // Press left arrow key -> Item 1 should become current, Item 3 should not
+    // be current anymore. After a previous fix of QTBUG-86744 it was working
+    // incorrectly - see QTBUG-98315
+    QTest::keyPress(window.get(), Qt::Key_Left);
+
+    QTRY_COMPARE(item0->property("isCurrent").toBool(), true);
+    QCOMPARE(item3->property("isCurrent").toBool(), false);
+}
+
 QTEST_MAIN(tst_QQuickListView2)
 
 #include "tst_qquicklistview2.moc"