emit QWindow::windowStateChanged only when state has changed

Upon programmatic window state changes, windowStateChange was fired
once in QWindow::setWindowStates and once when the visual state had
been changed by the window interface.

This patch adds if guards to ensure that the singal is fired only once.

It adds a corresponding autotest to tst_QWindow.
tst_QWidget::resizePropagation() is adapted to no longer expect double
signal emission.

Fixes: QTBUG-102478
Pick-to: 6.4 6.2
Change-Id: If093c0a883d76d8a676e4fab90db6b0676452267
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

src/gui/kernel/qguiapplication.cpp

@@ -2509,11 +2509,16 @@ void QGuiApplicationPrivate::processWindowStateChangedEvent(QWindowSystemInterfa
 {
     if (QWindow *window = wse->window.data()) {
         QWindowPrivate *windowPrivate = qt_window_private(window);
-        const auto newEffectiveState = QWindowPrivate::effectiveState(windowPrivate->windowState);
+        const auto originalEffectiveState = QWindowPrivate::effectiveState(windowPrivate->windowState);
-        QWindowStateChangeEvent e(wse->oldState);
+
         windowPrivate->windowState = wse->newState;
-        emit window->windowStateChanged(newEffectiveState);
+        const auto newEffectiveState = QWindowPrivate::effectiveState(windowPrivate->windowState);
+        if (newEffectiveState != originalEffectiveState)
+            emit window->windowStateChanged(newEffectiveState);
+
         windowPrivate->updateVisibility();
+
+        QWindowStateChangeEvent e(wse->oldState);
         QGuiApplication::sendSpontaneousEvent(window, &e);
     }
 }

src/gui/kernel/qwindow.cpp

@@ -1384,8 +1384,13 @@ void QWindow::setWindowStates(Qt::WindowStates state)
 
     if (d->platformWindow)
         d->platformWindow->setWindowState(state);
+
+    auto originalEffectiveState = QWindowPrivate::effectiveState(d->windowState);
     d->windowState = state;
-    emit windowStateChanged(QWindowPrivate::effectiveState(d->windowState));
+    auto newEffectiveState = QWindowPrivate::effectiveState(d->windowState);
+    if (newEffectiveState != originalEffectiveState)
+        emit windowStateChanged(newEffectiveState);
+
     d->updateVisibility();
 }
 

tests/auto/gui/kernel/qwindow/tst_qwindow.cpp

@@ -88,6 +88,7 @@ private slots:
     void activateDeactivateEvent();
     void qobject_castOnDestruction();
     void touchToMouseTranslationByPopup();
+    void stateChangeSignal();
 
 private:
     QPoint m_availableTopLeft;
@@ -2778,6 +2779,103 @@ void tst_QWindow::touchToMouseTranslationByPopup()
     QTRY_COMPARE(window.mouseReleaseButton, int(Qt::LeftButton));
 }
 
+// Test that windowStateChanged is not emitted on noop change (QTBUG-102478)
+void tst_QWindow::stateChangeSignal()
+{
+    // Test only for Windows, Linux and macOS
+#if !defined(Q_OS_LINUX) && !defined(Q_OS_WINDOWS) && !defined(Q_OS_DARWIN)
+    QSKIP("Singular windowStateChanged signal emission is guaranteed for Linux, Windows and macOS only.\n"
+          "On other operating systems, the signal may be emitted twice.");
+#endif
+    QWindow w;
+    Q_ASSUME(connect (&w, &QWindow::windowStateChanged, [](Qt::WindowState s){qCDebug(lcTests) << "State change to" << s;}));
+    QSignalSpy spy(&w, SIGNAL(windowStateChanged(Qt::WindowState)));
+    unsigned short signalCount = 0;
+    QList<Qt::WindowState> effectiveStates;
+    Q_ASSUME(connect(&w, &QWindow::windowStateChanged, [&effectiveStates](Qt::WindowState state)
+            { effectiveStates.append(state); }));
+    // Part 1:
+    // => test signal emission on programmatic state changes
+    QCOMPARE(w.windowState(), Qt::WindowNoState);
+    // - wait for target state to be set
+    // - wait for signal spy to have reached target count
+    // - extract state from signal and compare to target
+#define CHECK_STATE(State)\
+    QTRY_VERIFY(QTest::qWaitFor([&w](){return (w.windowState() == State); }));\
+    CHECK_SIGNAL(State)
+#define CHECK_SIGNAL(State)\
+    QTRY_COMPARE(spy.count(), signalCount);\
+    if (signalCount > 0) {\
+        QVariantList list = spy.at(signalCount - 1).toList();\
+        QCOMPARE(list.count(), 1);\
+        bool ok;\
+        const int stateInt = list.at(0).toInt(&ok);\
+        QVERIFY(ok);\
+        const Qt::WindowState newState = static_cast<Qt::WindowState>(stateInt);\
+        QCOMPARE(newState, State);\
+    }
+    // Check initialization
+    CHECK_STATE(Qt::WindowNoState);
+    // showMaximized after init
+    // expected behavior: signal emitted once with state == WindowMaximized
+    ++signalCount;
+    w.showMaximized();
+    CHECK_STATE(Qt::WindowMaximized);
+    // setWindowState to normal
+    // expected behavior: signal emitted once with state == WindowNoState
+    ++signalCount;
+    w.setWindowState(Qt::WindowNoState);
+    CHECK_STATE(Qt::WindowNoState);
+    // redundant setWindowState to normal - except windows, where the no-op is counted
+    // expected behavior: No emits.
+    // On Windows, a no-op state change causes a no-op resize and repaint, leading to a
+    // no-op state change and singal emission.
+#ifdef Q_OS_WINDOWS
+    ++signalCount;
+    ++signalCount;
+#endif
+    w.setWindowState(Qt::WindowNoState);
+    CHECK_STATE(Qt::WindowNoState);
+    // setWindowState to minimized
+    // expected behavior: signal emitted once with state == WindowMinimized
+    ++signalCount;
+    w.showMinimized();
+    CHECK_STATE(Qt::WindowMinimized);
+    // setWindowState to Normal
+    // expected behavior: signal emitted once with state == WindowNoState
+    ++signalCount;
+    w.showNormal();
+    CHECK_STATE(Qt::WindowNoState);
+    /*
+     - Testcase showFullScreen is omitted: Depending on window manager,
+     WindowFullScreen can be mapped to WindowMaximized
+     - Transition from WindowMinimized to WindowMaximized is omitted:
+     WindowNoState to WindowMaximized
+     */
+    // Part 2:
+    // => test signal emission on simulated user interaction
+    // To test the code path, inject state change events into the QPA event queue.
+    // Test the signal emission only, not the window's actual visible state.
+
+    // Flush pending events and clear
+    QCoreApplication::processEvents();
+    spy.clear();
+    effectiveStates.clear();
+    signalCount = 0;
+    // Maximize window
+    QWindowSystemInterface::handleWindowStateChanged(&w, Qt::WindowMaximized, w.windowState());
+    ++signalCount;
+    CHECK_SIGNAL(Qt::WindowMaximized);
+    // Normalize window
+    QWindowSystemInterface::handleWindowStateChanged(&w, Qt::WindowNoState, w.windowState());
+    ++signalCount;
+    CHECK_SIGNAL(Qt::WindowNoState);
+    // Minimize window
+    QWindowSystemInterface::handleWindowStateChanged(&w, Qt::WindowMinimized, w.windowState());
+    ++signalCount;
+    CHECK_SIGNAL(Qt::WindowMinimized);
+}
+
 #include <tst_qwindow.moc>
 QTEST_MAIN(tst_QWindow)
 

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

@@ -2752,18 +2752,15 @@ void tst_QWidget::resizePropagation()
         QSKIP("resizePropagation test is designed for XCB only");
 #endif
 
-    // Platform specific notes:
-    // Linux/XCB:
-    // - Unless maximized, a widget and its corresponding window can have different sizes
-    // - windowStateChanged can be fired multiple times (QTBUG-102478) when widget is maximized
-    //
     // Windows:
     // When a widget is maximized after it has been resized, the widget retains its original size,
-    // while the window shows maximum size
+    // while the window shows maximum size.
+    // windowStateChanged signal gets fired on a no-op change from/to WindowNoState
 
     // Initialize widget and signal spy for window handle
     QWidget widget;
     widget.showMaximized();
+    QVERIFY(QTest::qWaitForWindowExposed(&widget));
     QWindow *window = widget.windowHandle();
     QTRY_VERIFY(window);
     QSignalSpy spy(window, &QWindow::windowStateChanged);
@@ -2773,11 +2770,15 @@ void tst_QWidget::resizePropagation()
     const QSize size1 = QSize(screenSize.width() * 0.5, screenSize.height() * 0.5);
     const QSize size2 = QSize(screenSize.width() * 0.625, screenSize.height() * 0.833);
 
-    auto verifyResize = [&](const QSize &size, Qt::WindowState windowState, bool checkCountIncrement, bool checkTargetSize)
+    enum CountIncrementCheck {Equal, Greater};
+    enum TargetSizeCheck {Fail, Warn};
+    auto verifyResize = [&](const QSize &size, Qt::WindowState windowState,
+                            CountIncrementCheck checkCountIncrement,
+                            TargetSizeCheck checkTargetSize)
     {
         // Capture count of latest async signals
-        if (!checkCountIncrement)
+        if (checkCountIncrement == Equal)
-            count = spy.size();
+            count = spy.count();
 
         // Resize if required
         if (size.isValid())
@@ -2787,13 +2788,24 @@ void tst_QWidget::resizePropagation()
         QVERIFY(QTest::qWaitForWindowExposed(&widget));
 
         // Check signal count and qDebug output for fail analysis
-        if (checkCountIncrement) {
+        switch (checkCountIncrement) {
-            QTRY_VERIFY(spy.size() > count);
+        case Greater: {
-            qDebug() << "spy count:" << spy.size() << "previous count:" << count;
+                auto logger = qScopeGuard([&](){
-            count = spy.size();
+                    qDebug() << "spy count:" << spy.count() << "previous count:" << count;
-        } else {
+                });
-            qDebug() << spy << widget.windowState() << window->windowState();
+                QTRY_VERIFY(spy.count() > count);
-            QCOMPARE(spy.size(), count);
+                logger.dismiss();
+                count = spy.count();
+            }
+            break;
+        case Equal: {
+                auto logger = qScopeGuard([&](){
+                   qDebug() << spy << widget.windowState() << window->windowState();
+                });
+                QCOMPARE(spy.count(), count);
+                logger.dismiss();
+            }
+            break;
         }
 
         // QTRY necessary because state changes are propagated async
@@ -2801,32 +2813,37 @@ void tst_QWidget::resizePropagation()
         QTRY_COMPARE(window->windowState(), windowState);
 
         // Check target size with fail or warning
-        if (checkTargetSize) {
+        switch (checkTargetSize) {
+        case Fail:
             QCOMPARE(widget.size(), window->size());
-        } else if (widget.size() != window->size()) {
+            break;
-            qWarning() << m_platform << "size mismtach tolerated. Widget:"
+        case Warn:
-                       << widget.size() << "Window:" << window->size();
+            if (widget.size() != window->size()) {
+                qWarning() << m_platform << "size mismtach tolerated. Widget:"
+                           << widget.size() << "Window:" << window->size();
+            }
+            break;
         }
     };
 
     // test state and size consistency of maximized window
-    verifyResize(QSize(), Qt::WindowMaximized, true, true);
+    verifyResize(QSize(), Qt::WindowMaximized, Equal, Fail);
     if (QTest::currentTestFailed())
         return;
 
     // test state transition, state and size consistency after resize
-    verifyResize(size1, Qt::WindowNoState, true, !xcb );
+    verifyResize(size1, Qt::WindowNoState, Greater, xcb ? Warn : Fail );
     if (QTest::currentTestFailed())
         return;
 
     // test unchanged state, state and size consistency after resize
-    verifyResize(size2, Qt::WindowNoState, false, !xcb);
+    verifyResize(size2, Qt::WindowNoState, Equal, xcb ? Warn : Fail);
     if (QTest::currentTestFailed())
         return;
 
     // test state transition, state and size consistency after maximize
     widget.showMaximized();
-    verifyResize(QSize(), Qt::WindowMaximized, true, xcb);
+    verifyResize(QSize(), Qt::WindowMaximized, Greater, xcb ? Fail : Warn);
     if (QTest::currentTestFailed())
         return;