Centralize maybeLastWindowClosed checking in QWindow

Instead of plumbing QWidgetWindow close events via handleCloseEvent,
we just implement closeEvent directly. This allows QWindow do save
the state of the window/widget before the close event, so that we
know whether we should trigger lastWindowClosed handling, even if
the window was deleted as a result of the close event.

This also relieves QGuiApplication and QApplication from dealing
with the close logic in their notify functions, so that these
functions can focus on the propagation of events -- not how the
event is handled.

Change-Id: I8b586b53a53b1df1d8630c1acb635c60f191bb4b
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

src/gui/kernel/qguiapplication.cpp

@@ -1920,8 +1920,6 @@ void QGuiApplicationPrivate::captureGlobalModifierState(QEvent *e)
 */
 bool QGuiApplication::notify(QObject *object, QEvent *event)
 {
-    Q_D(QGuiApplication);
-
     if (object->isWindowType()) {
         if (QGuiApplicationPrivate::sendQWindowEventToQPlatformWindow(static_cast<QWindow *>(object), event))
             return true; // Platform plugin ate the event
@@ -1929,12 +1927,7 @@ bool QGuiApplication::notify(QObject *object, QEvent *event)
 
     QGuiApplicationPrivate::captureGlobalModifierState(event);
 
-    bool accepted = QCoreApplication::notify(object, event);
-
-    if (event->type() == QEvent::Close && object->isWindowType() && accepted)
-        d->maybeLastWindowClosed(static_cast<QWindow*>(object));
-
-    return accepted;
+    return QCoreApplication::notify(object, event);
 }
 
 /*! \reimp
@@ -3548,15 +3541,8 @@ bool QGuiApplication::quitOnLastWindowClosed()
     return QGuiApplicationPrivate::quitOnLastWindowClosed;
 }
 
-void QGuiApplicationPrivate::maybeLastWindowClosed(QWindow *closedWindow)
+void QGuiApplicationPrivate::maybeLastWindowClosed()
 {
-    Q_ASSERT(closedWindow);
-
-    // Only windows that themselves participates in last-window-closed
-    // should be allowed to trigger lastWindowClosed.
-    if (!qt_window_private(closedWindow)->participatesInLastWindowClosed())
-        return;
-
     if (!lastWindowClosed())
         return;
 
@@ -3583,10 +3569,11 @@ void QGuiApplicationPrivate::maybeLastWindowClosed(QWindow *closedWindow)
 bool QGuiApplicationPrivate::lastWindowClosed() const
 {
     for (auto *window : QGuiApplication::topLevelWindows()) {
-        if (!qt_window_private(window)->participatesInLastWindowClosed())
+        auto *windowPrivate = qt_window_private(window);
+        if (!windowPrivate->participatesInLastWindowClosed())
             continue;
 
-        if (window->isVisible())
+        if (windowPrivate->treatAsVisible())
             return false;
      }
 

src/gui/kernel/qguiapplication_p.h

@@ -108,7 +108,7 @@ public:
     bool canQuitAutomatically() override;
     void quit() override;
 
-    void maybeLastWindowClosed(QWindow *closedWindow);
+    void maybeLastWindowClosed();
     bool lastWindowClosed() const;
     static bool quitOnLastWindowClosed;
 

src/gui/kernel/qwindow.cpp

@@ -2292,6 +2292,12 @@ bool QWindowPrivate::participatesInLastWindowClosed() const
     return true;
 }
 
+bool QWindowPrivate::treatAsVisible() const
+{
+    Q_Q(const QWindow);
+    return q->isVisible();
+}
+
 /*!
     The expose event (\a ev) is sent by the window system when a window moves
     between the un-exposed and exposed states.
@@ -2466,11 +2472,25 @@ bool QWindow::event(QEvent *ev)
         break;
 #endif
 
-    case QEvent::Close:
+    case QEvent::Close: {
+
+        Q_D(QWindow);
+        const bool wasVisible = d->treatAsVisible();
+        const bool participatesInLastWindowClosed = d->participatesInLastWindowClosed();
+
+        // The window might be deleted in the close event handler
+        QPointer<QWindow> deletionGuard(this);
         closeEvent(static_cast<QCloseEvent*>(ev));
-        if (ev->isAccepted())
-            destroy();
+
+        if (ev->isAccepted()) {
+            if (deletionGuard)
+                destroy();
+            if (wasVisible && participatesInLastWindowClosed)
+                QGuiApplicationPrivate::instance()->maybeLastWindowClosed();
+        }
+
         break;
+    }
 
     case QEvent::Expose:
         exposeEvent(static_cast<QExposeEvent *>(ev));

src/gui/kernel/qwindow_p.h

@@ -119,6 +119,7 @@ public:
     virtual void processSafeAreaMarginsChanged() {}
 
     virtual bool participatesInLastWindowClosed() const;
+    virtual bool treatAsVisible() const;
 
     bool isPopup() const { return (windowFlags & Qt::WindowType_Mask) == Qt::Popup; }
     void setAutomaticPositionAndResizeEnabled(bool a)

src/widgets/kernel/qapplication.cpp

@@ -3279,11 +3279,6 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
             res = d->notify_helper(receiver, e);
             break;
         }
-    } else if (receiver->isWindowType()) {
-        res = d->notify_helper(receiver, e);
-        // We don't call QGuiApplication::notify here, so we need to duplicate the logic
-        if (res && e->type() == QEvent::Close)
-            d->maybeLastWindowClosed(static_cast<QWindow *>(receiver));
     } else {
         res = d->notify_helper(receiver, e);
     }

src/widgets/kernel/qwidgetwindow.cpp

@@ -120,6 +120,7 @@ public:
     }
 
     bool participatesInLastWindowClosed() const override;
+    bool treatAsVisible() const override;
 };
 
 QRectF QWidgetWindowPrivate::closestAcceptableGeometry(const QRectF &rect) const
@@ -230,6 +231,7 @@ static inline bool shouldBePropagatedToWidget(QEvent *event)
     case QEvent::ChildAdded:
     case QEvent::ChildRemoved:
     case QEvent::Paint:
+    case QEvent::Close: // Propagated manually in closeEvent
         return false;
     default:
         return true;
@@ -242,15 +244,6 @@ bool QWidgetWindow::event(QEvent *event)
         return QWindow::event(event);
 
     switch (event->type()) {
-    case QEvent::Close: {
-        // The widget might be deleted in the close event handler.
-        QPointer<QObject> guard = this;
-        handleCloseEvent(static_cast<QCloseEvent *>(event));
-        if (guard)
-            QWindow::event(event);
-        return true;
-    }
-
     case QEvent::Enter:
     case QEvent::Leave:
         handleEnterLeaveEvent(event);
@@ -838,7 +831,7 @@ void QWidgetWindow::handleResizeEvent(QResizeEvent *event)
     }
 }
 
-void QWidgetWindow::handleCloseEvent(QCloseEvent *event)
+void QWidgetWindow::closeEvent(QCloseEvent *event)
 {
     Q_D(QWidgetWindow);
     bool accepted = m_widget->d_func()->handleClose(d->inClose ? QWidgetPrivate::CloseWithEvent
@@ -860,6 +853,18 @@ bool QWidgetWindowPrivate::participatesInLastWindowClosed() const
     return QWindowPrivate::participatesInLastWindowClosed();
 }
 
+bool QWidgetWindowPrivate::treatAsVisible() const
+{
+    Q_Q(const QWidgetWindow);
+
+    // Widget windows may have Qt::WA_DontShowOnScreen, in which case the
+    // QQWidget will be visible, but the corresponding QWindow will not.
+    // Since the lastWindowClosed logic relies on checking whether the
+    // closed window was visible, and if there are any remaining visible
+    // windows, we need to reflect the QWidget state, not the QWindow one.
+    return q->widget()->isVisible();
+}
+
 #if QT_CONFIG(wheelevent)
 
 void QWidgetWindow::handleWheelEvent(QWheelEvent *event)

src/widgets/kernel/qwidgetwindow_p.h

@@ -83,7 +83,8 @@ public:
 protected:
     bool event(QEvent *) override;
 
-    void handleCloseEvent(QCloseEvent *);
+    void closeEvent(QCloseEvent *) override;
+
     void handleEnterLeaveEvent(QEvent *);
     void handleFocusInEvent(QFocusEvent *);
     void handleKeyEvent(QKeyEvent *);

tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp

@@ -12253,10 +12253,19 @@ protected:
 
 void tst_QWidget::deleteWindowInCloseEvent()
 {
-    // Just checking if closing this widget causes a crash
+    QSignalSpy quitSpy(qApp, &QGuiApplication::lastWindowClosed);
+
+    // Closing this widget should not cause a crash
     auto widget = new DeleteOnCloseEventWidget;
-    widget->close();
-    QVERIFY(true);
+    widget->show();
+    QVERIFY(QTest::qWaitForWindowExposed(widget));
+    QTimer::singleShot(0, widget, [&]{
+        widget->close();
+    });
+    QApplication::exec();
+
+    // It should still result in a single lastWindowClosed emit
+    QCOMPARE(quitSpy.count(), 1);
 }
 
 /*!