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

@@ -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;

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"