QHeaderView: Simplify and fix layoutChange handling

A layoutChange indicates that anything can have moved to anywhere else,
including as a result purely of new items being added. It can also
indicate that items are removed.

The old code here incorrectly assumed that the section count remained
constant over this operation by setting the size of the oldSectionHidden
QBitArray - whose size is the size before the layoutChange operation -
and then calling setBit with model rows numbered after the layoutChange
operation.  As the two are not necessarily the same dimensions, this can
result in asserts from the setBit call.

Simplify the handling of layoutChanged entirely by clearing section
information, and using the QPersistentIndexes which indicate hidden
state to restore that state after re-population.

Task-number: QTBUG-53221
Change-Id: I3cda13e86b51b3029b37b647a48748fb604db252
Reviewed-by: Thorbjørn Lund Martsum <tmartsum@gmail.com>

2 files changed, 59 insertions, 24 deletions

diff --git a/src/widgets/itemviews/qheaderview.cpp b/src/widgets/itemviews/qheaderview.cpp
index 298270a785..4e4c9572a3 100644
--- a/src/widgets/itemviews/qheaderview.cpp
+++ b/src/widgets/itemviews/qheaderview.cpp
@@ -2086,40 +2086,26 @@ void QHeaderViewPrivate::_q_layoutChanged()
 {
     Q_Q(QHeaderView);
     viewport->update();
-    if (persistentHiddenSections.isEmpty() || modelIsEmpty()) {
-        if (modelSectionCount() != sectionCount())
-            q->initializeSections();
-        persistentHiddenSections.clear();
+
+    const auto hiddenSections = persistentHiddenSections;
+    persistentHiddenSections.clear();
+
+    clear();
+    q->initializeSections();
+    invalidateCachedSizeHint();
+
+    if (modelIsEmpty()) {
         return;
     }
 
-    QBitArray oldSectionHidden = sectionsHiddenToBitVector();
-    oldSectionHidden.resize(sectionItems.size());
-    bool sectionCountChanged = false;
-
-    for (int i = 0; i < persistentHiddenSections.count(); ++i) {
-        QModelIndex index = persistentHiddenSections.at(i);
+    for (const auto &index : hiddenSections) {
         if (index.isValid()) {
             const int logical = (orientation == Qt::Horizontal
                                  ? index.column()
                                  : index.row());
             q->setSectionHidden(logical, true);
-            oldSectionHidden.setBit(logical, false);
-        } else if (!sectionCountChanged && (modelSectionCount() != sectionCount())) {
-            sectionCountChanged = true;
-            break;
         }
     }
-    persistentHiddenSections.clear();
-
-    for (int i = 0; i < oldSectionHidden.count(); ++i) {
-        if (oldSectionHidden.testBit(i))
-            q->setSectionHidden(i, false);
-    }
-
-    // the number of sections changed; we need to reread the state of the model
-    if (sectionCountChanged)
-        q->initializeSections();
 }
 
 /*!
diff --git a/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp b/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp
index fa543ae2c3..90019a1798 100644
--- a/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp
+++ b/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp
@@ -210,6 +210,7 @@ private slots:
     void QTBUG12268_hiddenMovedSectionSorting();
     void QTBUG14242_hideSectionAutoSize();
     void QTBUG50171_visualRegionForSwappedItems();
+    void QTBUG53221_assertShiftHiddenRow();
     void ensureNoIndexAtLength();
     void offsetConsistent();
 
@@ -2384,6 +2385,54 @@ void tst_QHeaderView::QTBUG50171_visualRegionForSwappedItems()
     headerView.testVisualRegionForSelection();
 }
 
+class QTBUG53221_Model : public QAbstractItemModel
+{
+public:
+    void insertRowAtBeginning()
+    {
+        Q_EMIT layoutAboutToBeChanged();
+        m_displayNames.insert(0, QStringLiteral("Item %1").arg(m_displayNames.count()));
+        // Rows are always inserted at the beginning, so move all others.
+        foreach (const QModelIndex &persIndex, persistentIndexList())
+        {
+            // The vertical header view will have a persistent index stored here on the second call to insertRowAtBeginning.
+            changePersistentIndex(persIndex, index(persIndex.row() + 1, persIndex.column(), persIndex.parent()));
+        }
+        Q_EMIT layoutChanged();
+    }
+
+    QVariant data(const QModelIndex &index, int role) const override
+    {
+        return (role == Qt::DisplayRole) ? m_displayNames.at(index.row()) : QVariant();
+    }
+
+    QModelIndex index(int row, int column, const QModelIndex &) const override { return createIndex(row, column); }
+    QModelIndex parent(const QModelIndex &) const override { return QModelIndex(); }
+    int rowCount(const QModelIndex &) const override { return m_displayNames.count(); }
+    int columnCount(const QModelIndex &) const override { return 1; }
+
+private:
+    QStringList m_displayNames;
+};
+
+void tst_QHeaderView::QTBUG53221_assertShiftHiddenRow()
+{
+    QTableView tableView;
+    QTBUG53221_Model modelTableView;
+    tableView.setModel(&modelTableView);
+
+    modelTableView.insertRowAtBeginning();
+    tableView.setRowHidden(0, true);
+    QCOMPARE(tableView.verticalHeader()->isSectionHidden(0), true);
+    modelTableView.insertRowAtBeginning();
+    QCOMPARE(tableView.verticalHeader()->isSectionHidden(0), false);
+    QCOMPARE(tableView.verticalHeader()->isSectionHidden(1), true);
+    modelTableView.insertRowAtBeginning();
+    QCOMPARE(tableView.verticalHeader()->isSectionHidden(0), false);
+    QCOMPARE(tableView.verticalHeader()->isSectionHidden(1), false);
+    QCOMPARE(tableView.verticalHeader()->isSectionHidden(2), true);
+}
+
 void protected_QHeaderView::testVisualRegionForSelection()
 {
     QRegion r = visualRegionForSelection(QItemSelection(model()->index(1, 0), model()->index(1, 2)));