macOS: Correctly record normalGeometry in Cocoa plugin

Cocoa sends QWidget the state-change notification after the window has
been resized already, at which point we cannot store the normal geometry
anymore.

Handle zoom and full screen callbacks prior to the state changing
to store the geometry in QCocoaWindow. We do not need to handle
minimized state, as the window will still reflect the original
geometry.

Return the stored value from an override of
QPlatformWindow::normalGeometry so that QWidget gets the correct values
even though the new state is already active.

Fix the tst_QWidget::normalGeometry test to make it pass on all
platforms by waiting for the window to actually have transitioned to
the new state before comparing geometries. Both macOS and Windows fully
pass; on Xcb, deminimizing a window using setWindowState does not work,
which is why the test was partially skipped (confirmed by visual
testing). Move those problematic, complex test cases to the end so
that most cases are covered on Xcb as well.

Done-with: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Pick-to: 6.2
Change-Id: I518a5db9169b80e8fa25fe4fa2b50bd1ea0e6db3
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

src/plugins/platforms/cocoa/qcocoawindow.h

@@ -113,6 +113,7 @@ public:
 
     void setGeometry(const QRect &rect) override;
     QRect geometry() const override;
+    QRect normalGeometry() const override;
     void setCocoaGeometry(const QRect &rect);
 
     void setVisible(bool visible) override;
@@ -171,6 +172,8 @@ public:
     Q_NOTIFICATION_HANDLER(NSWindowDidChangeOcclusionStateNotification) void windowDidChangeOcclusionState();
     Q_NOTIFICATION_HANDLER(NSWindowDidChangeScreenNotification) void windowDidChangeScreen();
 
+    void windowWillZoom();
+
     bool windowShouldClose();
     bool windowIsPopupType(Qt::WindowType type = Qt::Widget) const;
 
@@ -205,6 +208,8 @@ public:
 
     QPoint bottomLeftClippedByNSWindowOffset() const override;
 
+    void updateNormalGeometry();
+
     enum RecreationReason {
         RecreationNotNeeded = 0,
         ParentChanged = 0x1,
@@ -262,6 +267,7 @@ public: // for QNSView
 
     bool m_frameStrutEventsEnabled;
     QRect m_exposedRect;
+    QRect m_normalGeometry;
     int m_registerTouchCount;
     bool m_resizableTransientParent;
 

src/plugins/platforms/cocoa/qcocoawindow.mm

@@ -261,6 +261,40 @@ QRect QCocoaWindow::geometry() const
     return QPlatformWindow::geometry();
 }
 
+/*!
+    \brief the geometry of the window as it will appear when shown as
+    a normal (not maximized or full screen) top-level window.
+
+    For child windows this property always holds an empty rectangle.
+
+    \sa QWidget::normalGeometry()
+*/
+QRect QCocoaWindow::normalGeometry() const
+{
+    if (!isContentView())
+        return QRect();
+
+    // We only persist the normal the geometry when going into
+    // fullscreen and maximized states. For all other cases we
+    // can just report the geometry as is.
+
+    if (!(windowState() & (Qt::WindowFullScreen | Qt::WindowMaximized)))
+        return geometry();
+
+    return m_normalGeometry;
+}
+
+void QCocoaWindow::updateNormalGeometry()
+{
+    if (!isContentView())
+        return;
+
+    if (windowState() != Qt::WindowNoState)
+        return;
+
+    m_normalGeometry = geometry();
+}
+
 void QCocoaWindow::setCocoaGeometry(const QRect &rect)
 {
     qCDebug(lcQpaWindow) << "QCocoaWindow::setCocoaGeometry" << window() << rect;
@@ -754,6 +788,11 @@ void QCocoaWindow::toggleMaximized()
         window.styleMask &= ~NSWindowStyleMaskResizable;
 }
 
+void QCocoaWindow::windowWillZoom()
+{
+    updateNormalGeometry();
+}
+
 void QCocoaWindow::toggleFullScreen()
 {
     const NSWindow *window = m_view.window;
@@ -772,6 +811,8 @@ void QCocoaWindow::windowWillEnterFullScreen()
     if (!isContentView())
         return;
 
+    updateNormalGeometry();
+
     // The NSWindow needs to be resizable, otherwise we'll end up with
     // the normal window geometry, centered in the middle of the screen
     // on a black background. The styleMask will be reset below.

src/plugins/platforms/cocoa/qnswindowdelegate.mm

@@ -112,6 +112,14 @@ static QCocoaWindow *toPlatformWindow(NSWindow *window)
     return QCocoaScreen::mapToNative(maximizedFrame);
 }
 
+- (BOOL)windowShouldZoom:(NSWindow*)window toFrame:(NSRect)newFrame
+{
+    QCocoaWindow *platformWindow = toPlatformWindow(window);
+    Q_ASSERT(platformWindow);
+    platformWindow->windowWillZoom();
+    return YES;
+}
+
 - (BOOL)window:(NSWindow *)window shouldPopUpDocumentPathMenu:(NSMenu *)menu
 {
     Q_UNUSED(menu);

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

@@ -2486,6 +2486,24 @@ void tst_QWidget::activation()
 }
 #endif // Q_OS_WIN
 
+struct WindowStateChangeWatcher : public QObject
+{
+    WindowStateChangeWatcher(QWidget *widget)
+    {
+        Q_ASSERT(widget->window()->windowHandle());
+        widget->window()->windowHandle()->installEventFilter(this);
+        lastWindowStates = widget->window()->windowHandle()->windowState();
+    }
+    Qt::WindowStates lastWindowStates;
+protected:
+    bool eventFilter(QObject *receiver, QEvent *event) override
+    {
+        if (event->type() == QEvent::WindowStateChange)
+            lastWindowStates = static_cast<QWindow *>(receiver)->windowState();
+        return QObject::eventFilter(receiver, event);
+    }
+};
+
 void tst_QWidget::windowState()
 {
 #ifdef Q_OS_MACOS
@@ -3165,10 +3183,6 @@ void tst_QWidget::hideWhenFocusWidgetIsChild()
 
 void tst_QWidget::normalGeometry()
 {
-#ifdef Q_OS_MACOS
-    QSKIP("QTBUG-52974");
-#endif
-
     if (m_platform == QStringLiteral("wayland"))
         QSKIP("Wayland: This fails. Figure out why.");
     QWidget parent;
@@ -3179,96 +3193,101 @@ void tst_QWidget::normalGeometry()
     QCOMPARE(parent.normalGeometry(), parent.geometry());
     QCOMPARE(child->normalGeometry(), QRect());
 
-    const QRect testGeom = QRect(m_availableTopLeft + QPoint(100 ,100), m_testWidgetSize);
+    parent.setGeometry(QRect(m_availableTopLeft + QPoint(100 ,100), m_testWidgetSize));
-    parent.setGeometry(testGeom);
     parent.showNormal();
     QVERIFY(QTest::qWaitForWindowExposed(&parent));
-    QApplication::processEvents();
+    WindowStateChangeWatcher stateChangeWatcher(&parent);
 
-    QRect geom = parent.geometry();
+    const QRect normalGeometry = parent.geometry();
-    // ### the window manager places the top-left corner at
+    // We can't make any assumptions about the actual geometry compared to the
-    // ### 100,100... making geom something like 102,124 (offset by
+    // requested geometry. In this test, we only care about normalGeometry.
-    // ### the frame/frame)... this indicates a rather large different
+    QCOMPARE(parent.normalGeometry(), normalGeometry);
-    // ### between how X11 and Windows works
-    // QCOMPARE(geom, QRect(100, 100, 200, 200));
-    QCOMPARE(parent.normalGeometry(), geom);
 
     parent.setWindowState(parent.windowState() ^ Qt::WindowMaximized);
     QTRY_VERIFY(parent.windowState() & Qt::WindowMaximized);
-    QTRY_VERIFY(parent.geometry() != geom);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
-    QTRY_COMPARE(parent.normalGeometry(), geom);
+    QTRY_VERIFY(parent.geometry() != normalGeometry);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
 
     parent.setWindowState(parent.windowState() ^ Qt::WindowMaximized);
     QTRY_VERIFY(!(parent.windowState() & Qt::WindowMaximized));
-    QTRY_COMPARE(parent.geometry(), geom);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
-    QTRY_COMPARE(parent.normalGeometry(), geom);
+    QTRY_COMPARE(parent.geometry(), normalGeometry);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
 
     parent.showMaximized();
-    QTRY_VERIFY(parent.windowState() & Qt::WindowMaximized);
+    QTRY_VERIFY(parent.windowHandle()->windowState() & Qt::WindowMaximized);
-    QTRY_VERIFY(parent.geometry() != geom);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
-    QCOMPARE(parent.normalGeometry(), geom);
+    QTRY_VERIFY(parent.geometry() != normalGeometry);
+    QCOMPARE(parent.normalGeometry(), normalGeometry);
 
     parent.showNormal();
     QTRY_VERIFY(!(parent.windowState() & Qt::WindowMaximized));
-    QTRY_COMPARE(parent.geometry(), geom);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
-    QCOMPARE(parent.normalGeometry(), geom);
+    QTRY_COMPARE(parent.geometry(), normalGeometry);
+    QCOMPARE(parent.normalGeometry(), normalGeometry);
+
+    parent.setWindowState(parent.windowState() ^ Qt::WindowFullScreen);
+    QTRY_VERIFY(parent.windowState() & Qt::WindowFullScreen);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
+    QTRY_VERIFY(parent.geometry() != normalGeometry);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
+
+    parent.setWindowState(Qt::WindowNoState);
+    QTRY_VERIFY(!(parent.windowState() & Qt::WindowFullScreen));
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
+    QTRY_COMPARE(parent.geometry(), normalGeometry);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
+
+    parent.showFullScreen();
+    QTRY_VERIFY(parent.window()->windowState() & Qt::WindowFullScreen);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
+    QTRY_VERIFY(parent.geometry() != normalGeometry);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
+
+    parent.showNormal();
+    QTRY_VERIFY(!(parent.windowHandle()->windowState() & Qt::WindowFullScreen));
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
+    QTRY_COMPARE(parent.geometry(), normalGeometry);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
 
-    parent.setWindowState(parent.windowState() ^ Qt::WindowMinimized);
-    QTest::qWait(10);
-    parent.setWindowState(parent.windowState() ^ Qt::WindowMaximized);
-    QTest::qWait(10);
     if (m_platform == QStringLiteral("xcb"))
         QSKIP("QTBUG-26424");
-    QTRY_VERIFY(parent.windowState() & (Qt::WindowMinimized|Qt::WindowMaximized));
+
+    parent.setWindowState(parent.windowState() ^ Qt::WindowMaximized);
+    QTRY_VERIFY(stateChangeWatcher.lastWindowStates & Qt::WindowMaximized);
+    parent.setWindowState(parent.windowState() ^ Qt::WindowMinimized);
+    QTRY_VERIFY(stateChangeWatcher.lastWindowStates & Qt::WindowMinimized);
+
+    QTRY_COMPARE(parent.windowState() & (Qt::WindowMinimized|Qt::WindowMaximized), Qt::WindowMinimized|Qt::WindowMaximized);
+    QTRY_VERIFY(stateChangeWatcher.lastWindowStates & (Qt::WindowMinimized|Qt::WindowMaximized));
     // ### when minimized and maximized at the same time, the geometry
     // ### does *NOT* have to be the normal geometry, it could be the
     // ### maximized geometry.
     // QCOMPARE(parent.geometry(), geom);
-    QTRY_COMPARE(parent.normalGeometry(), geom);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
 
     parent.setWindowState(parent.windowState() ^ Qt::WindowMinimized);
-    QTest::qWait(10);
     QTRY_VERIFY(!(parent.windowState() & Qt::WindowMinimized));
     QTRY_VERIFY(parent.windowState() & Qt::WindowMaximized);
-    QTRY_VERIFY(parent.geometry() != geom);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
-    QTRY_COMPARE(parent.normalGeometry(), geom);
+    QTRY_VERIFY(parent.geometry() != normalGeometry);
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
 
     parent.setWindowState(parent.windowState() ^ Qt::WindowMaximized);
-    QTest::qWait(10);
     QTRY_VERIFY(!(parent.windowState() & Qt::WindowMaximized));
-    QTRY_COMPARE(parent.geometry(), geom);
+    QTRY_COMPARE(stateChangeWatcher.lastWindowStates, parent.windowState());
-    QTRY_COMPARE(parent.normalGeometry(), geom);
+    QTRY_COMPARE(parent.geometry(), normalGeometry);
-
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
-    parent.setWindowState(parent.windowState() ^ Qt::WindowFullScreen);
-    QTest::qWait(10);
-    QTRY_VERIFY(parent.windowState() & Qt::WindowFullScreen);
-    QTRY_VERIFY(parent.geometry() != geom);
-    QTRY_COMPARE(parent.normalGeometry(), geom);
-
-    parent.setWindowState(parent.windowState() ^ Qt::WindowFullScreen);
-    QTest::qWait(10);
-    QVERIFY(!(parent.windowState() & Qt::WindowFullScreen));
-    QTRY_COMPARE(parent.geometry(), geom);
-    QTRY_COMPARE(parent.normalGeometry(), geom);
-
-    parent.showFullScreen();
-    QTest::qWait(10);
-    QTRY_VERIFY(parent.windowState() & Qt::WindowFullScreen);
-    QTRY_VERIFY(parent.geometry() != geom);
-    QTRY_COMPARE(parent.normalGeometry(), geom);
-
-    parent.showNormal();
-    QTest::qWait(10);
-    QTRY_VERIFY(!(parent.windowState() & Qt::WindowFullScreen));
-    QTRY_COMPARE(parent.geometry(), geom);
-    QTRY_COMPARE(parent.normalGeometry(), geom);
 
     parent.showNormal();
+    stateChangeWatcher.lastWindowStates = {};
     parent.setWindowState(Qt:: WindowFullScreen | Qt::WindowMaximized);
     parent.setWindowState(Qt::WindowMinimized | Qt:: WindowFullScreen | Qt::WindowMaximized);
     parent.setWindowState(Qt:: WindowFullScreen | Qt::WindowMaximized);
-    QTest::qWait(10);
+    // the actual window will be either fullscreen or maximized
-    QTRY_COMPARE(parent.normalGeometry(), geom);
+    QTRY_VERIFY(stateChangeWatcher.lastWindowStates & (Qt:: WindowFullScreen | Qt::WindowMaximized));
+    QTRY_COMPARE(parent.normalGeometry(), normalGeometry);
 }
 
 void tst_QWidget::setGeometry()