From bedc58869844cf3199f95d7960d7d74f99095a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 15 Sep 2021 13:19:00 +0200 Subject: Always reset close-status of QWidget when trying to close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the status setting and resetting back into handleClose so that we don't end up with it being set if handleClose is never called in response to a close attempt. This can happen when QWindow's platform window has already been destroyed. Since QWindow::close handles that case gracefully and returns true, we can safely call it multiple times. Add test coverage to verify that we get exactly those close event calls that we want. Change-Id: Ica77bf17c26d923c3b79b1e5a688addbc88a6277 Reviewed-by: Tor Arne Vestbø Reviewed-by: Volker Hilsheimer --- src/widgets/kernel/qwidget.cpp | 12 +++++---- tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp | 30 +++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 7e0d2c7291..dcd3d1f1d9 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -8389,11 +8389,13 @@ void QWidgetPrivate::hideChildren(bool spontaneous) */ bool QWidgetPrivate::handleClose(CloseMode mode) { - Q_Q(QWidget); + if (data.is_closing) + return true; // We might not have initiated the close, so update the state now that we know data.is_closing = true; + Q_Q(QWidget); QPointer that = q; QPointer parentWidget = (q->parentWidget() && !QObjectPrivate::get(q->parentWidget())->wasDeleted) ? q->parentWidget() : nullptr; @@ -8482,10 +8484,10 @@ bool QWidget::close() bool QWidgetPrivate::close() { - if (data.is_closing) - return true; - - data.is_closing = true; + // FIXME: We're not setting is_closing here, even though that would + // make sense, as the code below will not end up in handleClose to + // reset is_closing when there's a QWindow, but no QPlatformWindow, + // and we can't assume close is synchronous so we can't reset it here. // Close native widgets via QWindow::close() in order to run QWindow // close code. The QWidget-specific close code in handleClose() will diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index 0da8d2971b..0fd91ebcfa 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -11808,6 +11808,36 @@ void tst_QWidget::closeEvent() widget.windowHandle()->close(); widget.windowHandle()->close(); QCOMPARE(widget.closeCount, 1); + + CloseCountingWidget widget2; + widget2.show(); + QVERIFY(QTest::qWaitForWindowExposed(&widget2)); + widget2.close(); + widget2.close(); + QCOMPARE(widget2.closeCount, 1); + widget2.closeCount = 0; + + widget2.show(); + QVERIFY(QTest::qWaitForWindowExposed(&widget2)); + widget2.close(); + QCOMPARE(widget2.closeCount, 1); + + CloseCountingWidget widget3; + widget3.close(); + widget3.close(); + QEXPECT_FAIL("", "Closing a widget without a window will unconditionally send close events", Continue); + QCOMPARE(widget3.closeCount, 0); + + QWidget parent; + CloseCountingWidget child; + child.setParent(&parent); + parent.show(); + QVERIFY(QTest::qWaitForWindowExposed(&parent)); + child.close(); + QCOMPARE(child.closeCount, 1); + child.close(); + QEXPECT_FAIL("", "Closing a widget without a window will unconditionally send close events", Continue); + QCOMPARE(child.closeCount, 1); } void tst_QWidget::closeWithChildWindow() -- cgit v1.2.3