From ec16b18d824397459daff6631a789db93638118c Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Mon, 21 Oct 2024 13:49:03 +0200
Subject: [PATCH] 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.5
Change-Id: I94d78dd49f486ee4aa6ffbddf528d985a206820d
Reviewed-by: Christian Ehrlicher <ch.ehrlicher@gmx.de>
(cherry picked from commit 665e1ea7a45d4f196958bf2b6d8f15c69d77340e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/widgets/widgets/qmdiarea.cpp              | 14 +++---
 .../widgets/widgets/qmdiarea/tst_qmdiarea.cpp | 43 +++++++++++++++++++
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/src/widgets/widgets/qmdiarea.cpp b/src/widgets/widgets/qmdiarea.cpp
index 79b83453aca..865efa01917 100644
--- a/src/widgets/widgets/qmdiarea.cpp
+++ b/src/widgets/widgets/qmdiarea.cpp
@@ -1075,6 +1075,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);
@@ -1101,13 +1108,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;
 
diff --git a/tests/auto/widgets/widgets/qmdiarea/tst_qmdiarea.cpp b/tests/auto/widgets/widgets/qmdiarea/tst_qmdiarea.cpp
index ef00dcaac0f..af0b9ce3670 100644
--- a/tests/auto/widgets/widgets/qmdiarea/tst_qmdiarea.cpp
+++ b/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;
@@ -2784,6 +2787,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"