macOS: Avoid adding modal session for window that's already added

Investigating QCocoaWindow::setVisible idempotence revealed that our
QCocoaEventDispatcherPrivate::beginModalSession() implementation would
happily add the same window multiple times, resulting in trying to
re-start a modal session for the same window when the window was
closed, confusing AppKit's logic for choosing the next key window
when a modal window is closed.

We now bail out if we detect this scenario, even if setVisible has
been fixed. Additional logging has also been added.

Pick-to: 6.6 6.5
Change-Id: I07418c12b421fb0b4ebf875050e32f56fdad6197
Reviewed-by: Doris Verria <doris.verria@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 23 insertions, 1 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.h b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.h
index 558071e5e1..c942aa9304 100644
--- a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.h
+++ b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.h
@@ -60,6 +60,8 @@
 
 #include <CoreFoundation/CoreFoundation.h>
 
+Q_FORWARD_DECLARE_OBJC_CLASS(NSWindow);
+
 QT_BEGIN_NAMESPACE
 
 Q_DECLARE_LOGGING_CATEGORY(lcEventDispatcher);
@@ -68,7 +70,7 @@ typedef struct _NSModalSession *NSModalSession;
 typedef struct _QCocoaModalSessionInfo {
     QPointer<QWindow> window;
     NSModalSession session;
-    void *nswindow;
+    NSWindow *nswindow;
 } QCocoaModalSessionInfo;
 
 class QCocoaEventDispatcherPrivate;
diff --git a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
index a594259d99..77d636793b 100644
--- a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
+++ b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
@@ -375,6 +375,7 @@ bool QCocoaEventDispatcher::processEvents(QEventLoop::ProcessEventsFlags flags)
             // [NSApp run], which is the normal code path for cocoa applications.
             if (NSModalSession session = d->currentModalSession()) {
                 QBoolBlocker execGuard(d->currentExecIsNSAppRun, false);
+                qCDebug(lcEventDispatcher) << "Running modal session" << session;
                 while ([NSApp runModalSession:session] == NSModalResponseContinue && !d->interrupt) {
                     qt_mac_waitForMoreEvents(NSModalPanelRunLoopMode);
                     if (session != d->currentModalSessionCached) {
@@ -418,6 +419,7 @@ bool QCocoaEventDispatcher::processEvents(QEventLoop::ProcessEventsFlags flags)
                     // to use cocoa's native way of running modal sessions:
                     if (flags & QEventLoop::WaitForMoreEvents)
                         qt_mac_waitForMoreEvents(NSModalPanelRunLoopMode);
+                    qCDebug(lcEventDispatcher) << "Running modal session" << session;
                     NSInteger status = [NSApp runModalSession:session];
                     if (status != NSModalResponseContinue && session == d->currentModalSessionCached) {
                         // INVARIANT: Someone called [NSApp stopModal:] from outside the event
@@ -618,6 +620,8 @@ void QCocoaEventDispatcherPrivate::temporarilyStopAllModalSessions()
     for (int i=0; i<stackSize; ++i) {
         QCocoaModalSessionInfo &info = cocoaModalSessionStack[i];
         if (info.session) {
+            qCDebug(lcEventDispatcher) << "Temporarily ending modal session" << info.session
+                                       << "for" << info.nswindow;
             [NSApp endModalSession:info.session];
             info.session = nullptr;
             [(NSWindow*) info.nswindow release];
@@ -657,6 +661,8 @@ NSModalSession QCocoaEventDispatcherPrivate::currentModalSession()
             [(NSWindow*) info.nswindow retain];
             QRect rect = cocoaWindow->geometry();
             info.session = [NSApp beginModalSessionForWindow:nswindow];
+            qCDebug(lcEventDispatcher) << "Begun modal session" << info.session
+                                       << "for" << nswindow;
 
             // The call to beginModalSessionForWindow above processes events and may
             // have deleted or destroyed the window. Check if it's still valid.
@@ -705,6 +711,8 @@ void QCocoaEventDispatcherPrivate::cleanupModalSessions()
         currentModalSessionCached = nullptr;
         if (info.session) {
             Q_ASSERT(info.nswindow);
+            qCDebug(lcEventDispatcher) << "Ending modal session" << info.session
+                                       << "for" << info.nswindow;
             [NSApp endModalSession:info.session];
             [(NSWindow *)info.nswindow release];
         }
@@ -717,6 +725,14 @@ void QCocoaEventDispatcherPrivate::cleanupModalSessions()
 
 void QCocoaEventDispatcherPrivate::beginModalSession(QWindow *window)
 {
+    qCDebug(lcEventDispatcher) << "Adding modal session for" << window;
+
+    if (std::any_of(cocoaModalSessionStack.constBegin(), cocoaModalSessionStack.constEnd(),
+        [&](const auto &sessionInfo) { return sessionInfo.window == window; })) {
+        qCWarning(lcEventDispatcher) << "Modal session for" << window << "already exists!";
+        return;
+    }
+
     // We need to start spinning the modal session. Usually this is done with
     // QDialog::exec() for Qt Widgets based applications, but for others that
     // just call show(), we need to interrupt().
@@ -737,6 +753,8 @@ void QCocoaEventDispatcherPrivate::beginModalSession(QWindow *window)
 
 void QCocoaEventDispatcherPrivate::endModalSession(QWindow *window)
 {
+    qCDebug(lcEventDispatcher) << "Removing modal session for" << window;
+
     Q_Q(QCocoaEventDispatcher);
 
     // Mark all sessions attached to window as pending to be stopped. We do this
@@ -967,6 +985,8 @@ QCocoaEventDispatcher::~QCocoaEventDispatcher()
     for (int i = 0; i < d->cocoaModalSessionStack.count(); ++i) {
         QCocoaModalSessionInfo &info = d->cocoaModalSessionStack[i];
         if (info.session) {
+            qCDebug(lcEventDispatcher) << "Ending modal session" << info.session
+                                       << "for" << info.nswindow << "during shutdown";
             [NSApp endModalSession:info.session];
             [(NSWindow *)info.nswindow release];
         }