From 7ba75d088c3eba81a1d2bb708119442991d9f30b Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Thu, 2 Sep 2021 13:21:46 +0200 Subject: [PATCH] QWidget: close the QWindow in QWidget::close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to close the window, end full screen mode on macOS, and free platform resources. This is all done by QWindow::close. QWindow::close closes the platform window, triggering a closeEvent to QWidgetWindow, which then calls QWidgetPrivate::close_helper. This way, closing a window via QWidget::close, QWindow::close, or interactively by the user are all equivalent. The QCloseEvent generated by the widget needs to be spontaneous for window-system generated events (i.e. the user clicked the close button), and non-spontaneous if the window closes because of a call to QWindow::close. To keep track of whether the event originated in an explicit call to QWindow::close, add a boolean to the QWindowPrivate. Add a test case that verifies that the window resources is destroyed, and that events are delivered as they should. Done-with: Morten Johan Sørvig Fixes: QTBUG-46701 Pick-to: 6.2 Change-Id: Iacb6a2c8d5e880b16b0c8f0c9257ed94bed36f5b Reviewed-by: Tor Arne Vestbø --- src/gui/kernel/qwindow.cpp | 1 + src/gui/kernel/qwindow_p.h | 1 + src/widgets/kernel/qwidget.cpp | 25 +++++++ src/widgets/kernel/qwidgetwindow.cpp | 4 +- .../qwidget_window/tst_qwidget_window.cpp | 66 +++++++++++++++++++ 5 files changed, 96 insertions(+), 1 deletion(-) diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp index 7f6e5044cb..845627756e 100644 --- a/src/gui/kernel/qwindow.cpp +++ b/src/gui/kernel/qwindow.cpp @@ -2254,6 +2254,7 @@ bool QWindow::close() if (!d->platformWindow) return true; + QBoolBlocker inCloseReset(d->inClose); return d->platformWindow->close(); } diff --git a/src/gui/kernel/qwindow_p.h b/src/gui/kernel/qwindow_p.h index 5d9de82295..82d4bd430c 100644 --- a/src/gui/kernel/qwindow_p.h +++ b/src/gui/kernel/qwindow_p.h @@ -136,6 +136,7 @@ public: bool visible= false; bool visibilityOnDestroy = false; bool exposed = false; + bool inClose = false; QSurfaceFormat requestedFormat; QString windowTitle; QString windowFilePath; diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 61a8605d22..c96b422b14 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -8360,6 +8360,24 @@ void QWidgetPrivate::hideChildren(bool spontaneous) } } +/*! + \internal + + For windows, this is called from the QWidgetWindow::handleCloseEvent implementation, + which QWidget::close indirectly calls by closing the QWindow. \a mode will be + CloseWithEvent if QWidgetWindow::handleCloseEvent is called indirectly by + QWindow::close, and CloseWithSpontaneousEvent if the close event originates from the + system (i.e. the user clicked the close button in the title bar). + + QDialog calls this method directly in its hide() implementation, which might be + called from the QDialog::closeEvent override. \a mode will be set to CloseNoEvent + to prevent recursion. + + For non-windows, this is called directly by QWidget::close, and \a mode will be + CloseWithEvent. + + The function is also called by the QWidget destructor, with \a mode set to CloseNoEvent. +*/ bool QWidgetPrivate::close_helper(CloseMode mode) { if (data.is_closing) @@ -8384,6 +8402,7 @@ bool QWidgetPrivate::close_helper(CloseMode mode) } } + // even for windows, make sure we deliver a hide event and that all children get hidden if (!that.isNull() && !q->isHidden()) q->hide(); @@ -8446,6 +8465,12 @@ bool QWidgetPrivate::close_helper(CloseMode mode) bool QWidget::close() { + // Close native widgets via QWindow::close() in order to run QWindow + // close code. The QWidget-specific close code in close_helper() will + // in this case be called from the Close event handler in QWidgetWindow. + if (QWindow *widgetWindow = windowHandle()) + return widgetWindow->close(); + return d_func()->close_helper(QWidgetPrivate::CloseWithEvent); } diff --git a/src/widgets/kernel/qwidgetwindow.cpp b/src/widgets/kernel/qwidgetwindow.cpp index 346289cafe..029d152f5d 100644 --- a/src/widgets/kernel/qwidgetwindow.cpp +++ b/src/widgets/kernel/qwidgetwindow.cpp @@ -840,7 +840,9 @@ void QWidgetWindow::handleResizeEvent(QResizeEvent *event) void QWidgetWindow::handleCloseEvent(QCloseEvent *event) { - bool is_closing = m_widget->d_func()->close_helper(QWidgetPrivate::CloseWithSpontaneousEvent); + Q_D(QWidgetWindow); + bool is_closing = m_widget->d_func()->close_helper(d->inClose ? QWidgetPrivate::CloseWithEvent + : QWidgetPrivate::CloseWithSpontaneousEvent); event->setAccepted(is_closing); } diff --git a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp b/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp index dbd541c736..6d6affa23a 100644 --- a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp +++ b/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp @@ -90,6 +90,8 @@ private slots: void tst_show_resize(); void tst_show_resize_hide_show(); + void close(); + void tst_windowFilePathAndwindowTitle_data(); void tst_windowFilePathAndwindowTitle(); void tst_windowFilePath_data(); @@ -247,6 +249,70 @@ void tst_QWidget_window::tst_show_resize_hide_show() QCOMPARE(w.size(), m_testWidgetSize); } +void tst_QWidget_window::close() +{ + // Verfy that closing a QWidgetWindow deletes its platform window, + // as expected of a QWindow subclass. This must be done also + // if QWidget API is used to close. The QCloseEvent must not be + // spontaneous if the close is triggered by a Qt API that the application + // would call in response to an event, and spontaneous if it is directly + // caused by user interaction, such as clicking the (x) in the titlebar. + // We can simulate this only by generating a WindowSystemEvent. + // Children of the window should get a hide event (never spontaneous when + // caused by closing the window). + + struct Widget : public QWidget + { + using QWidget::QWidget; + int spontClose = -1; + int spontHide = -1; + protected: + void hideEvent(QHideEvent *e) + { spontHide = e->spontaneous() ? 1 : 0; } + void closeEvent(QCloseEvent *e) + { spontClose = e->spontaneous() ? 1 : 0; } + }; + + // QWindow::close() + { + Widget w; + Widget child(&w); + w.winId(); + QVERIFY(w.windowHandle()); + QVERIFY(w.windowHandle()->handle()); + w.windowHandle()->close(); + QCOMPARE(w.spontClose, 0); + QCOMPARE(child.spontHide, -1); // was never shown + QVERIFY(w.windowHandle()); + QVERIFY(!w.windowHandle()->handle()); + } + + // QWidget::close() + { + Widget w; + Widget child(&w); + w.show(); + QVERIFY(w.windowHandle()); + QVERIFY(w.windowHandle()->handle()); + w.close(); + QCOMPARE(w.spontClose, 0); + QCOMPARE(child.spontHide, 0); + QVERIFY(w.windowHandle()); + QVERIFY(!w.windowHandle()->handle()); + } + + // User-initiated close + { + Widget w; + Widget child(&w); + w.show(); + QWindowSystemInterface::handleCloseEvent(w.windowHandle()); + QApplication::processEvents(); + QCOMPARE(w.spontClose, 1); + QCOMPARE(child.spontHide, 0); + } +} + class PaintTestWidget : public QWidget { public: