Fix crash when invalidating a QSortFilterProxyModel

Backport of d7a15fbfd93fb566c7793596ea50d8786b9eb654 from qtbase.

Task-number: QTBUG-27122
Change-Id: I0b1eda292fd9648e6f08629f7a069b66bb8b59e8
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>

2 files changed, 83 insertions, 6 deletions

diff --git a/src/gui/itemviews/qsortfilterproxymodel.cpp b/src/gui/itemviews/qsortfilterproxymodel.cpp
index d937b63..9fe224e 100644
--- a/src/gui/itemviews/qsortfilterproxymodel.cpp
+++ b/src/gui/itemviews/qsortfilterproxymodel.cpp
@@ -1036,18 +1036,34 @@ void QSortFilterProxyModelPrivate::filter_changed(const QModelIndex &source_pare
     Mapping *m = it.value();
     QSet<int> rows_removed = handle_filter_changed(m->proxy_rows, m->source_rows, source_parent, Qt::Vertical);
     QSet<int> columns_removed = handle_filter_changed(m->proxy_columns, m->source_columns, source_parent, Qt::Horizontal);
-    QVector<QModelIndex> mappedChildren = m->mapped_children;
-    QVector<QModelIndex>::iterator it2 = mappedChildren.end();
-    while (it2 != mappedChildren.begin()) {
-        --it2;
-        const QModelIndex source_child_index = *it2;
+
+    // We need to iterate over a copy of m->mapped_children because otherwise it may be changed by other code, invalidating
+    // the iterator it2.
+    // The m->mapped_children vector can be appended to with indexes which are no longer filtered
+    // out (in create_mapping) when this function recurses for child indexes.
+    const QVector<QModelIndex> mappedChildren = m->mapped_children;
+    QVector<int> indexesToRemove;
+    for (int i = 0; i < mappedChildren.size(); ++i) {
+        const QModelIndex source_child_index = mappedChildren.at(i);
         if (rows_removed.contains(source_child_index.row()) || columns_removed.contains(source_child_index.column())) {
-            it2 = mappedChildren.erase(it2);
+            indexesToRemove.push_back(i);
             remove_from_mapping(source_child_index);
         } else {
             filter_changed(source_child_index);
         }
     }
+    QVector<int>::const_iterator removeIt = indexesToRemove.constEnd();
+    const QVector<int>::const_iterator removeBegin = indexesToRemove.constBegin();
+
+    // We can't just remove these items from mappedChildren while iterating above and then
+    // do something like m->mapped_children = mappedChildren, because mapped_children might
+    // be appended to in create_mapping, and we would lose those new items.
+    // Because they are always appended in create_mapping, we can still remove them by
+    // position here.
+    while (removeIt != removeBegin) {
+        --removeIt;
+        m->mapped_children.remove(*removeIt);
+    }
 }
 
 /*!
diff --git a/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
index 8ab5098..d0887fc 100644
--- a/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -152,6 +152,7 @@ private slots:
     void testResetInternalData();
     void filteredColumns();
     void hierarchyFilterInvalidation();
+    void simpleFilterInvalidation();
 
 protected:
     void buildHierarchy(const QStringList &data, QAbstractItemModel *model);
@@ -3475,5 +3476,65 @@ void tst_QSortFilterProxyModel::hierarchyFilterInvalidation()
     proxy.setMode(true);
 }
 
+
+class FilterProxy2 : public QSortFilterProxyModel
+{
+    Q_OBJECT
+public:
+    FilterProxy2(QObject *parent = 0)
+      : QSortFilterProxyModel(parent),
+        mode(false)
+    {
+
+    }
+
+public slots:
+    void setMode(bool on)
+    {
+        mode = on;
+        invalidateFilter();
+    }
+
+protected:
+    virtual bool filterAcceptsRow ( int source_row, const QModelIndex & source_parent ) const
+    {
+        if (source_parent.isValid()) {
+            return true;
+        } else {
+            if (0 == source_row) {
+                return true;
+            } else {
+                return !mode;
+            }
+        }
+    }
+
+private:
+    bool mode;
+};
+
+void tst_QSortFilterProxyModel::simpleFilterInvalidation()
+{
+    QStandardItemModel model;
+    for (int i = 0; i < 2; ++i) {
+        QStandardItem *child = new QStandardItem(QString("Row %1").arg(i));
+        child->appendRow(new QStandardItem("child"));
+        model.appendRow(child);
+    }
+
+    FilterProxy2 proxy;
+    proxy.setSourceModel(&model);
+
+    QTreeView view;
+    view.setModel(&proxy);
+
+    view.show();
+    QTest::qWaitForWindowShown(&view);
+
+    proxy.setMode(true);
+    model.insertRow(0, new QStandardItem("extra"));
+}
+
+
 QTEST_MAIN(tst_QSortFilterProxyModel)
 #include "tst_qsortfilterproxymodel.moc"