Close widgets properly from session management.

Introduce new virtual QGuiApplicationPrivate::tryCloseAllWindows()
which allows overriding the behavior in QApplication to properly close
the widgets first.

Without this, QGuiApplication closes the widget windows leaving a stale
window handle behind in the associated QWidget which then causes the
application not to terminate since QApplication::shouldQuit() stills
finds the affected widgets to be visible.

Task-number: QTBUG-35986

Change-Id: I19ac4b5a19250ee68d09e461c03dbace458c98e4
Reviewed-by: David Faure <david.faure@kdab.com>

4 files changed, 64 insertions, 35 deletions

diff --git a/src/gui/kernel/qguiapplication.cpp b/src/gui/kernel/qguiapplication.cpp
index 0f3f8d8ff2..3953437372 100644
--- a/src/gui/kernel/qguiapplication.cpp
+++ b/src/gui/kernel/qguiapplication.cpp
@@ -2696,6 +2696,27 @@ bool QGuiApplicationPrivate::shouldQuitInternal(const QWindowList &processedWind
     return true;
 }
 
+bool QGuiApplicationPrivate::tryCloseAllWindows()
+{
+    return tryCloseRemainingWindows(QWindowList());
+}
+
+bool QGuiApplicationPrivate::tryCloseRemainingWindows(QWindowList processedWindows)
+{
+    QWindowList list = QGuiApplication::topLevelWindows();
+    for (int i = 0; i < list.size(); ++i) {
+        QWindow *w = list.at(i);
+        if (w->isVisible() && !processedWindows.contains(w)) {
+            if (!w->close())
+                return false;
+            processedWindows.append(w);
+            list = QGuiApplication::topLevelWindows();
+            i = -1;
+        }
+    }
+    return true;
+}
+
 /*!
     \since 5.2
     \fn Qt::ApplicationState QGuiApplication::applicationState()
@@ -2902,23 +2923,8 @@ void QGuiApplicationPrivate::commitData()
     Q_Q(QGuiApplication);
     is_saving_session = true;
     emit q->commitDataRequest(*session_manager);
-    if (session_manager->allowsInteraction()) {
-        QWindowList done;
-        QWindowList list = QGuiApplication::topLevelWindows();
-        bool cancelled = false;
-        for (int i = 0; !cancelled && i < list.size(); ++i) {
-            QWindow* w = list.at(i);
-            if (w->isVisible() && !done.contains(w)) {
-                cancelled = !w->close();
-                if (!cancelled)
-                    done.append(w);
-                list = QGuiApplication::topLevelWindows();
-                i = -1;
-            }
-        }
-        if (cancelled)
-            session_manager->cancel();
-    }
+    if (session_manager->allowsInteraction() && !tryCloseAllWindows())
+        session_manager->cancel();
     is_saving_session = false;
 }
 
diff --git a/src/gui/kernel/qguiapplication_p.h b/src/gui/kernel/qguiapplication_p.h
index c23232182c..1ec808ec27 100644
--- a/src/gui/kernel/qguiapplication_p.h
+++ b/src/gui/kernel/qguiapplication_p.h
@@ -92,6 +92,7 @@ public:
     virtual bool shouldQuit();
 
     bool shouldQuitInternal(const QWindowList &processedWindows);
+    virtual bool tryCloseAllWindows();
 
     static Qt::KeyboardModifiers modifier_buttons;
     static Qt::MouseButtons mouse_buttons;
@@ -291,6 +292,7 @@ public:
 
 protected:
     virtual void notifyThemeChanged();
+    bool tryCloseRemainingWindows(QWindowList processedWindows);
 #ifndef QT_NO_DRAGANDDROP
     virtual void notifyDragStarted(const QDrag *);
 #endif // QT_NO_DRAGANDDROP
diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp
index 513d98867c..3309b4be06 100644
--- a/src/widgets/kernel/qapplication.cpp
+++ b/src/widgets/kernel/qapplication.cpp
@@ -1733,6 +1733,41 @@ QFontMetrics QApplication::fontMetrics()
     return desktop()->fontMetrics();
 }
 
+bool QApplicationPrivate::tryCloseAllWidgetWindows(QWindowList *processedWindows)
+{
+    Q_ASSERT(processedWindows);
+    while (QWidget *w = QApplication::activeModalWidget()) {
+        if (!w->isVisible() || w->data->is_closing)
+            break;
+        QWindow *window = w->windowHandle();
+        if (!w->close()) // Qt::WA_DeleteOnClose may cause deletion.
+            return false;
+        if (window)
+            processedWindows->append(window);
+    }
+
+    QWidgetList list = QApplication::topLevelWidgets();
+    for (int i = 0; i < list.size(); ++i) {
+        QWidget *w = list.at(i);
+        if (w->isVisible() && w->windowType() != Qt::Desktop && !w->data->is_closing) {
+            QWindow *window = w->windowHandle();
+            if (!w->close())  // Qt::WA_DeleteOnClose may cause deletion.
+                return false;
+            if (window)
+                processedWindows->append(window);
+            list = QApplication::topLevelWidgets();
+            i = -1;
+        }
+    }
+    return true;
+}
+
+bool QApplicationPrivate::tryCloseAllWindows()
+{
+    QWindowList processedWindows;
+    return QApplicationPrivate::tryCloseAllWidgetWindows(&processedWindows)
+        && QGuiApplicationPrivate::tryCloseRemainingWindows(processedWindows);
+}
 
 /*!
     Closes all top-level windows.
@@ -1754,24 +1789,8 @@ QFontMetrics QApplication::fontMetrics()
 */
 void QApplication::closeAllWindows()
 {
-    bool did_close = true;
-    QWidget *w;
-    while ((w = activeModalWidget()) && did_close) {
-        if (!w->isVisible() || w->data->is_closing)
-            break;
-        did_close = w->close();
-    }
-    QWidgetList list = QApplication::topLevelWidgets();
-    for (int i = 0; did_close && i < list.size(); ++i) {
-        w = list.at(i);
-        if (w->isVisible()
-            && w->windowType() != Qt::Desktop
-            && !w->data->is_closing) {
-            did_close = w->close();
-            list = QApplication::topLevelWidgets();
-            i = -1;
-        }
-    }
+    QWindowList processedWindows;
+    QApplicationPrivate::tryCloseAllWidgetWindows(&processedWindows);
 }
 
 /*!
diff --git a/src/widgets/kernel/qapplication_p.h b/src/widgets/kernel/qapplication_p.h
index 68ddc13362..93c9ffe002 100644
--- a/src/widgets/kernel/qapplication_p.h
+++ b/src/widgets/kernel/qapplication_p.h
@@ -120,6 +120,7 @@ public:
     virtual void notifyActiveWindowChange(QWindow *);
 
     virtual bool shouldQuit();
+    bool tryCloseAllWindows() Q_DECL_OVERRIDE;
 
 #if defined(Q_WS_X11)
 #ifndef QT_NO_SETTINGS
@@ -294,6 +295,7 @@ public:
     QPixmap applyQIconStyleHelper(QIcon::Mode mode, const QPixmap& base) const;
 private:
     static QApplicationPrivate *self;
+    static bool tryCloseAllWidgetWindows(QWindowList *processedWindows);
 
     static void giveFocusAccordingToFocusPolicy(QWidget *w, QEvent *event, QPoint localPos);
     static bool shouldSetFocus(QWidget *w, Qt::FocusPolicy policy);