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.5
Change-Id: I07418c12b421fb0b4ebf875050e32f56fdad6197
Reviewed-by: Doris Verria <doris.verria@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 78cb91c5caa47d49a4105c1113edaf3a87827b98)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Tor Arne Vestbø 2023-10-22 14:36:53 +02:00 committed by Qt Cherry-pick Bot
parent fa74a99c8e
commit 44902f09cf
2 changed files with 23 additions and 1 deletions

View File

@ -59,6 +59,8 @@
#include <CoreFoundation/CoreFoundation.h>
Q_FORWARD_DECLARE_OBJC_CLASS(NSWindow);
QT_BEGIN_NAMESPACE
Q_DECLARE_LOGGING_CATEGORY(lcEventDispatcher);
@ -67,7 +69,7 @@ typedef struct _NSModalSession *NSModalSession;
typedef struct _QCocoaModalSessionInfo {
QPointer<QWindow> window;
NSModalSession session;
void *nswindow;
NSWindow *nswindow;
} QCocoaModalSessionInfo;
class QCocoaEventDispatcherPrivate;

View File

@ -374,6 +374,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) {
@ -417,6 +418,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
@ -617,6 +619,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];
@ -656,6 +660,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.
@ -704,6 +710,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];
}
@ -716,6 +724,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().
@ -736,6 +752,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
@ -966,6 +984,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];
}