From 8b9346c7404287b4c546b49c23f24ccbef0f8b6e Mon Sep 17 00:00:00 2001
From: Gabriel de Dietrich <gabriel.dedietrich@theqtcompany.com>
Date: Mon, 26 Oct 2015 15:28:36 +0100
Subject: [PATCH] Item Views: Avoid reentrant call in updateEditorGeometries()

This may result in incrementing an invalid iterator after the
iterator's container has changed. Also, for this to happen,
the view needs to have an active editor.

The reentrant call happens as follows in QTreeView, after the
model adds new rows to the view:

    QTreeView::rowsInserted()
    QAbstractItemView::rowsInserted()
    QAbstractItemView::updateEditorGeometries()
    QTreeView::visualRect()
    QAbstractItemViewPrivate::executePostedLayout()
    QTreeView::doItemsLayout()
    QAbstractItemView::doItemsLayout()
    QTreeView::updateGeometries()
    QAbstractItemView::updateGeometries()
    QAbstractItemView::updateEditorGeometries()

Other concrete item view classes may be prone to the same issue.

The fix consists in relayouting the items if needed, which should
trigger calling updateEditorGeometries() again. This doesn't
invalidate previous optimizations regarding item relayouting since
we only force relayouting when it'll be done by visualRect().

Change-Id: Id31507fdc8d9a84d50265298191d690d1a06792b
Task-number: QTBUG-48968
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
---
 src/widgets/itemviews/qabstractitemview.cpp   |  5 +++
 .../tst_qabstractitemview.cpp                 | 38 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp
index 3bb4d0624f8..ad7be840d0c 100644
--- a/src/widgets/itemviews/qabstractitemview.cpp
+++ b/src/widgets/itemviews/qabstractitemview.cpp
@@ -2636,6 +2636,11 @@ void QAbstractItemView::updateEditorGeometries()
     Q_D(QAbstractItemView);
     if(d->editorIndexHash.isEmpty())
         return;
+    if (d->delayedPendingLayout) {
+        // doItemsLayout() will end up calling this function again
+        d->executePostedLayout();
+        return;
+    }
     QStyleOptionViewItem option = d->viewOptionsV1();
     QEditorIndexHash::iterator it = d->editorIndexHash.begin();
     QWidgetList editorsToRelease;
diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp
index fac3f5857b9..3a17f7c690e 100644
--- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp
+++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp
@@ -250,6 +250,7 @@ private slots:
     void QTBUG39324_settingSameInstanceOfIndexWidget();
     void sizeHintChangeTriggersLayout();
     void shiftSelectionAfterChangingModelContents();
+    void QTBUG48968_reentrant_updateEditorGeometries();
 };
 
 class MyAbstractItemDelegate : public QAbstractItemDelegate
@@ -1990,5 +1991,42 @@ void tst_QAbstractItemView::shiftSelectionAfterChangingModelContents()
     QVERIFY(selected.contains(indexE));
 }
 
+void tst_QAbstractItemView::QTBUG48968_reentrant_updateEditorGeometries()
+{
+
+    QStandardItemModel *m = new QStandardItemModel(this);
+    for (int i=0; i<10; ++i) {
+        QStandardItem *item = new QStandardItem(QString("Item number %1").arg(i));
+        item->setEditable(true);
+        for (int j=0; j<5; ++j) {
+            QStandardItem *child = new QStandardItem(QString("Child Item number %1").arg(j));
+            item->setChild(j, 0, child);
+        }
+        m->setItem(i, 0, item);
+    }
+
+    QTreeView tree;
+    tree.setModel(m);
+    tree.setRootIsDecorated(false);
+    QObject::connect(&tree, SIGNAL(doubleClicked(QModelIndex)), &tree, SLOT(setRootIndex(QModelIndex)));
+    tree.show();
+    QTest::qWaitForWindowActive(&tree);
+
+    // Trigger editing idx
+    QModelIndex idx = m->index(1, 0);
+    const QPoint pos = tree.visualRect(idx).center();
+    QTest::mouseClick(tree.viewport(), Qt::LeftButton, Qt::NoModifier, pos);
+    QTest::mouseDClick(tree.viewport(), Qt::LeftButton, Qt::NoModifier, pos);
+
+    // Add more children to idx
+    QStandardItem *item = m->itemFromIndex(idx);
+    for (int j=5; j<10; ++j) {
+        QStandardItem *child = new QStandardItem(QString("Child Item number %1").arg(j));
+        item->setChild(j, 0, child);
+    }
+
+    // No crash, all fine.
+}
+
 QTEST_MAIN(tst_QAbstractItemView)
 #include "tst_qabstractitemview.moc"