File 0002-QQmlDelegateModel-fix-delegates-not-being-... of Package qt6-declarative

Overview Repositories Revisions Requests Users Attributes Meta

File 0002-QQmlDelegateModel-fix-delegates-not-being-created-in.patch of Package qt6-declarative (Revision 923004f59fdd0d6740d6990d72792bc6)

Currently displaying revision 923004f59fdd0d6740d6990d72792bc6 , Show latest

From 2aefbca84d2f3dca2c2697f13710b6907c0c7e59 Mon Sep 17 00:00:00 2001
From: Mitch Curtis <mitch.curtis@qt.io>
Date: Tue, 24 Sep 2024 10:22:12 +0800
Subject: [PATCH 2/2] QQmlDelegateModel: fix delegates not being created in
 certain cases v2

Since 837c2f18cd223707e7cedb213257b0158ea07146, we connect to
modelAboutToBeReset rather than modelReset so that we can handle role
name changes. _q_modelAboutToBeReset now connects modelReset to
handleModelReset with a single shot connection instead.

However, it's possible for user code to begin the reset before
connectToAbstractItemModel is called (QTBUG-125053), in which case we
connect to modelReset too late and handleModelReset is never called,
resulting in delegates not being created in certain cases.

So, we check at the earliest point we can if the model is in the
process of being reset, and if so, connect modelReset to
handleModelReset.

This is a less intrusive alternative to
6561344dd2d1ba69abe6edec4fe340b256da9e13, which caused regressions and
was reverted.

Fixes: QTBUG-125053
Task-number: QTBUG-127340
Pick-to: 6.7 6.5
Change-Id: I2bfe192ed61eddaa481de4b1e14b1fa5d07a51c1
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 4bb16ce2c8ea94f768991593a581c8838d48f3a3)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/qmlmodels/qqmldelegatemodel.cpp           |  17 ++
 .../auto/qml/qqmldelegatemodel/CMakeLists.txt |   1 +
 ...yModelWithDelayedSourceModelInListView.qml |  30 +++
 .../auto/qml/qqmldelegatemodel/data/reset.qml |  28 +++
 .../data/resetInQAIMConstructor.qml           |  28 +++
 .../tst_qqmldelegatemodel.cpp                 | 210 +++++++++++++++++-
 6 files changed, 304 insertions(+), 10 deletions(-)
 create mode 100644 tests/auto/qml/qqmldelegatemodel/data/proxyModelWithDelayedSourceModelInListView.qml
 create mode 100644 tests/auto/qml/qqmldelegatemodel/data/reset.qml
 create mode 100644 tests/auto/qml/qqmldelegatemodel/data/resetInQAIMConstructor.qml

diff --git a/src/qmlmodels/qqmldelegatemodel.cpp b/src/qmlmodels/qqmldelegatemodel.cpp
index 7cfa662aa6..f8de5cba82 100644
--- a/src/qmlmodels/qqmldelegatemodel.cpp
+++ b/src/qmlmodels/qqmldelegatemodel.cpp
@@ -3,6 +3,8 @@
 
 #include "qqmldelegatemodel_p_p.h"
 
+#include <QtCore/private/qabstractitemmodel_p.h>
+
 #include <QtQml/qqmlinfo.h>
 
 #include <private/qqmlabstractdelegatecomponent_p.h>
@@ -409,6 +411,21 @@ void QQmlDelegateModel::setModel(const QVariant &model)
         _q_itemsInserted(0, d->adaptorModelCount());
         d->requestMoreIfNecessary();
     }
+
+    // Since 837c2f18cd223707e7cedb213257b0158ea07146, we connect to modelAboutToBeReset
+    // rather than modelReset so that we can handle role name changes. _q_modelAboutToBeReset
+    // now connects modelReset to handleModelReset with a single shot connection instead.
+    // However, it's possible for user code to begin the reset before connectToAbstractItemModel is called
+    // (QTBUG-125053), in which case we connect to modelReset too late and handleModelReset is never called,
+    // resulting in delegates not being created in certain cases.
+    // So, we check at the earliest point we can if the model is in the process of being reset,
+    // and if so, connect modelReset to handleModelReset.
+    if (d->m_adaptorModel.adaptsAim()) {
+        auto *aim = d->m_adaptorModel.aim();
+        auto *aimPrivate = QAbstractItemModelPrivate::get(aim);
+        if (aimPrivate->resetting)
+            QObject::connect(aim, &QAbstractItemModel::modelReset, this, &QQmlDelegateModel::handleModelReset, Qt::SingleShotConnection);
+    }
 }
 
 /*!
diff --git a/tests/auto/qml/qqmldelegatemodel/CMakeLists.txt b/tests/auto/qml/qqmldelegatemodel/CMakeLists.txt
index 8d8a90e0a7..966f5229df 100644
--- a/tests/auto/qml/qqmldelegatemodel/CMakeLists.txt
+++ b/tests/auto/qml/qqmldelegatemodel/CMakeLists.txt
@@ -29,6 +29,7 @@ qt_internal_add_test(tst_qqmldelegatemodel
         Qt::QmlModelsPrivate
         Qt::QmlPrivate
         Qt::Quick
+        Qt::QuickPrivate
         Qt::QuickTestUtilsPrivate
     TESTDATA ${test_data}
 )
diff --git a/tests/auto/qml/qqmldelegatemodel/data/proxyModelWithDelayedSourceModelInListView.qml b/tests/auto/qml/qqmldelegatemodel/data/proxyModelWithDelayedSourceModelInListView.qml
new file mode 100644
index 0000000000..b6733bd38c
--- /dev/null
+++ b/tests/auto/qml/qqmldelegatemodel/data/proxyModelWithDelayedSourceModelInListView.qml
@@ -0,0 +1,30 @@
+import QtQuick
+import Test
+
+Window {
+    id: root
+    title: listView.count
+
+    property alias listView: listView
+    property ProxySourceModel connectionModel: null
+
+    Component {
+        id: modelComponent
+        ProxySourceModel {}
+    }
+
+    ListView {
+        id: listView
+        anchors.fill: parent
+
+        delegate: Text {
+            text: model.Name
+        }
+
+        model: ProxyModel {
+            sourceModel: root.connectionModel
+        }
+    }
+
+    Component.onCompleted: root.connectionModel = modelComponent.createObject(root)
+}
diff --git a/tests/auto/qml/qqmldelegatemodel/data/reset.qml b/tests/auto/qml/qqmldelegatemodel/data/reset.qml
new file mode 100644
index 0000000000..0fcd5e8afa
--- /dev/null
+++ b/tests/auto/qml/qqmldelegatemodel/data/reset.qml
@@ -0,0 +1,28 @@
+import QtQuick
+import Test
+
+Window {
+    id: root
+    width: 200
+    height: 200
+
+    property alias listView: listView
+
+    ResettableModel {
+        id: resetModel
+    }
+
+    ListView {
+        id: listView
+        anchors.fill: parent
+        model: resetModel
+
+        delegate: Rectangle {
+            implicitWidth: 100
+            implicitHeight: 50
+            color: "olivedrab"
+
+            required property string display
+        }
+    }
+}
diff --git a/tests/auto/qml/qqmldelegatemodel/data/resetInQAIMConstructor.qml b/tests/auto/qml/qqmldelegatemodel/data/resetInQAIMConstructor.qml
new file mode 100644
index 0000000000..cb1f226737
--- /dev/null
+++ b/tests/auto/qml/qqmldelegatemodel/data/resetInQAIMConstructor.qml
@@ -0,0 +1,28 @@
+import QtQuick
+import Test
+
+Window {
+    id: root
+    width: 200
+    height: 200
+
+    property alias listView: listView
+
+    ResetInConstructorModel {
+        id: resetInConstructorModel
+    }
+
+    ListView {
+        id: listView
+        anchors.fill: parent
+        model: resetInConstructorModel
+
+        delegate: Rectangle {
+            implicitWidth: 100
+            implicitHeight: 50
+            color: "olivedrab"
+
+            required property string display
+        }
+    }
+}
diff --git a/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp b/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp
index 2cacda5513..3f08d8fc85 100644
--- a/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp
+++ b/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp
@@ -3,7 +3,9 @@
 
 #include <QtTest/qtest.h>
 #include <QtCore/qjsonobject.h>
+#include <QtCore/qsortfilterproxymodel.h>
 #include <QtCore/QConcatenateTablesProxyModel>
+#include <QtCore/qtimer.h>
 #include <QtGui/QStandardItemModel>
 #include <QtQml/qqmlcomponent.h>
 #include <QtQml/qqmlapplicationengine.h>
@@ -11,11 +13,17 @@
 #include <QtQmlModels/private/qqmllistmodel_p.h>
 #include <QtQuick/qquickview.h>
 #include <QtQuick/qquickitem.h>
+#include <QtQuick/private/qquickitemview_p_p.h>
+#include <QtQuick/private/qquicklistview_p.h>
+#include <QtQuickTest/quicktest.h>
 #include <QtQuickTestUtils/private/qmlutils_p.h>
+#include <QtQuickTestUtils/private/visualtestutils_p.h>
 #include <QtTest/QSignalSpy>
 
 #include <forward_list>
 
+using namespace QQuickVisualTestUtils;
+
 class tst_QQmlDelegateModel : public QQmlDataTest
 {
     Q_OBJECT
@@ -25,6 +33,8 @@ public:
 
 private slots:
     void resettingRolesRespected();
+    void resetInQAIMConstructor();
+    void reset();
     void valueWithoutCallingObjectFirst_data();
     void valueWithoutCallingObjectFirst();
     void qtbug_86017();
@@ -42,18 +52,12 @@ private slots:
     void doNotUnrefObjectUnderConstruction();
     void clearCacheDuringInsertion();
     void viewUpdatedOnDelegateChoiceAffectingRoleChange();
+    void proxyModelWithDelayedSourceModelInListView();
 };
 
-class AbstractItemModel : public QAbstractItemModel
+class BaseAbstractItemModel : public QAbstractItemModel
 {
-    Q_OBJECT
 public:
-    AbstractItemModel()
-    {
-        for (int i = 0; i < 3; ++i)
-            mValues.append(QString::fromLatin1("Item %1").arg(i));
-    }
-
     QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const override
     {
         if (parent.isValid())
@@ -91,10 +95,21 @@ public:
         return mValues.at(index.row());
     }
 
-private:
+protected:
     QVector<QString> mValues;
 };
 
+class AbstractItemModel : public BaseAbstractItemModel
+{
+    Q_OBJECT
+public:
+    AbstractItemModel()
+    {
+        for (int i = 0; i < 3; ++i)
+            mValues.append(QString::fromLatin1("Item %1").arg(i));
+    }
+};
+
 tst_QQmlDelegateModel::tst_QQmlDelegateModel()
     : QQmlDataTest(QT_QMLTEST_DATADIR)
 {
@@ -153,7 +168,109 @@ void tst_QQmlDelegateModel::resettingRolesRespected()
     QObject *root = engine.rootObjects().constFirst();
     QVERIFY(!root->property("success").toBool());
     model->change();
-    QTRY_VERIFY(root->property("success").toBool());
+    QTRY_VERIFY_WITH_TIMEOUT(root->property("success").toBool(), 100);
+}
+
+class ResetInConstructorModel : public BaseAbstractItemModel
+{
+    Q_OBJECT
+    QML_ELEMENT
+
+public:
+    ResetInConstructorModel()
+    {
+        beginResetModel();
+        QTimer::singleShot(0, this, &ResetInConstructorModel::finishReset);
+    }
+
+private:
+    void finishReset()
+    {
+        mValues.append("First");
+        endResetModel();
+    }
+};
+
+void tst_QQmlDelegateModel::resetInQAIMConstructor()
+{
+    qmlRegisterTypesAndRevisions<ResetInConstructorModel>("Test", 1);
+
+    QQuickApplicationHelper helper(this, "resetInQAIMConstructor.qml");
+    QVERIFY2(helper.ready, helper.failureMessage());
+    QQuickWindow *window = helper.window;
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+
+    auto *listView = window->property("listView").value<QQuickListView *>();
+    QVERIFY(listView);
+    QTRY_VERIFY_WITH_TIMEOUT(listView->itemAtIndex(0), 100);
+    QQuickItem *firstDelegateItem = listView->itemAtIndex(0);
+    QVERIFY(firstDelegateItem);
+    QCOMPARE(firstDelegateItem->property("display").toString(), "First");
+}
+
+class ResettableModel : public BaseAbstractItemModel
+{
+    Q_OBJECT
+    QML_ELEMENT
+
+public:
+    ResettableModel()
+    {
+        mValues.append("First");
+    }
+
+    void callBeginResetModel()
+    {
+        beginResetModel();
+        mValues.clear();
+    }
+
+    void appendData()
+    {
+        mValues.append(QString::fromLatin1("Item %1").arg(mValues.size()));
+    }
+
+    void callEndResetModel()
+    {
+        endResetModel();
+    }
+};
+
+// Tests that everything works as expected when calling beginResetModel/endResetModel
+// after the QAIM subclass constructor has run.
+void tst_QQmlDelegateModel::reset()
+{
+    qmlRegisterTypesAndRevisions<ResettableModel>("Test", 1);
+
+    QQuickApplicationHelper helper(this, "reset.qml");
+    QVERIFY2(helper.ready, helper.failureMessage());
+    QQuickWindow *window = helper.window;
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+
+    auto *listView = window->property("listView").value<QQuickListView *>();
+    QVERIFY(listView);
+    QQuickItem *firstDelegateItem = listView->itemAtIndex(0);
+    QVERIFY(firstDelegateItem);
+    QCOMPARE(firstDelegateItem->property("display").toString(), "First");
+
+    const auto delegateModel = QQuickItemViewPrivate::get(listView)->model;
+    QSignalSpy rootIndexChangedSpy(delegateModel, SIGNAL(rootIndexChanged()));
+    QVERIFY(rootIndexChangedSpy.isValid());
+
+    auto *model = listView->model().value<ResettableModel *>();
+    model->callBeginResetModel();
+    model->appendData();
+    model->callEndResetModel();
+    // This is verifies that handleModelReset isn't called
+    // more than once during this process, since it unconditionally emits rootIndexChanged.
+    QCOMPARE(rootIndexChangedSpy.count(), 1);
+
+    QTRY_VERIFY_WITH_TIMEOUT(listView->itemAtIndex(0), 100);
+    firstDelegateItem = listView->itemAtIndex(0);
+    QVERIFY(firstDelegateItem);
+    QCOMPARE(firstDelegateItem->property("display").toString(), "Item 0");
 }
 
 void tst_QQmlDelegateModel::valueWithoutCallingObjectFirst_data()
@@ -616,6 +733,79 @@ void tst_QQmlDelegateModel::viewUpdatedOnDelegateChoiceAffectingRoleChange()
     QVERIFY(returnedValue);
 }
 
+class ProxySourceModel : public QAbstractListModel
+{
+    Q_OBJECT
+    QML_ELEMENT
+public:
+    explicit ProxySourceModel(QObject *parent = nullptr)
+        : QAbstractListModel(parent)
+    {
+        for (int i = 0; i < rows; ++i) {
+            beginInsertRows(QModelIndex(), i, i);
+            endInsertRows();
+        }
+    }
+
+    ~ProxySourceModel() override = default;
+
+    int rowCount(const QModelIndex &) const override
+    {
+        return rows;
+    }
+
+    QVariant data(const QModelIndex &, int ) const override
+    {
+        return "Hello";
+    }
+
+    QHash<int, QByteArray> roleNames() const override
+    {
+        QHash<int, QByteArray> roles = QAbstractListModel::roleNames();
+        roles[Qt::UserRole + 1] = "Name";
+
+        return roles;
+    }
+
+    static const int rows = 1;
+};
+
+class ProxyModel : public QSortFilterProxyModel
+{
+    Q_OBJECT
+    QML_ELEMENT
+    Q_PROPERTY(QAbstractItemModel *sourceModel READ sourceModel WRITE setSourceModel)
+
+public:
+    explicit ProxyModel(QObject *parent = nullptr)
+        : QSortFilterProxyModel(parent)
+    {
+    }
+
+    ~ProxyModel() override = default;
+};
+
+// Checks that the correct amount of delegates are created when using a proxy
+// model whose source model is set after a delay.
+void tst_QQmlDelegateModel::proxyModelWithDelayedSourceModelInListView()
+{
+    QTest::failOnWarning();
+
+    qmlRegisterTypesAndRevisions<ProxySourceModel>("Test", 1);
+    qmlRegisterTypesAndRevisions<ProxyModel>("Test", 1);
+
+    QQuickApplicationHelper helper(this, "proxyModelWithDelayedSourceModelInListView.qml");
+    QVERIFY2(helper.ready, helper.failureMessage());
+    QQuickWindow *window = helper.window;
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+
+    auto *listView = window->property("listView").value<QQuickListView *>();
+    QVERIFY(listView);
+    const auto delegateModel = QQuickItemViewPrivate::get(listView)->model;
+    QTRY_COMPARE(listView->count(), 1);
+}
+
 QTEST_MAIN(tst_QQmlDelegateModel)
 
 #include "tst_qqmldelegatemodel.moc"
-- 
2.46.1

Places

File 0002-QQmlDelegateModel-fix-delegates-not-being-created-in.patch of Package qt6-declarative (Revision 923004f59fdd0d6740d6990d72792bc6)

Places