QHeaderView: fix visual/logical index corruption when restoring state

This is a followup to 77a8e90 which
didn't handle the case where no columns had been moved.
visualIndices and logicalIndices are empty until initializeIndexMapping()
is called, in which case appending is wrong.

As a result, visualIndex(i) would return -1 for the values over
those added by read(), and an assert would happen at painting time.

The fix is to leave visualIndices and logicalIndices empty if
they are empty already, leaving it to initializeIndexMapping()
to fill them later if necessary (e.g. when moving a column).

Task-number: QTBUG-60837
Change-Id: Ia7e4b9d3122647984acd434dfaa0400df319d065
Reviewed-by: Marc Mutz <[email protected]>
Reviewed-by: Thorbjørn Lund Martsum <[email protected]>

src/widgets/itemviews/qheaderview.cpp

@@ -3876,9 +3876,11 @@ bool QHeaderViewPrivate::read(QDataStream &in)
     const int currentCount = (orient == Qt::Horizontal ? model->columnCount(root) : model->rowCount(root));
     if (newSectionItems.count() < currentCount) {
         // we have sections not in the saved state, give them default settings
-        for (int i = newSectionItems.count(); i < currentCount; ++i) {
-            visualIndicesIn.append(i);
-            logicalIndicesIn.append(i);
+        if (!visualIndicesIn.isEmpty() && !logicalIndicesIn.isEmpty()) {
+            for (int i = newSectionItems.count(); i < currentCount; ++i) {
+                visualIndicesIn.append(i);
+                logicalIndicesIn.append(i);
+            }
         }
         const int insertCount = currentCount - newSectionItems.count();
         const int insertLength = defaultSectionSizeIn * insertCount;

tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp

@@ -171,6 +171,7 @@ private slots:
     void saveRestore();
     void restoreQt4State();
     void restoreToMoreColumns();
+    void restoreToMoreColumnsNoMovedColumns();
     void restoreBeforeSetModel();
     void defaultSectionSizeTest();
     void defaultSectionSizeTestStyles();
@@ -1690,6 +1691,55 @@ void tst_QHeaderView::restoreToMoreColumns()
     QCOMPARE(h4.hiddenSectionCount(), 1);
     QCOMPARE(h4.sortIndicatorSection(), 2);
     QCOMPARE(h4.sortIndicatorOrder(), Qt::DescendingOrder);
+    QCOMPARE(h4.logicalIndex(0), 2);
+    QCOMPARE(h4.logicalIndex(1), 1);
+    QCOMPARE(h4.logicalIndex(2), 0);
+    QCOMPARE(h4.visualIndex(0), 2);
+    QCOMPARE(h4.visualIndex(1), 1);
+    QCOMPARE(h4.visualIndex(2), 0);
+
+    // Repainting shouldn't crash
+    h4.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&h4));
+}
+
+void tst_QHeaderView::restoreToMoreColumnsNoMovedColumns()
+{
+    // Given a model with 2 columns, for saving state
+    QHeaderView h1(Qt::Horizontal);
+    QStandardItemModel model1(1, 2);
+    h1.setModel(&model1);
+    QCOMPARE(h1.visualIndex(0), 0);
+    QCOMPARE(h1.visualIndex(1), 1);
+    QCOMPARE(h1.logicalIndex(0), 0);
+    QCOMPARE(h1.logicalIndex(1), 1);
+    const QByteArray savedState = h1.saveState();
+
+    // And a model with 3 columns, to apply that state upon
+    QHeaderView h2(Qt::Horizontal);
+    QStandardItemModel model2(1, 3);
+    h2.setModel(&model2);
+    QCOMPARE(h2.visualIndex(0), 0);
+    QCOMPARE(h2.visualIndex(1), 1);
+    QCOMPARE(h2.visualIndex(2), 2);
+    QCOMPARE(h2.logicalIndex(0), 0);
+    QCOMPARE(h2.logicalIndex(1), 1);
+    QCOMPARE(h2.logicalIndex(2), 2);
+
+    // When calling restoreState()
+    QVERIFY(h2.restoreState(savedState));
+
+    // Then the index mapping should still be as default
+    QCOMPARE(h2.visualIndex(0), 0);
+    QCOMPARE(h2.visualIndex(1), 1);
+    QCOMPARE(h2.visualIndex(2), 2);
+    QCOMPARE(h2.logicalIndex(0), 0);
+    QCOMPARE(h2.logicalIndex(1), 1);
+    QCOMPARE(h2.logicalIndex(2), 2);
+
+    // And repainting shouldn't crash
+    h2.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&h2));
 }
 
 void tst_QHeaderView::restoreBeforeSetModel()