QMdiArea: update indices list before potentially operating on it

When a subwindow before the current active window is removed, the
mapping of window index to index in the activated history needs to be
updated. This was done too late, so intermediate calls (e.g. to
update the layout of the area as a side-effect of calling
updateTabBarGeometry) operated on indices that were out of bounds,
leading to asserts.

Fix this bug by updating the index list as the first thing in
updateActiveWindow() which is always called when a window gets removed.
As a drive-by, make use of the fact that C++ knows about references,
instead of taking the address of the integer in the list.

Add a test case that asserts without the fix, and passes with it.

Fixes: QTBUG-129596
Pick-to: 6.8 6.5
Change-Id: I94d78dd49f486ee4aa6ffbddf528d985a206820d
Reviewed-by: Christian Ehrlicher <ch.ehrlicher@gmx.de>

src/widgets/widgets/qmdiarea.cpp

@@ -1073,6 +1073,13 @@ void QMdiAreaPrivate::updateActiveWindow(int removedIndex, bool activeRemoved)
 {
     Q_ASSERT(indicesToActivatedChildren.size() == childWindows.size());
 
+    // Update indices list first so that we don't rely
+    for (int i = 0; i < indicesToActivatedChildren.size(); ++i) {
+        int &index = indicesToActivatedChildren[i];
+        if (index > removedIndex)
+            --index;
+    }
+
 #if QT_CONFIG(tabbar)
     if (tabBar && removedIndex >= 0) {
         const QSignalBlocker blocker(tabBar);
@@ -1099,13 +1106,6 @@ void QMdiAreaPrivate::updateActiveWindow(int removedIndex, bool activeRemoved)
             --indexToHighlighted;
     }
 
-    // Update indices list
-    for (int i = 0; i < indicesToActivatedChildren.size(); ++i) {
-        int *index = &indicesToActivatedChildren[i];
-        if (*index > removedIndex)
-            --*index;
-    }
-
     if (!activeRemoved)
         return;
 

tests/auto/widgets/widgets/qmdiarea/tst_qmdiarea.cpp

@@ -26,6 +26,8 @@
 
 #include <QtWidgets/private/qapplication_p.h>
 
+using namespace Qt::StringLiterals;
+
 static const Qt::WindowFlags DefaultWindowFlags
     = Qt::SubWindow | Qt::WindowSystemMenuHint
       | Qt::WindowTitleHint | Qt::WindowMinMaxButtonsHint | Qt::WindowCloseButtonHint;
@@ -264,6 +266,7 @@ private slots:
     void tabbedview_activefirst();
     void tabbedview_activesecond();
     void tabbedview_activethird();
+    void tabbedview_closeInactive();
 
 private:
     QMdiSubWindow *activeWindow;
@@ -2768,6 +2771,46 @@ void tst_QMdiArea::tabbedview_activethird()
     QCOMPARE(mdiArea.activeSubWindow(), sub2);
 }
 
+void tst_QMdiArea::tabbedview_closeInactive()
+{
+    QMdiArea mdiArea;
+    auto createNewWindow = [&mdiArea](const QString &name){
+        QMdiSubWindow *subWindow = new QMdiSubWindow;
+        subWindow->setObjectName(name);
+        subWindow->setAttribute(Qt::WA_DeleteOnClose);
+        subWindow->setWindowTitle(name);
+        mdiArea.addSubWindow(subWindow);
+        subWindow->show();
+        return subWindow;
+    };
+
+    mdiArea.setViewMode(QMdiArea::TabbedView);
+    mdiArea.setTabsClosable(true);
+    mdiArea.setTabPosition(QTabWidget::South);
+    mdiArea.setOption(QMdiArea::DontMaximizeSubWindowOnActivation, true);
+    mdiArea.setActivationOrder(QMdiArea::ActivationHistoryOrder);
+
+    mdiArea.resize(800, 600);
+    mdiArea.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&mdiArea));
+    // This is needed for QMdiAreaPrivate::updateTabBarGeometry to update the
+    // viewport margins.
+    mdiArea.setStyleSheet(uR"qss(
+        QTabBar::tab:bottom:selected {
+            border-bottom: 1px solid;
+        }
+    )qss"_s);
+
+    QPointer<QMdiSubWindow> mdi1 = createNewWindow(u"mdi1"_s);
+    QPointer<QMdiSubWindow> mdi2 = createNewWindow(u"mdi2"_s);
+    QTRY_COMPARE(mdiArea.subWindowList().size() , 2);
+    QCOMPARE(mdiArea.activeSubWindow(), mdi2.data());
+
+    mdi1->close();
+
+    QTRY_COMPARE(mdiArea.subWindowList().size() , 1);
+    QTRY_VERIFY(!mdi1);
+}
 
 QTEST_MAIN(tst_QMdiArea)
 #include "tst_qmdiarea.moc"