emit layoutAboutToBeChanged timely

layoutAboutToBeChanged must be called before
persistentIndexList as the user might create persistent indexes
as a response to the signal

Fixes: QTBUG-93466
Pick-to: 6.2 5.15
Change-Id: I73c24501f536ef9b6092c3374821497f0a8f0de4
Reviewed-by: David Faure <david.faure@kdab.com>

6 files changed, 221 insertions, 2 deletions

diff --git a/tests/auto/corelib/itemmodels/qconcatenatetablesproxymodel/tst_qconcatenatetablesproxymodel.cpp b/tests/auto/corelib/itemmodels/qconcatenatetablesproxymodel/tst_qconcatenatetablesproxymodel.cpp
index 90972caa57..0188768db5 100644
--- a/tests/auto/corelib/itemmodels/qconcatenatetablesproxymodel/tst_qconcatenatetablesproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qconcatenatetablesproxymodel/tst_qconcatenatetablesproxymodel.cpp
@@ -118,7 +118,7 @@ private Q_SLOTS:
     void shouldPropagateDropAfterLastRow();
     void qtbug91788();
     void qtbug91878();
-
+    void createPersistentOnLayoutAboutToBeChanged();
 private:
     QStandardItemModel mod;
     QStandardItemModel mod2;
@@ -860,6 +860,44 @@ void tst_QConcatenateTablesProxyModel::qtbug91878()
     QCOMPARE(pm.rowCount(), 4);
 }
 
+void tst_QConcatenateTablesProxyModel::createPersistentOnLayoutAboutToBeChanged() // QTBUG-93466
+{
+    QStandardItemModel model1(3, 1);
+    QStandardItemModel model2(3, 1);
+    for (int row = 0; row < 3; ++row) {
+        model1.setData(model1.index(row, 0), row);
+        model2.setData(model2.index(row, 0), row + 5);
+    }
+    QConcatenateTablesProxyModel proxy;
+    new QAbstractItemModelTester(&proxy, &proxy);
+    proxy.addSourceModel(&model1);
+    proxy.addSourceModel(&model2);
+    QList<QPersistentModelIndex> idxList;
+    QSignalSpy layoutAboutToBeChangedSpy(&proxy, &QAbstractItemModel::layoutAboutToBeChanged);
+    QSignalSpy layoutChangedSpy(&proxy, &QAbstractItemModel::layoutChanged);
+    connect(&proxy, &QAbstractItemModel::layoutAboutToBeChanged, this, [&idxList, &proxy](){
+        idxList.clear();
+        for (int row = 0; row < 3; ++row)
+            idxList << QPersistentModelIndex(proxy.index(row, 0));
+    });
+    connect(&proxy, &QAbstractItemModel::layoutChanged, this, [&idxList](){
+        QCOMPARE(idxList.size(), 3);
+        QCOMPARE(idxList.at(0).row(), 1);
+        QCOMPARE(idxList.at(0).column(), 0);
+        QCOMPARE(idxList.at(0).data().toInt(), 0);
+        QCOMPARE(idxList.at(1).row(), 0);
+        QCOMPARE(idxList.at(1).column(), 0);
+        QCOMPARE(idxList.at(1).data().toInt(), -1);
+        QCOMPARE(idxList.at(2).row(), 2);
+        QCOMPARE(idxList.at(2).column(), 0);
+        QCOMPARE(idxList.at(2).data().toInt(), 2);
+    });
+    QVERIFY(model1.setData(model1.index(1, 0), -1));
+    model1.sort(0);
+    QCOMPARE(layoutAboutToBeChangedSpy.size(), 1);
+    QCOMPARE(layoutChangedSpy.size(), 1);
+}
+
 QTEST_GUILESS_MAIN(tst_QConcatenateTablesProxyModel)
 
 #include "tst_qconcatenatetablesproxymodel.moc"
diff --git a/tests/auto/corelib/itemmodels/qidentityproxymodel/tst_qidentityproxymodel.cpp b/tests/auto/corelib/itemmodels/qidentityproxymodel/tst_qidentityproxymodel.cpp
index 4828df8b94..3f93938ed9 100644
--- a/tests/auto/corelib/itemmodels/qidentityproxymodel/tst_qidentityproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qidentityproxymodel/tst_qidentityproxymodel.cpp
@@ -77,7 +77,7 @@ private slots:
     void itemData();
 
     void persistIndexOnLayoutChange();
-
+    void createPersistentOnLayoutAboutToBeChanged();
 protected:
     void verifyIdentity(QAbstractItemModel *model, const QModelIndex &parent = QModelIndex());
 
@@ -494,5 +494,40 @@ void tst_QIdentityProxyModel::persistIndexOnLayoutChange()
     QVERIFY(persistentIndex.isValid());
 }
 
+void tst_QIdentityProxyModel::createPersistentOnLayoutAboutToBeChanged() // QTBUG-93466
+{
+    QStandardItemModel model(3, 1);
+    for (int row = 0; row < 3; ++row)
+        model.setData(model.index(row, 0), row, Qt::UserRole);
+    model.setSortRole(Qt::UserRole);
+    QIdentityProxyModel proxy;
+    new QAbstractItemModelTester(&proxy, &proxy);
+    proxy.setSourceModel(&model);
+    QList<QPersistentModelIndex> idxList;
+    QSignalSpy layoutAboutToBeChangedSpy(&proxy, &QAbstractItemModel::layoutAboutToBeChanged);
+    QSignalSpy layoutChangedSpy(&proxy, &QAbstractItemModel::layoutChanged);
+    connect(&proxy, &QAbstractItemModel::layoutAboutToBeChanged, this, [&idxList, &proxy](){
+        idxList.clear();
+        for (int row = 0; row < 3; ++row)
+            idxList << QPersistentModelIndex(proxy.index(row, 0));
+    });
+    connect(&proxy, &QAbstractItemModel::layoutChanged, this, [&idxList](){
+        QCOMPARE(idxList.size(), 3);
+        QCOMPARE(idxList.at(0).row(), 1);
+        QCOMPARE(idxList.at(0).column(), 0);
+        QCOMPARE(idxList.at(0).data(Qt::UserRole).toInt(), 0);
+        QCOMPARE(idxList.at(1).row(), 0);
+        QCOMPARE(idxList.at(1).column(), 0);
+        QCOMPARE(idxList.at(1).data(Qt::UserRole).toInt(), -1);
+        QCOMPARE(idxList.at(2).row(), 2);
+        QCOMPARE(idxList.at(2).column(), 0);
+        QCOMPARE(idxList.at(2).data(Qt::UserRole).toInt(), 2);
+    });
+    model.setData(model.index(1, 0), -1, Qt::UserRole);
+    model.sort(0);
+    QCOMPARE(layoutAboutToBeChangedSpy.size(), 1);
+    QCOMPARE(layoutChangedSpy.size(), 1);
+}
+
 QTEST_MAIN(tst_QIdentityProxyModel)
 #include "tst_qidentityproxymodel.moc"
diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp
index d937a708e9..6da65e59e8 100644
--- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp
@@ -5487,4 +5487,39 @@ void tst_QSortFilterProxyModel::filterRegularExpressionBinding()
     QVERIFY(proxyModel.bindableFilterCaseSensitivity().hasBinding());
 }
 
+void tst_QSortFilterProxyModel::createPersistentOnLayoutAboutToBeChanged() // QTBUG-93466
+{
+    QStandardItemModel model(3, 1);
+    for (int row = 0; row < 3; ++row)
+        model.setData(model.index(row, 0), row, Qt::UserRole);
+    QSortFilterProxyModel proxy;
+    new QAbstractItemModelTester(&proxy, &proxy);
+    proxy.setSourceModel(&model);
+    proxy.setSortRole(Qt::UserRole);
+    QList<QPersistentModelIndex> idxList;
+    QSignalSpy layoutAboutToBeChangedSpy(&proxy, &QAbstractItemModel::layoutAboutToBeChanged);
+    QSignalSpy layoutChangedSpy(&proxy, &QAbstractItemModel::layoutChanged);
+    connect(&proxy, &QAbstractItemModel::layoutAboutToBeChanged, this, [&idxList, &proxy](){
+        idxList.clear();
+        for (int row = 0; row < 3; ++row)
+            idxList << QPersistentModelIndex(proxy.index(row, 0));
+    });
+    connect(&proxy, &QAbstractItemModel::layoutChanged, this, [&idxList](){
+        QCOMPARE(idxList.size(), 3);
+        QCOMPARE(idxList.at(0).row(), 1);
+        QCOMPARE(idxList.at(0).column(), 0);
+        QCOMPARE(idxList.at(0).data(Qt::UserRole).toInt(), 0);
+        QCOMPARE(idxList.at(1).row(), 0);
+        QCOMPARE(idxList.at(1).column(), 0);
+        QCOMPARE(idxList.at(1).data(Qt::UserRole).toInt(), -1);
+        QCOMPARE(idxList.at(2).row(), 2);
+        QCOMPARE(idxList.at(2).column(), 0);
+        QCOMPARE(idxList.at(2).data(Qt::UserRole).toInt(), 2);
+    });
+    model.setData(model.index(1, 0), -1, Qt::UserRole);
+    proxy.sort(0);
+    QCOMPARE(layoutAboutToBeChangedSpy.size(), 1);
+    QCOMPARE(layoutChangedSpy.size(), 1);
+}
+
 #include "tst_qsortfilterproxymodel.moc"
diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h
index 63b328c41d..baad28ec06 100644
--- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h
+++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h
@@ -55,6 +55,7 @@ private slots:
     void sort();
     void sortHierarchy_data();
     void sortHierarchy();
+    void createPersistentOnLayoutAboutToBeChanged();
 
     void insertRows_data();
     void insertRows();
diff --git a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
index 79ad4145a2..5fe8784768 100644
--- a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
+++ b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
@@ -93,6 +93,7 @@ private slots:
 
     void itemData();
     void setItemData();
+    void createPersistentOnLayoutAboutToBeChanged();
 };
 
 void tst_QStringListModel::moveRowsInvalid_data()
@@ -447,5 +448,34 @@ void tst_QStringListModel::supportedDragDropActions()
     QCOMPARE(model.supportedDropActions(), Qt::CopyAction | Qt::MoveAction);
 }
 
+void tst_QStringListModel::createPersistentOnLayoutAboutToBeChanged() // QTBUG-93466
+{
+    QStringListModel model(QStringList{QStringLiteral("1"), QStringLiteral("2"), QStringLiteral("3")});
+    QList<QPersistentModelIndex> idxList;
+    QSignalSpy layoutAboutToBeChangedSpy(&model, &QAbstractItemModel::layoutAboutToBeChanged);
+    QSignalSpy layoutChangedSpy(&model, &QAbstractItemModel::layoutChanged);
+    connect(&model, &QAbstractItemModel::layoutAboutToBeChanged, this, [&idxList, &model](){
+        idxList.clear();
+        for (int row = 0; row < 3; ++row)
+            idxList << QPersistentModelIndex(model.index(row, 0));
+    });
+    connect(&model, &QAbstractItemModel::layoutChanged, this, [&idxList](){
+        QCOMPARE(idxList.size(), 3);
+        QCOMPARE(idxList.at(0).row(), 1);
+        QCOMPARE(idxList.at(0).column(), 0);
+        QCOMPARE(idxList.at(0).data().toString(), QStringLiteral("1"));
+        QCOMPARE(idxList.at(1).row(), 0);
+        QCOMPARE(idxList.at(1).column(), 0);
+        QCOMPARE(idxList.at(1).data().toString(), QStringLiteral("0"));
+        QCOMPARE(idxList.at(2).row(), 2);
+        QCOMPARE(idxList.at(2).column(), 0);
+        QCOMPARE(idxList.at(2).data().toString(), QStringLiteral("3"));
+    });
+    model.setData(model.index(1, 0), QStringLiteral("0"));
+    model.sort(0);
+    QCOMPARE(layoutAboutToBeChangedSpy.size(), 1);
+    QCOMPARE(layoutChangedSpy.size(), 1);
+}
+
 QTEST_MAIN(tst_QStringListModel)
 #include "tst_qstringlistmodel.moc"
diff --git a/tests/auto/corelib/itemmodels/qtransposeproxymodel/tst_qtransposeproxymodel.cpp b/tests/auto/corelib/itemmodels/qtransposeproxymodel/tst_qtransposeproxymodel.cpp
index 8e8d1ec709..f577c3c41b 100644
--- a/tests/auto/corelib/itemmodels/qtransposeproxymodel/tst_qtransposeproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qtransposeproxymodel/tst_qtransposeproxymodel.cpp
@@ -84,6 +84,8 @@ private Q_SLOTS:
     void setItemData();
     void moveRowsBase();
     void moveColumnsProxy();
+    void sortPersistentIndex();
+    void createPersistentOnLayoutAboutToBeChanged();
 private:
     void testTransposed(
         const QAbstractItemModel *const baseModel,
@@ -343,6 +345,84 @@ void tst_QTransposeProxyModel::removeColumnBase()
     delete model;
 }
 
+void tst_QTransposeProxyModel::sortPersistentIndex()
+{
+    QStringListModel model(QStringList{QStringLiteral("Alice"), QStringLiteral("Charlie"), QStringLiteral("Bob")});
+    QTransposeProxyModel proxy;
+    new QAbstractItemModelTester(&proxy, &proxy);
+    proxy.setSourceModel(&model);
+    QPersistentModelIndex aliceIdx = proxy.index(0, 0);
+    QPersistentModelIndex bobIdx = proxy.index(0, 2);
+    QPersistentModelIndex charlieIdx = proxy.index(0, 1);
+    connect(&proxy, &QAbstractItemModel::layoutAboutToBeChanged, this, [&aliceIdx, &bobIdx, &charlieIdx](){
+        QCOMPARE(aliceIdx.row(), 0);
+        QCOMPARE(aliceIdx.column(), 0);
+        QCOMPARE(aliceIdx.data().toString(), QStringLiteral("Alice"));
+        QCOMPARE(bobIdx.row(), 0);
+        QCOMPARE(bobIdx.column(), 2);
+        QCOMPARE(bobIdx.data().toString(), QStringLiteral("Bob"));
+        QCOMPARE(charlieIdx.row(), 0);
+        QCOMPARE(charlieIdx.column(), 1);
+        QCOMPARE(charlieIdx.data().toString(), QStringLiteral("Charlie"));
+    });
+    connect(&proxy, &QAbstractItemModel::layoutChanged, this, [&aliceIdx, &bobIdx, &charlieIdx](){
+        QCOMPARE(aliceIdx.row(), 0);
+        QCOMPARE(aliceIdx.column(), 0);
+        QCOMPARE(aliceIdx.data().toString(), QStringLiteral("Alice"));
+        QCOMPARE(bobIdx.row(), 0);
+        QCOMPARE(bobIdx.column(), 1);
+        QCOMPARE(bobIdx.data().toString(), QStringLiteral("Bob"));
+        QCOMPARE(charlieIdx.row(), 0);
+        QCOMPARE(charlieIdx.column(), 2);
+        QCOMPARE(charlieIdx.data().toString(), QStringLiteral("Charlie"));
+    });
+    model.sort(0);
+    QCOMPARE(aliceIdx.row(), 0);
+    QCOMPARE(aliceIdx.column(), 0);
+    QCOMPARE(aliceIdx.data().toString(), QStringLiteral("Alice"));
+    QCOMPARE(bobIdx.row(), 0);
+    QCOMPARE(bobIdx.column(), 1);
+    QCOMPARE(bobIdx.data().toString(), QStringLiteral("Bob"));
+    QCOMPARE(charlieIdx.row(), 0);
+    QCOMPARE(charlieIdx.column(), 2);
+    QCOMPARE(charlieIdx.data().toString(), QStringLiteral("Charlie"));
+}
+
+void tst_QTransposeProxyModel::createPersistentOnLayoutAboutToBeChanged() // QTBUG-93466
+{
+    QStandardItemModel model(3, 1);
+    for (int row = 0; row < 3; ++row)
+        model.setData(model.index(row, 0), row, Qt::UserRole);
+    model.setSortRole(Qt::UserRole);
+    QTransposeProxyModel proxy;
+    new QAbstractItemModelTester(&proxy, &proxy);
+    proxy.setSourceModel(&model);
+    QList<QPersistentModelIndex> idxList;
+    QSignalSpy layoutAboutToBeChangedSpy(&proxy, &QAbstractItemModel::layoutAboutToBeChanged);
+    QSignalSpy layoutChangedSpy(&proxy, &QAbstractItemModel::layoutChanged);
+    connect(&proxy, &QAbstractItemModel::layoutAboutToBeChanged, this, [&idxList, &proxy](){
+        idxList.clear();
+        for (int row = 0; row < 3; ++row)
+            idxList << QPersistentModelIndex(proxy.index(0, row));
+    });
+    connect(&proxy, &QAbstractItemModel::layoutChanged, this, [&idxList](){
+        QCOMPARE(idxList.size(), 3);
+        QCOMPARE(idxList.at(0).row(), 0);
+        QCOMPARE(idxList.at(0).column(), 1);
+        QCOMPARE(idxList.at(0).data(Qt::UserRole).toInt(), 0);
+        QCOMPARE(idxList.at(1).row(), 0);
+        QCOMPARE(idxList.at(1).column(), 0);
+        QCOMPARE(idxList.at(1).data(Qt::UserRole).toInt(), -1);
+        QCOMPARE(idxList.at(2).row(), 0);
+        QCOMPARE(idxList.at(2).column(), 2);
+        QCOMPARE(idxList.at(2).data(Qt::UserRole).toInt(), 2);
+    });
+    model.setData(model.index(1, 0), -1, Qt::UserRole);
+    model.sort(0);
+    QCOMPARE(layoutAboutToBeChangedSpy.size(), 1);
+    QCOMPARE(layoutChangedSpy.size(), 1);
+}
+
 void tst_QTransposeProxyModel::insertColumnBase_data()
 {
     QTest::addColumn<QAbstractItemModel *>("model");