QDockWidget: Remove "group" bool trap

The unplug() and startDrag() functions of QMainWindowLayout and
QDockWidget used a boolean argument specifying whether a single dock
widget or a group of dock widgets should be unplugged. The argument
defaulted to true.

That has lead to inconsistent unplug operations, broken item_lists and
crashes, especially when the methods were called without an argument.

To improve code readability, replace bool trap with a meaningful
enum. Remove default arguments, in order to force explicit calls.

This patch does not change behavior, it is just carved out to
facilitate reviews.

Task-number: QTBUG-118578
Task-number: QTBUG-118579
Pick-to: 6.5
Change-Id: I50341b055f0bb76c2797b2fb1126a10de1fee7dd
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit c93ab8c2a015b40b9a487ed9f23a72aebea8d52a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

5 files changed, 19 insertions, 13 deletions

diff --git a/src/widgets/widgets/qdockwidget.cpp b/src/widgets/widgets/qdockwidget.cpp
index 89e696950f..2e2216f47b 100644
--- a/src/widgets/widgets/qdockwidget.cpp
+++ b/src/widgets/widgets/qdockwidget.cpp
@@ -770,7 +770,7 @@ void QDockWidgetPrivate::initDrag(const QPoint &pos, bool nca)
     tabbed widgets, and false if the dock widget should always be dragged
     alone.
  */
-void QDockWidgetPrivate::startDrag(bool group)
+void QDockWidgetPrivate::startDrag(DragScope scope)
 {
     Q_Q(QDockWidget);
 
@@ -782,7 +782,7 @@ void QDockWidgetPrivate::startDrag(bool group)
 
     bool wasFloating = q->isFloating();
 
-    state->widgetItem = layout->unplug(q, group);
+    state->widgetItem = layout->unplug(q, scope);
     if (state->widgetItem == nullptr) {
         /*  Dock widget has a QMainWindow parent, but was never inserted with
             QMainWindow::addDockWidget, so the QMainWindowLayout has no
@@ -996,7 +996,7 @@ bool QDockWidgetPrivate::mouseMoveEvent(QMouseEvent *event)
             } else
 #endif
             {
-                startDrag();
+                startDrag(DragScope::Group);
                 q->grabMouse();
                 ret = true;
             }
@@ -1105,7 +1105,7 @@ void QDockWidgetPrivate::nonClientAreaMouseEvent(QMouseEvent *event)
                 break;
             state->ctrlDrag = (event->modifiers() & Qt::ControlModifier) ||
                               (!hasFeature(this, QDockWidget::DockWidgetMovable) && q->isFloating());
-            startDrag();
+            startDrag(DragScope::Group);
             break;
         case QEvent::NonClientAreaMouseMove:
             if (state == nullptr || !state->dragging)
diff --git a/src/widgets/widgets/qdockwidget_p.h b/src/widgets/widgets/qdockwidget_p.h
index 326c55cc78..d03ac54586 100644
--- a/src/widgets/widgets/qdockwidget_p.h
+++ b/src/widgets/widgets/qdockwidget_p.h
@@ -51,6 +51,11 @@ class QDockWidgetPrivate : public QWidgetPrivate
     };
 
 public:
+    enum class DragScope {
+        Group,
+        Widget
+    };
+
     void init();
     void _q_toggleView(bool); // private slot
     void _q_toggleTopLevel(); // private slot
@@ -86,7 +91,7 @@ public:
     void setWindowState(bool floating, bool unplug = false, const QRect &rect = QRect());
     void nonClientAreaMouseEvent(QMouseEvent *event);
     void initDrag(const QPoint &pos, bool nca);
-    void startDrag(bool group = true);
+    void startDrag(DragScope scope);
     void endDrag(bool abort = false);
     void moveEvent(QMoveEvent *event);
     void recalculatePressPos(QResizeEvent *event);
diff --git a/src/widgets/widgets/qmainwindowlayout.cpp b/src/widgets/widgets/qmainwindowlayout.cpp
index f51fc41494..fc0bb07e86 100644
--- a/src/widgets/widgets/qmainwindowlayout.cpp
+++ b/src/widgets/widgets/qmainwindowlayout.cpp
@@ -1792,7 +1792,7 @@ void QMainWindowTabBar::mouseMoveEvent(QMouseEvent *e)
                 QDockWidgetPrivate *dockPriv = static_cast<QDockWidgetPrivate *>(QObjectPrivate::get(draggingDock));
                 QDockWidgetLayout *dwlayout = static_cast<QDockWidgetLayout *>(draggingDock->layout());
                 dockPriv->initDrag(dwlayout->titleArea().center(), true);
-                dockPriv->startDrag(false);
+                dockPriv->startDrag(QDockWidgetPrivate::DragScope::Widget);
                 if (dockPriv->state)
                     dockPriv->state->ctrlDrag = e->modifiers() & Qt::ControlModifier;
             }
@@ -2552,25 +2552,25 @@ static QTabBar::Shape tabwidgetPositionToTabBarShape(QWidget *w)
     Returns the QLayoutItem of the dragged element.
     The layout item is kept in the layout but set as a gap item.
  */
-QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, bool group)
+QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, QDockWidgetPrivate::DragScope scope)
 {
 #if QT_CONFIG(dockwidget) && QT_CONFIG(tabwidget)
     auto *groupWindow = qobject_cast<const QDockWidgetGroupWindow *>(widget->parentWidget());
     if (!widget->isWindow() && groupWindow) {
-        if (group && groupWindow->tabLayoutInfo()) {
+        if (scope == QDockWidgetPrivate::DragScope::Group && groupWindow->tabLayoutInfo()) {
             // We are just dragging a floating window as it, not need to do anything, we just have to
             // look up the corresponding QWidgetItem* if it exists
             if (QDockAreaLayoutInfo *info = dockInfo(widget->parentWidget())) {
                 QList<int> groupWindowPath = info->indexOf(widget->parentWidget());
                 return groupWindowPath.isEmpty() ? nullptr : info->item(groupWindowPath).widgetItem;
             }
-            qCDebug(lcQpaDockWidgets) << "Drag only:" << widget << "Group:" << group;
+            qCDebug(lcQpaDockWidgets) << "Drag only:" << widget << "Group:" << (scope == QDockWidgetPrivate::DragScope::Group);
             return nullptr;
         }
         QList<int> path = groupWindow->layoutInfo()->indexOf(widget);
         QDockAreaLayoutItem parentItem = groupWindow->layoutInfo()->item(path);
         QLayoutItem *item = parentItem.widgetItem;
-        if (group && path.size() > 1
+        if (scope == QDockWidgetPrivate::DragScope::Group && path.size() > 1
             && unplugGroup(this, &item, parentItem)) {
             qCDebug(lcQpaDockWidgets) << "Unplugging:" << widget << "from" << item;
             return item;
@@ -2663,7 +2663,7 @@ QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, bool group)
     if (QDockWidget *dw = qobject_cast<QDockWidget*>(widget)) {
         Q_ASSERT(path.constFirst() == 1);
 #if QT_CONFIG(tabwidget)
-        if (group && (dockOptions & QMainWindow::GroupedDragging) && path.size() > 3
+        if (scope == QDockWidgetPrivate::DragScope::Group && (dockOptions & QMainWindow::GroupedDragging) && path.size() > 3
             && unplugGroup(this, &item,
                            layoutState.dockAreaLayout.item(path.mid(1, path.size() - 2)))) {
             path.removeLast();
diff --git a/src/widgets/widgets/qmainwindowlayout_p.h b/src/widgets/widgets/qmainwindowlayout_p.h
index e554d2b30e..4d2fc614e4 100644
--- a/src/widgets/widgets/qmainwindowlayout_p.h
+++ b/src/widgets/widgets/qmainwindowlayout_p.h
@@ -29,6 +29,7 @@
 #include "QtCore/qset.h"
 #include "private/qlayoutengine_p.h"
 #include "private/qwidgetanimator_p.h"
+#include "private/qdockwidget_p.h"
 
 #if QT_CONFIG(dockwidget)
 #include "qdockarealayout_p.h"
@@ -569,7 +570,7 @@ public:
 
     void hover(QLayoutItem *hoverTarget, const QPoint &mousePos);
     bool plug(QLayoutItem *widgetItem);
-    QLayoutItem *unplug(QWidget *widget, bool group = false);
+    QLayoutItem *unplug(QWidget *widget, QDockWidgetPrivate::DragScope scope);
     void revert(QLayoutItem *widgetItem);
     void applyState(QMainWindowLayoutState &newState, bool animate = true);
     void restore(bool keepSavedState = false);
diff --git a/src/widgets/widgets/qtoolbar.cpp b/src/widgets/widgets/qtoolbar.cpp
index c2def3c6d4..2bef4dc201 100644
--- a/src/widgets/widgets/qtoolbar.cpp
+++ b/src/widgets/widgets/qtoolbar.cpp
@@ -181,7 +181,7 @@ void QToolBarPrivate::startDrag(bool moving)
     const bool wasFloating = q->isFloating();
 
     if (!moving) {
-        state->widgetItem = layout->unplug(q);
+        state->widgetItem = layout->unplug(q, QDockWidgetPrivate::DragScope::Group);
         Q_ASSERT(state->widgetItem != nullptr);
     }
     state->dragging = !moving;