QMainWindow: don't crash when restored state is modified before applied

Amends 32edae5e26, after which we keep a
copy of the restored state if the state couldn't be applied yet. Since
making a copy of the entire state results in multiple copies of layout
item pointers, we might end up with dangling pointers if the layout
structure is modified while we keep the copy. This can happen if methods
such as tabifyDockWidgets or splitDockWidget get called; e.g. tabifying
dock widgets will destroy the layout items that were added for them.

Unfortunately, the layout items do not have a pointer back to the layout
they live in, and the items in the stored state might not yet live in a
layout anyway. So we cannot remove the items from their layout in a
QDockWidgetItem destructor implementation.

Instead, we have to forget the stored state. Add a helper function that
writes the stored state back to the actual state, and deletes the stored
state afterwards. Call this function when the layout might get modified
programmatically.

Add a test case that reproduces the crash without the fix, and passes
with the patch.

Fixes: QTBUG-120025
Pick-to: 6.7 6.6 6.5
Change-Id: I8f7e886f3c4ac38e25f9b8bc194eea0833e5974f
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>

src/widgets/widgets/qmainwindowlayout.cpp

@@ -1741,6 +1741,7 @@ bool QMainWindowLayout::restoreDockWidget(QDockWidget *dockwidget)
 #if QT_CONFIG(tabbar)
 void QMainWindowLayout::tabifyDockWidget(QDockWidget *first, QDockWidget *second)
 {
+    applyRestoredState();
     addChildWidget(second);
     layoutState.dockAreaLayout.tabifyDockWidget(first, second);
     emit second->dockLocationChanged(dockWidgetArea(first));
@@ -1862,6 +1863,7 @@ void QMainWindowLayout::splitDockWidget(QDockWidget *after,
                                         QDockWidget *dockwidget,
                                         Qt::Orientation orientation)
 {
+    applyRestoredState();
     addChildWidget(dockwidget);
     layoutState.dockAreaLayout.splitDockWidget(after, dockwidget, orientation);
     emit dockwidget->dockLocationChanged(dockWidgetArea(after));
@@ -2211,6 +2213,32 @@ QLayoutItem *QMainWindowLayout::takeAt(int index)
     return nullptr;
 }
 
+
+/*!
+    \internal
+
+    restoredState stores what we earlier read from storage, but it couldn't
+    be applied as the mainwindow wasn't large enough (yet) to fit the state.
+    Usually, the restored state would be applied lazily in setGeometry below.
+    However, if the mainwindow's layout is modified (e.g. by a call to tabify or
+    splitDockWidgets), then we have to forget the restored state as it might contain
+    dangling pointers (QDockWidgetLayoutItem has a copy constructor that copies the
+    layout item pointer, and splitting or tabify might have to delete some of those
+    layout structures).
+
+    Functions that might result in the QMainWindowLayoutState storing dangling pointers
+    have to call this function first, so that the restoredState becomes the actual state
+    first, and is forgotten afterwards.
+*/
+void QMainWindowLayout::applyRestoredState()
+{
+    if (restoredState) {
+        layoutState = *restoredState;
+        restoredState.reset();
+        discardRestoredStateTimer.stop();
+    }
+}
+
 void QMainWindowLayout::setGeometry(const QRect &_r)
 {
     if (savedState.isValid())

src/widgets/widgets/qmainwindowlayout_p.h

@@ -587,6 +587,7 @@ public:
     QLayoutItem *unplug(QWidget *widget, QDockWidgetPrivate::DragScope scope);
     void revert(QLayoutItem *widgetItem);
     void applyState(QMainWindowLayoutState &newState, bool animate = true);
+    void applyRestoredState();
     void restore(bool keepSavedState = false);
     void animationFinished(QWidget *widget);
 

tests/auto/widgets/widgets/qmainwindow/tst_qmainwindow.cpp

@@ -104,6 +104,7 @@ private slots:
     void restoreStateFromPreviousVersion();
     void restoreStateSizeChanged_data();
     void restoreStateSizeChanged();
+    void restoreAndModify();
     void createPopupMenu();
     void hideBeforeLayout();
 #ifdef QT_BUILD_INTERNAL
@@ -1476,6 +1477,70 @@ void tst_QMainWindow::restoreStateSizeChanged()
     }
 }
 
+/*!
+    If a main window's state is restored but also modified, then we
+    might have to forget the restored state to avoid dangling pointers.
+    See comment in QMainWindowLayout::applyRestoredState() and QTBUG-120025.
+*/
+void tst_QMainWindow::restoreAndModify()
+{
+    class MainWindow : public QMainWindow
+    {
+    public:
+        MainWindow()
+        {
+            setCentralWidget(new QTextEdit);
+
+            customers = new QDockWidget(tr("Customers"), this);
+            customers->setObjectName("Customers");
+            customers->setAllowedAreas(Qt::LeftDockWidgetArea |
+                                                  Qt::RightDockWidgetArea);
+            customers->setWidget(new QTextEdit);
+            addDockWidget(Qt::RightDockWidgetArea, customers);
+
+            paragraphs = new QDockWidget(tr("Paragraphs"), this);
+            paragraphs->setObjectName("Paragraphs");
+            paragraphs->setWidget(new QTextEdit);
+            addDockWidget(Qt::RightDockWidgetArea, paragraphs);
+        }
+
+        void restore()
+        {
+            if (!savedGeometry.isEmpty())
+                restoreGeometry(savedGeometry);
+            setWindowState(Qt::WindowMaximized);
+            if (!savedState.isEmpty())
+                restoreState(savedState);
+
+            tabifyDockWidget(customers, paragraphs);
+        }
+    protected:
+        void closeEvent(QCloseEvent *event) override
+        {
+            savedGeometry = saveGeometry();
+            savedState = saveState();
+
+            return QMainWindow::closeEvent(event);
+        }
+    private:
+        QByteArray savedGeometry;
+        QByteArray savedState;
+
+        QDockWidget *customers;
+        QDockWidget *paragraphs;
+
+    } mainWindow;
+
+    mainWindow.restore();
+    mainWindow.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&mainWindow));
+    mainWindow.close();
+
+    mainWindow.restore();
+    mainWindow.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&mainWindow));
+}
+
 void tst_QMainWindow::createPopupMenu()
 {
     {