QDockWidget: Fix group unplugging

A floating dock widget could either be a single QDockWidget object,
or a QDockWidgetGroupWindow with a single QDockWidget child.
The former could be dropped on the latter. Dropping the latter on
the former caused a crash.

The existence of QDockWidgetGroupWindows with a single dock widget
child was accepted to be by design.
Previous fixes, such as  9ff40b59da58160dc26c54204a615a2456e07405,
attempted to wrap all single floating dock widgets in
QDockWidgetGroupWindows.

These attempts fell short, because of the manifold programmatic and
manual options to create a floating dock widget:
- drag a single dock widget out of a main window dock area
- drag a dock widget out of a tab bar on the main window
- drag a dock widget out of a floating tab
- call `QDockWidget::setFloating(true)` in any situation
- create a new QDockWidget, that floats from the beginning

Whenever a QDockWidgetGroupWindow with a single QDockWidget child
was hovered and/or dropped on a QDockWidget without a group window,
crashes or screen artifacts were observed. Previous fixes made them
occur less often.

QDockWidgetGroupWindow is not designed to hold a single QDockWidget
child. Such a state is inconsistent and may only exist, while a
QDockWidgetGroupWindow is constructed.

The reason why such invalid QDockWidgetGroupWindows started to exist,
is a bool trap: QDockWidgetPrivate::mouseMoveEvent() starts a drag
operation, when a dock widget is moved by mouse.
It called startDrag() with no argument, which defaulted to
startDrag(true) and caused a group drag.

This assumption is
*correct*, when a tabbed group of dock widgets is dragged out of the
main dock as a whole, to become floating tabs.
*wrong*, when a single dock widget is dragged out of a docked group,
to become a single floating dock widget.

In the second case, the dock widget was wrapped in a new, floating,
invisible QDockWidgetGroupWindow. Looking like a single, floating dock
widget, the group window caused a crash, when attempted to be dropped
on another dock widget.

This patch eliminates all cases, where a QDockWidgetGroupWindow with
a single QDockWidget is created:
(1) Implement QDockWidgetPrivate::isTabbed().
This enables mouseMoveEvent to determine, whether the move relates to a
group of tabbed dock widgets, or to a single dock widget.
startDrag() can therefore be called with the right argument. It will no
longer create a QDockWidgetGroupWindow with a single QDockWidget child.

(2) Change QMainWindowTabBar::mouseReleaseEvent
When a dock widget was dragged out of a tab bar and became a single,
floating dock widget, it was still parented to the group window.
That is wrong, because it has no more relationship with that group
window.
=> Reparent it to the main window, just like any other single floating
dock widget. That enables QDockWidgetGroupWindow to detect, that the
2nd last child has gone and no more group window is needed (see next
point).

(3) React to reparenting, closing and deleting
If the second last dock widget in a floating tab gets closed (manually
or programmatically), reparented or deleted, also unplug the last one
and remove the group window.

(4) Amend 9ff40b59da58160dc26c54204a615a2456e07405
Remove the code path where a QDockWidgetGroupWindow with a single
QDockWidget child was created 'just in case', to make it compatible
others, created by (1), (2) or (3).

(5) Change QMainWindowLayout::hover()
When the hover ends without a successful drop and a temporary group
window with a single dock widget child has been created, remove the
group window.

The patch fixes smaller inconsistencies, which have not become visible
due to assertions and crashes earlier in the chain.

The patch finally extends tst_QDockWidget, to cover all 4 cases.

- Creation of floating tabs
The creation of floating tabs is extracted from floatingTabs() to
the helper function createFloatingTabs(). In addition to creating
floating tabs, the helper verifies that dragging a dock widget out
of the main window doesn't accidently wrap it in a group window.
This covers case (1).

- tst_QDockWidget::floatingTabs()
The test function verifies now, that both test dock widgets have the
same path before plugging them together and after unplugging them from
the floating tab. This covers case(4).

- tst_QDockwidget::deleteFloatingTabWithSingleDockWidget()
This test function is added, to cover cases (2) and (3).

- tst_QDockWidget::hoverWithoutDrop()
This test function hovers two floating dock widgets hover each other,
and returns the moved dock widget to its origin before releasing the
mouse. This covers case(5).

This fixes a lot of long standing bugs, making the author of this patch
modestly happy :-)

Fixes: QTBUG-118223
Fixes: QTBUG-99136
Fixes: QTBUG-118578
Fixes: QTBUG-118579
Fixes: QTBUG-56799
Fixes: QTBUG-35736
Fixes: QTBUG-63448
Fixes: QTBUG-88329
Fixes: QTBUG-88157
Fixes: QTBUG-94097
Fixes: QTBUG-44540
Fixes: QTBUG-53808
Fixes: QTBUG-72915
Fixes: QTBUG-53438
Found-by: Keith Kyzivat <keith.kyzivat@qt.io>
Found-by: Frederic Lefebvre <frederic.lefebvre@qt.io>
Pick-to: 6.6 6.5
Change-Id: I51b5f9e40cb2dbe55fb14d769541067730538463
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

1 files changed, 192 insertions, 83 deletions

diff --git a/src/widgets/widgets/qmainwindowlayout.cpp b/src/widgets/widgets/qmainwindowlayout.cpp
index 170c594d0f..6711bff009 100644
--- a/src/widgets/widgets/qmainwindowlayout.cpp
+++ b/src/widgets/widgets/qmainwindowlayout.cpp
@@ -151,6 +151,21 @@ QDebug operator<<(QDebug debug, const QMainWindowLayout *layout)
     return debug;
 }
 
+// Use this to dump item lists of all populated main window docks.
+// Use DUMP macro inside QMainWindowLayout
+#if 0
+static void dumpItemLists(const QMainWindowLayout *layout, const char *function, const char *comment)
+{
+    for (int i = 0; i < QInternal::DockCount; ++i) {
+        const auto &list = layout->layoutState.dockAreaLayout.docks[i].item_list;
+        if (list.isEmpty())
+            continue;
+        qDebug() << function << comment << "Dock" << i << list;
+    }
+}
+#define DUMP(comment) dumpItemLists(this, __FUNCTION__, comment)
+#endif // 0
+
 #endif // QT_CONFIG(dockwidget) && !defined(QT_NO_DEBUG)
 
 /******************************************************************************
@@ -403,8 +418,8 @@ void QDockWidgetGroupWindow::destroyOrHideIfEmpty()
     }
 
     // Make sure to reparent the possibly floating or hidden QDockWidgets to the parent
-    const auto dockWidgets = findChildren<QDockWidget *>(Qt::FindDirectChildrenOnly);
-    for (QDockWidget *dw : dockWidgets) {
+    const auto dockWidgetsList = dockWidgets();
+    for (QDockWidget *dw : dockWidgetsList) {
         const bool wasFloating = dw->isFloating();
         const bool wasHidden = dw->isHidden();
         dw->setParent(parentWidget());
@@ -607,6 +622,108 @@ void QDockWidgetGroupWindow::apply()
     layoutInfo()->apply(false);
 }
 
+void QDockWidgetGroupWindow::childEvent(QChildEvent *event)
+{
+    switch (event->type()) {
+    case QEvent::ChildRemoved:
+        if (auto *dockWidget = qobject_cast<QDockWidget *>(event->child()))
+            dockWidget->removeEventFilter(this);
+        destroyIfSingleItemLeft();
+        break;
+    case QEvent::ChildAdded:
+        if (auto *dockWidget = qobject_cast<QDockWidget *>(event->child()))
+            dockWidget->installEventFilter(this);
+        break;
+    default:
+        break;
+    }
+}
+
+bool QDockWidgetGroupWindow::eventFilter(QObject *obj, QEvent *event)
+{
+    auto *dockWidget = qobject_cast<QDockWidget *>(obj);
+    if (!dockWidget)
+        return QWidget::eventFilter(obj, event);
+
+    switch (event->type()) {
+    case QEvent::Close:
+        // We don't want closed dock widgets in a floating tab
+        // => dock it to the main dock, before closing;
+        reparent(dockWidget);
+        dockWidget->setFloating(false);
+        break;
+
+    case QEvent::Hide:
+        // if the dock widget is not an active tab, it is hidden anyway.
+        // if it is the active tab, hide the whole group.
+        if (dockWidget->isVisible())
+            hide();
+        break;
+
+    default:
+        break;
+    }
+    return QWidget::eventFilter(obj, event);
+}
+
+void QDockWidgetGroupWindow::destroyIfSingleItemLeft()
+{
+    const auto &dockWidgets = this->dockWidgets();
+
+    // Handle only the last dock
+    if (dockWidgets.count() != 1)
+        return;
+
+    auto *lastDockWidget = dockWidgets.at(0);
+
+    // If the last remaining dock widget is not in the group window's item_list,
+    // a group window is being docked on a main window docking area.
+    // => don't interfere
+    if (layoutInfo()->indexOf(lastDockWidget).isEmpty())
+        return;
+
+    auto *mainWindow = qobject_cast<QMainWindow *>(parentWidget());
+    QMainWindowLayout *mwLayout = qt_mainwindow_layout(mainWindow);
+
+    // Unplug the last remaining dock widget and hide the group window, to avoid flickering
+    mwLayout->unplug(lastDockWidget, QDockWidgetPrivate::DragScope::Widget);
+    lastDockWidget->setGeometry(geometry());
+    hide();
+
+    // Get the layout info for the main window dock, where dock widgets need to go
+    QDockAreaLayoutInfo &parentInfo = mwLayout->layoutState.dockAreaLayout.docks[layoutInfo()->dockPos];
+
+    // Re-parent last dock widget
+    reparent(lastDockWidget);
+
+    // the group window could still have placeholder items => clear everything
+    layoutInfo()->item_list.clear();
+
+    // remove the group window and the dock's item_list pointing to it.
+    parentInfo.remove(this);
+    destroyOrHideIfEmpty();
+}
+
+void QDockWidgetGroupWindow::reparent(QDockWidget *dockWidget)
+{
+    // reparent a dockWidget to the main window
+    // - remove it from the floating dock's layout info
+    // - insert it to the main dock's layout info
+    // Finally, set draggingDock to nullptr, since the drag is finished.
+    auto *mainWindow = qobject_cast<QMainWindow *>(parentWidget());
+    Q_ASSERT(mainWindow);
+    QMainWindowLayout *mwLayout = qt_mainwindow_layout(mainWindow);
+    Q_ASSERT(mwLayout);
+    QDockAreaLayoutInfo &parentInfo = mwLayout->layoutState.dockAreaLayout.docks[layoutInfo()->dockPos];
+    dockWidget->removeEventFilter(this);
+    parentInfo.add(dockWidget);
+    layoutInfo()->remove(dockWidget);
+    const bool wasFloating = dockWidget->isFloating();
+    const bool wasVisible = dockWidget->isVisible();
+    dockWidget->setParent(mainWindow);
+    dockWidget->setFloating(wasFloating);
+    dockWidget->setVisible(wasVisible);
+}
 #endif
 
 /******************************************************************************
@@ -1745,11 +1862,14 @@ void QMainWindowLayout::keepSize(QDockWidget *w)
 // Handle custom tooltip, and allow to drag tabs away.
 class QMainWindowTabBar : public QTabBar
 {
+    Q_OBJECT
     QMainWindow *mainWindow;
     QPointer<QDockWidget> draggingDock; // Currently dragging (detached) dock widget
     ~QMainWindowTabBar();
 public:
     QMainWindowTabBar(QMainWindow *parent);
+    QDockWidget *dockAt(int index) const;
+    QList<QDockWidget *> dockWidgets() const;
 protected:
     bool event(QEvent *e) override;
     void mouseReleaseEvent(QMouseEvent*) override;
@@ -1763,6 +1883,29 @@ QMainWindowTabBar::QMainWindowTabBar(QMainWindow *parent)
     setExpanding(false);
 }
 
+QList<QDockWidget *> QMainWindowTabBar::dockWidgets() const
+{
+    QList<QDockWidget *> docks;
+    for (int i = 0; i < count(); ++i) {
+        if (QDockWidget *dock = dockAt(i))
+            docks << dock;
+    }
+    return docks;
+}
+
+QDockWidget *QMainWindowTabBar::dockAt(int index) const
+{
+    QMainWindowTabBar *that = const_cast<QMainWindowTabBar *>(this);
+    QMainWindowLayout* mlayout = qt_mainwindow_layout(mainWindow);
+    QDockAreaLayoutInfo *info = mlayout->dockInfo(that);
+    if (!info)
+        return nullptr;
+    const int itemIndex = info->tabIndexToListIndex(index);
+    Q_ASSERT(itemIndex >= 0 && itemIndex < info->item_list.count());
+    const QDockAreaLayoutItem &item = info->item_list.at(itemIndex);
+    return item.widgetItem ? qobject_cast<QDockWidget *>(item.widgetItem->widget()) : nullptr;
+}
+
 void QMainWindowTabBar::mouseMoveEvent(QMouseEvent *e)
 {
     // The QTabBar handles the moving (reordering) of tabs.
@@ -1776,13 +1919,8 @@ void QMainWindowTabBar::mouseMoveEvent(QMouseEvent *e)
         offset *= 3;
         QRect r = rect().adjusted(-offset, -offset, offset, offset);
         if (d->dragInProgress && !r.contains(e->position().toPoint()) && d->validIndex(d->pressedIndex)) {
-            QMainWindowLayout* mlayout = qt_mainwindow_layout(mainWindow);
-            QDockAreaLayoutInfo *info = mlayout->dockInfo(this);
-            Q_ASSERT(info);
-            int idx = info->tabIndexToListIndex(d->pressedIndex);
-            const QDockAreaLayoutItem &item = info->item_list.at(idx);
-            if (item.widgetItem
-                    && (draggingDock = qobject_cast<QDockWidget *>(item.widgetItem->widget()))) {
+            draggingDock = dockAt(d->pressedIndex);
+            if (draggingDock) {
                 // We should drag this QDockWidget away by unpluging it.
                 // First cancel the QTabBar's internal move
                 d->moveTabFinished(d->pressedIndex);
@@ -1858,6 +1996,23 @@ bool QMainWindowTabBar::event(QEvent *e)
     return true;
 }
 
+bool QMainWindowLayout::isDockWidgetTabbed(const QDockWidget *dockWidget) const
+{
+    for (auto *bar : std::as_const(usedTabBars)) {
+        // A single dock widget in a tab bar is not considered to be tabbed.
+        // This is to make sure, we don't drag an empty QDockWidgetGroupWindow around.
+        // => only consider tab bars with two or more tabs.
+        if (bar->count() <= 1)
+            continue;
+        auto *tabBar = qobject_cast<QMainWindowTabBar *>(bar);
+        Q_ASSERT(tabBar);
+        const auto dockWidgets = tabBar->dockWidgets();
+        if (std::find(dockWidgets.begin(), dockWidgets.end(), dockWidget) != dockWidgets.end())
+            return true;
+    }
+    return false;
+}
+
 QTabBar *QMainWindowLayout::getTabBar()
 {
     if (!usedTabBars.isEmpty() && !isInRestoreState) {
@@ -2596,73 +2751,10 @@ QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, QDockWidgetPrivate::Drag
             // We are unplugging a single dock widget from a floating window.
             QDockWidget *dockWidget = qobject_cast<QDockWidget *>(widget);
             Q_ASSERT(dockWidget); // cannot be a QDockWidgetGroupWindow because it's not floating.
-
-            // unplug the widget first
             dockWidget->d_func()->unplug(widget->geometry());
 
-            // Create a floating tab, copy properties and generate layout info
-            QDockWidgetGroupWindow *floatingTabs = createTabbedDockWindow();
-            const QInternal::DockPosition dockPos = groupWindow->layoutInfo()->dockPos;
-            QDockAreaLayoutInfo *info = floatingTabs->layoutInfo();
-
-            const QTabBar::Shape shape = tabwidgetPositionToTabBarShape(dockWidget);
-
-            // Populate newly created DockAreaLayoutInfo of floating tabs
-            *info = QDockAreaLayoutInfo(&layoutState.dockAreaLayout.sep, dockPos,
-                                        Qt::Horizontal, shape,
-                                        layoutState.mainWindow);
-
-            // Create tab and hide it as group window contains only one widget
-            info->tabbed = true;
-            info->tabBar = getTabBar();
-            info->tabBar->hide();
-            updateGapIndicator();
-
-            // Reparent it to a QDockWidgetGroupLayout
-            floatingTabs->setGeometry(dockWidget->geometry());
-
-            // Append reference to floatingTabs to the dock's item_list
-            parentItem.widgetItem = new QDockWidgetGroupWindowItem(floatingTabs);
-            layoutState.dockAreaLayout.docks[dockPos].item_list.append(parentItem);
-
-            // use populated parentItem to set reference to dockWidget as the first item in own list
-            parentItem.widgetItem = new QDockWidgetItem(dockWidget);
-            info->item_list = {parentItem};
-
-            // Add non-gap items of the dock to the tab bar
-            for (const auto &listItem : layoutState.dockAreaLayout.docks[dockPos].item_list) {
-                if (listItem.GapItem || !listItem.widgetItem)
-                    continue;
-                info->tabBar->addTab(listItem.widgetItem->widget()->objectName());
-            }
-
-            // Re-parent and fit
-            floatingTabs->setParent(layoutState.mainWindow);
-            floatingTabs->layoutInfo()->fitItems();
-            floatingTabs->layoutInfo()->apply(dockOptions & QMainWindow::AnimatedDocks);
-            groupWindow->layoutInfo()->fitItems();
-            groupWindow->layoutInfo()->apply(dockOptions & QMainWindow::AnimatedDocks);
-            dockWidget->d_func()->tabPosition = layoutState.mainWindow->tabPosition(toDockWidgetArea(dockPos));
-            info->reparentWidgets(floatingTabs);
-            dockWidget->setParent(floatingTabs);
-            info->updateTabBar();
-
-            // Show the new item
-            const QList<int> path = layoutState.indexOf(floatingTabs);
-            QRect r = layoutState.itemRect(path);
-            savedState = layoutState;
-            savedState.fitLayout();
-
-            // Update gap, fix orientation, raise and show
-            currentGapPos = path;
-            currentGapRect = r;
-            updateGapIndicator();
-            fixToolBarOrientation(parentItem.widgetItem, currentGapPos.at(1));
-            floatingTabs->show();
-            floatingTabs->raise();
-
             qCDebug(lcQpaDockWidgets) << "Unplugged from floating dock:" << widget << "from" << parentItem.widgetItem;
-            return parentItem.widgetItem;
+            return item;
         }
     }
 #endif
@@ -2815,7 +2907,7 @@ void QMainWindowLayout::hover(QLayoutItem *hoverTarget,
                 continue;
 
             // Check permission to dock on another dock widget or floating dock
-            // FIXME in 6.4
+            // FIXME in Qt 7
 
             if (w != widget && w->isWindow() && w->isVisible() && !w->isMinimized())
                 candidates << w;
@@ -2853,16 +2945,26 @@ void QMainWindowLayout::hover(QLayoutItem *hoverTarget,
                     floatingTabs->setGeometry(dropTo->geometry());
                     QDockAreaLayoutInfo *info = floatingTabs->layoutInfo();
                     const QTabBar::Shape shape = tabwidgetPositionToTabBarShape(dropTo);
-                    const QInternal::DockPosition dockPosition = toDockPos(dockWidgetArea(dropTo));
+
+                    // dropTo and widget may be in a state where they transition
+                    // from being a group window child to a single floating dock widget.
+                    // In that case, their path to a main window dock may not have been
+                    // updated yet.
+                    // => ask both and fall back to dock 1 (right dock)
+                    QInternal::DockPosition dockPosition = toDockPos(dockWidgetArea(dropTo));
+                    if (dockPosition == QInternal::DockPosition::DockCount)
+                        dockPosition = toDockPos(dockWidgetArea(widget));
+                    if (dockPosition == QInternal::DockPosition::DockCount)
+                        dockPosition = QInternal::DockPosition::RightDock;
+
                     *info = QDockAreaLayoutInfo(&layoutState.dockAreaLayout.sep, dockPosition,
                                                 Qt::Horizontal, shape,
                                                 static_cast<QMainWindow *>(parentWidget()));
                     info->tabBar = getTabBar();
                     info->tabbed = true;
-                    QLayout *parentLayout = dropTo->parentWidget()->layout();
-                    info->item_list.append(
-                        QDockAreaLayoutItem(parentLayout->takeAt(parentLayout->indexOf(dropTo))));
-
+                    info->add(dropTo);
+                    QDockAreaLayoutInfo &parentInfo = layoutState.dockAreaLayout.docks[dockPosition];
+                    parentInfo.add(floatingTabs);
                     dropTo->setParent(floatingTabs);
                     qCDebug(lcQpaDockWidgets) << "Wrapping" << widget << "into floating tabs" << floatingTabs;
                     w = floatingTabs;
@@ -2875,15 +2977,21 @@ void QMainWindowLayout::hover(QLayoutItem *hoverTarget,
                 qCDebug(lcQpaDockWidgets) << "Raising" << widget;
             }
 #endif
-            auto group = qobject_cast<QDockWidgetGroupWindow *>(w);
-            Q_ASSERT(group);
-            if (group->hover(hoverTarget, group->mapFromGlobal(mousePos))) {
-                setCurrentHoveredFloat(group);
+            auto *groupWindow = qobject_cast<QDockWidgetGroupWindow *>(w);
+            Q_ASSERT(groupWindow);
+            if (groupWindow->hover(hoverTarget, groupWindow->mapFromGlobal(mousePos))) {
+                setCurrentHoveredFloat(groupWindow);
                 applyState(layoutState); // update the tabbars
             }
             return;
         }
     }
+
+    // If a temporary group window has been created during a hover,
+    // remove it, if it has only one dockwidget child
+    if (currentHoveredFloat)
+        currentHoveredFloat->destroyIfSingleItemLeft();
+
     setCurrentHoveredFloat(nullptr);
     layoutState.dockAreaLayout.fallbackToSizeHints = false;
 #endif // QT_CONFIG(dockwidget)
@@ -3091,4 +3199,5 @@ Qt::DropAction QMainWindowLayout::performPlatformWidgetDrag(QLayoutItem *widgetI
 
 QT_END_NAMESPACE
 
+#include "qmainwindowlayout.moc"
 #include "moc_qmainwindowlayout_p.cpp"