6 files changed, 71 insertions, 103 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm b/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm
index ad40c6b0cb..e1f664c9da 100644
--- a/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm
+++ b/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm
@@ -246,7 +246,7 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
     return iface->text(QAccessible::Name).toNSString();
 }
 
-- (BOOL) accessibilityEnabledAttribute {
+- (BOOL) isAccessibilityEnabled {
     QAccessibleInterface *iface = QAccessible::accessibleInterface(axid);
     if (!iface || !iface->isValid())
         return false;
diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index e88cc54143..68af82b5cd 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -290,31 +290,26 @@ void QCocoaMenu::syncSeparatorsCollapsible(bool enable)
     QMacAutoReleasePool pool;
     if (enable) {
         bool previousIsSeparator = true; // setting to true kills all the separators placed at the top.
-        NSMenuItem *previousItem = nil;
+        NSMenuItem *lastVisibleItem = nil;
 
         for (NSMenuItem *item in m_nativeMenu.itemArray) {
             if (item.separatorItem) {
                 // hide item if previous was a separator, or if it's explicitly hidden
-                bool itemVisible = !previousIsSeparator;
-                if (auto *cocoaItem = qt_objc_cast<QCocoaNSMenuItem *>(item).platformMenuItem) {
-                    cocoaItem->setVisible(!previousIsSeparator && cocoaItem->isVisible());
-                    itemVisible = cocoaItem->isVisible();
-                }
-                item.hidden = !itemVisible;
+                bool hideItem = previousIsSeparator;
+                if (auto *cocoaItem = qt_objc_cast<QCocoaNSMenuItem *>(item).platformMenuItem)
+                    hideItem = previousIsSeparator || !cocoaItem->isVisible();
+                item.hidden = hideItem;
             }
 
             if (!item.hidden) {
-                previousItem = item;
-                previousIsSeparator = previousItem.separatorItem;
+                lastVisibleItem = item;
+                previousIsSeparator = lastVisibleItem.separatorItem;
             }
         }
 
         // We now need to check the final item since we don't want any separators at the end of the list.
-        if (previousItem && previousIsSeparator) {
-            if (auto *cocoaItem = qt_objc_cast<QCocoaNSMenuItem *>(previousItem).platformMenuItem)
-                cocoaItem->setVisible(false);
-            previousItem.hidden = YES;
-        }
+        if (lastVisibleItem && lastVisibleItem.separatorItem)
+            lastVisibleItem.hidden = YES;
     } else {
         for (auto *item : qAsConst(m_menuItems)) {
             if (!item->isSeparator())
diff --git a/src/plugins/platforms/cocoa/qcocoascreen.h b/src/plugins/platforms/cocoa/qcocoascreen.h
index dcf6f1c753..93dff027a6 100644
--- a/src/plugins/platforms/cocoa/qcocoascreen.h
+++ b/src/plugins/platforms/cocoa/qcocoascreen.h
@@ -45,6 +45,7 @@
 #include "qcocoacursor.h"
 
 #include <qpa/qplatformintegration.h>
+#include <QtCore/private/qcore_mac_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -96,8 +97,8 @@ private:
     static void updateScreens();
     static void cleanupScreens();
 
-    static bool updateScreensIfNeeded();
-    static NSArray *s_screenConfigurationBeforeUpdate;
+    static QMacNotificationObserver s_screenParameterObserver;
+    static CGDisplayReconfigurationCallBack s_displayReconfigurationCallBack;
 
     static void add(CGDirectDisplayID displayId);
     QCocoaScreen(CGDirectDisplayID displayId);
diff --git a/src/plugins/platforms/cocoa/qcocoascreen.mm b/src/plugins/platforms/cocoa/qcocoascreen.mm
index 6a3172fb19..581ea01fcc 100644
--- a/src/plugins/platforms/cocoa/qcocoascreen.mm
+++ b/src/plugins/platforms/cocoa/qcocoascreen.mm
@@ -72,91 +72,33 @@ namespace CoreGraphics {
     Q_ENUM_NS(DisplayChange)
 }
 
-NSArray *QCocoaScreen::s_screenConfigurationBeforeUpdate = nil;
+QMacNotificationObserver QCocoaScreen::s_screenParameterObserver;
+CGDisplayReconfigurationCallBack QCocoaScreen::s_displayReconfigurationCallBack = nullptr;
 
 void QCocoaScreen::initializeScreens()
 {
     updateScreens();
 
-    CGDisplayRegisterReconfigurationCallback([](CGDirectDisplayID displayId, CGDisplayChangeSummaryFlags flags, void *userInfo) {
+    s_displayReconfigurationCallBack = [](CGDirectDisplayID displayId, CGDisplayChangeSummaryFlags flags, void *userInfo) {
         Q_UNUSED(userInfo);
 
-        // Displays are reconfigured in batches, and we want to update our screens
-        // once a batch ends, so that all the states of the displays are up to date.
-        static int displayReconfigurationsInProgress = 0;
-
         const bool beforeReconfigure = flags & kCGDisplayBeginConfigurationFlag;
-        qCDebug(lcQpaScreen).verbosity(0).nospace() << "Display " << displayId
-                << (beforeReconfigure ? " about to reconfigure" : " was ")
-                << QFlags<CoreGraphics::DisplayChange>(flags)
-                << " with " << displayReconfigurationsInProgress
-                << " display configuration(s) in progress";
-
-        if (!flags) {
-            // CGDisplayRegisterReconfigurationCallback has been observed to be called
-            // with flags unset. This seems like a bug. The callback is not paired with
-            // a matching "completion" callback either, so we don't know whether to treat
-            // it as a begin or end of reconfigure.
-            return;
-        }
-
-        if (beforeReconfigure) {
-            if (!displayReconfigurationsInProgress++) {
-                // There might have been a screen reconfigure before this that
-                // we didn't process yet, so do that now if that's the case.
-                updateScreensIfNeeded();
-
-                Q_ASSERT(!s_screenConfigurationBeforeUpdate);
-                s_screenConfigurationBeforeUpdate = NSScreen.screens;
-                qCDebug(lcQpaScreen, "Display reconfigure transaction started"
-                    " with screen configuration %p", s_screenConfigurationBeforeUpdate);
-
-                static void (^tryScreenUpdate)();
-                tryScreenUpdate = ^void () {
-                    qCDebug(lcQpaScreen) << "Attempting screen update from runloop block";
-                    if (!updateScreensIfNeeded())
-                        CFRunLoopPerformBlock(CFRunLoopGetMain(), kCFRunLoopCommonModes, tryScreenUpdate);
-                };
-                CFRunLoopPerformBlock(CFRunLoopGetMain(), kCFRunLoopCommonModes, tryScreenUpdate);
-            }
-        } else {
-            Q_ASSERT_X(displayReconfigurationsInProgress, "QCococaScreen",
-                "Display configuration transactions are expected to be balanced");
+        qCDebug(lcQpaScreen).verbosity(0) << "Display" << displayId
+                << (beforeReconfigure ? "beginning" : "finished") << "reconfigure"
+                << QFlags<CoreGraphics::DisplayChange>(flags);
 
-            if (!--displayReconfigurationsInProgress) {
-                qCDebug(lcQpaScreen) << "Display reconfigure transaction completed";
-                // We optimistically update now, in case the NSScreens have changed
-                updateScreensIfNeeded();
-            }
-        }
-    }, nullptr);
+        if (!beforeReconfigure)
+            updateScreens();
+    };
+    CGDisplayRegisterReconfigurationCallback(s_displayReconfigurationCallBack, nullptr);
 
-    static QMacNotificationObserver screenParameterObserver(NSApplication.sharedApplication,
+    s_screenParameterObserver = QMacNotificationObserver(NSApplication.sharedApplication,
         NSApplicationDidChangeScreenParametersNotification, [&]() {
             qCDebug(lcQpaScreen) << "Received screen parameter change notification";
-            updateScreensIfNeeded(); // As a last resort we update screens here
+            updateScreens();
         });
 }
 
-bool QCocoaScreen::updateScreensIfNeeded()
-{
-    if (!s_screenConfigurationBeforeUpdate) {
-        qCDebug(lcQpaScreen) << "QScreens have already been updated, all good";
-        return true;
-    }
-
-    if (s_screenConfigurationBeforeUpdate == NSScreen.screens) {
-        qCDebug(lcQpaScreen) << "Still waiting for NSScreen configuration change";
-        return false;
-    }
-
-    qCDebug(lcQpaScreen, "NSScreen configuration changed to %p", NSScreen.screens);
-    updateScreens();
-
-    s_screenConfigurationBeforeUpdate = nil;
-    return true;
-}
-
 /*
     Update the list of available QScreens, and the properties of existing screens.
 
@@ -239,6 +181,12 @@ void QCocoaScreen::cleanupScreens()
     // Remove screens in reverse order to avoid crash in case of multiple screens
     for (QScreen *screen : backwards(QGuiApplication::screens()))
         static_cast<QCocoaScreen*>(screen->handle())->remove();
+
+    Q_ASSERT(s_displayReconfigurationCallBack);
+    CGDisplayRemoveReconfigurationCallback(s_displayReconfigurationCallBack, nullptr);
+    s_displayReconfigurationCallBack = nullptr;
+
+    s_screenParameterObserver.remove();
 }
 
 void QCocoaScreen::remove()
@@ -310,15 +258,18 @@ void QCocoaScreen::update(CGDirectDisplayID displayId)
 
     Q_ASSERT(isOnline());
 
+    // Some properties are only available via NSScreen
+    NSScreen *nsScreen = nativeScreen();
+    if (!nsScreen) {
+        qCDebug(lcQpaScreen) << "Corresponding NSScreen not yet available. Deferring update";
+        return;
+    }
+
     const QRect previousGeometry = m_geometry;
     const QRect previousAvailableGeometry = m_availableGeometry;
     const QDpi previousLogicalDpi = m_logicalDpi;
     const qreal previousRefreshRate = m_refreshRate;
 
-    // Some properties are only available via NSScreen
-    NSScreen *nsScreen = nativeScreen();
-    Q_ASSERT(nsScreen);
-
     // The reference screen for the geometry is always the primary screen
     QRectF primaryScreenGeometry = QRectF::fromCGRect(CGDisplayBounds(CGMainDisplayID()));
     m_geometry = qt_mac_flip(QRectF::fromCGRect(nsScreen.frame), primaryScreenGeometry).toRect();
@@ -714,13 +665,17 @@ QList<QPlatformScreen*> QCocoaScreen::virtualSiblings() const
 
 QCocoaScreen *QCocoaScreen::get(NSScreen *nsScreen)
 {
-    if (s_screenConfigurationBeforeUpdate) {
-        qCWarning(lcQpaScreen) << "Trying to resolve screen while waiting for screen reconfigure!";
-        if (!updateScreensIfNeeded())
-            qCWarning(lcQpaScreen) << "Failed to do last minute screen update. Expect crashes.";
+    auto displayId = nsScreen.qt_displayId;
+    auto *cocoaScreen = get(displayId);
+    if (!cocoaScreen) {
+        qCWarning(lcQpaScreen) << "Failed to map" << nsScreen
+            << "to QCocoaScreen. Doing last minute update.";
+        updateScreens();
+        cocoaScreen = get(displayId);
+        if (!cocoaScreen)
+            qCWarning(lcQpaScreen) << "Last minute update failed!";
     }
-
-    return get(nsScreen.qt_displayId);
+    return cocoaScreen;
 }
 
 QCocoaScreen *QCocoaScreen::get(CGDirectDisplayID displayId)
diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm
index 5be65f2141..ad3ebf61b4 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.mm
+++ b/src/plugins/platforms/cocoa/qcocoawindow.mm
@@ -522,7 +522,12 @@ NSUInteger QCocoaWindow::windowStyleMask(Qt::WindowFlags flags)
     NSUInteger styleMask = (frameless || !resizable) ? NSWindowStyleMaskBorderless : NSWindowStyleMaskResizable;
 
     if (frameless) {
-        // No further customizations for frameless since there are no window decorations.
+        // Frameless windows do not display the traffic lights buttons for
+        // e.g. minimize, however StyleMaskMiniaturizable is required to allow
+        // programatic minimize. However, for framless tool windows (e.g. dock windows)
+        // we don't want that, as it breaks translucency.
+        if (type != Qt::Tool)
+            styleMask |= NSWindowStyleMaskMiniaturizable;
     } else if (flags & Qt::CustomizeWindowHint) {
         if (flags & Qt::WindowTitleHint)
             styleMask |= NSWindowStyleMaskTitled;
@@ -1265,11 +1270,15 @@ void QCocoaWindow::windowDidChangeScreen()
         return;
 
     // Note: When a window is resized to 0x0 Cocoa will report the window's screen as nil
-    auto *currentScreen = QCocoaScreen::get(m_view.window.screen);
+    NSScreen *nsScreen = m_view.window.screen;
+
+    qCDebug(lcQpaWindow) << window() << "did change" << nsScreen;
+    QCocoaScreen::updateScreens();
+
     auto *previousScreen = static_cast<QCocoaScreen*>(screen());
+    auto *currentScreen = QCocoaScreen::get(nsScreen);
 
-    Q_ASSERT_X(!m_view.window.screen || currentScreen,
-        "QCocoaWindow", "Failed to get QCocoaScreen for NSScreen");
+    qCDebug(lcQpaWindow) << "Screen changed for" << window() << "from" << previousScreen << "to" << currentScreen;
 
     // Note: The previous screen may be the same as the current screen, either because
     // a) the screen was just reconfigured, which still results in AppKit sending an
@@ -1282,7 +1291,6 @@ void QCocoaWindow::windowDidChangeScreen()
     // device-pixel ratio may have changed, and needs to be delivered to all
     // windows, both top level and child windows.
 
-    qCDebug(lcQpaWindow) << "Screen changed for" << window() << "from" << previousScreen << "to" << currentScreen;
     QWindowSystemInterface::handleWindowScreenChanged<QWindowSystemInterface::SynchronousDelivery>(
         window(), currentScreen ? currentScreen->screen() : nullptr);
 
@@ -1307,10 +1315,19 @@ void QCocoaWindow::windowWillClose()
 bool QCocoaWindow::windowShouldClose()
 {
     qCDebug(lcQpaWindow) << "QCocoaWindow::windowShouldClose" << window();
+
     // This callback should technically only determine if the window
     // should (be allowed to) close, but since our QPA API to determine
     // that also involves actually closing the window we do both at the
     // same time, instead of doing the latter in windowWillClose.
+
+    // If the window is closed, we will release and deallocate the NSWindow.
+    // But frames higher up in the stack might still expect the window to
+    // be alive, since the windowShouldClose: callback is technically only
+    // supposed to answer YES or NO. To ensure the window is still alive
+    // we put an autorelease in the closest pool (typically the runloop).
+    [[m_view.window retain] autorelease];
+
     return QWindowSystemInterface::handleCloseEvent<QWindowSystemInterface::SynchronousDelivery>(window());
 }
 
diff --git a/src/plugins/platforms/cocoa/qnswindow.mm b/src/plugins/platforms/cocoa/qnswindow.mm
index 8967636fd2..1ce8ab5f53 100644
--- a/src/plugins/platforms/cocoa/qnswindow.mm
+++ b/src/plugins/platforms/cocoa/qnswindow.mm
@@ -104,7 +104,7 @@ static bool isMouseEvent(NSEvent *ev)
 
     // Unfortunately there's no NSWindowListOrderedBackToFront,
     // so we have to manually reverse the order using an array.
-    NSMutableArray<NSWindow *> *windows = [NSMutableArray<NSWindow *> new];
+    NSMutableArray<NSWindow *> *windows = [[NSMutableArray<NSWindow *> new] autorelease];
     [application enumerateWindowsWithOptions:NSWindowListOrderedFrontToBack
         usingBlock:^(NSWindow *window, BOOL *) {
             // For some reason AppKit will give us nil-windows, skip those