QSplashScreen: Enforce visibility by processing events

A splash screen is usually shown during initialization, before the main
window is shown. At that point, an application might not spin an event
loop. The documentation therefore recommends to run processEvents()
periodically, in order to ensure that the splash screen is actually
shown. Async window managers (e.g. XCB) might require more than one call
to processEvents() for the splash screen to be shown. It can't be
guaranteed, that a specific number of calls to processEvents() make the
splash screen visible. Calls to processEvents() are not considered
good practice in application code.

QSplashScreen::finish() uses a copy of QTest::qWaitForWindowExposed(),
to automatically close the splash screen, when the main window is
shown.

=> Re-write the static inline waitForWindowExposed to wait for the
widget's windowHandle()->isVisible(). That ensures visibility to the
window system.
The previous version waited for isExposed() to become true. That
ensured exposure to the application, but not visibility.

=> Intercept QEvent::Show and process events, until the splash screen
becomes visible.

=> Remove references to processEvents() as a requirement to show the
splash screen from the documentation. It remains a requirement for
processing mouse and keyboard events.

=> Re-activate tst_QSplashScreen and test visibility in the constructor
test function.

Drive by:
- implement switch in event override
- factor out paint event handling in a function
- update documentation about calling processEvents() to enable mouse
handling.

Fixes: QTBUG-119225
Pick-to: 6.7 6.6 6.5
Change-Id: I07910705246b4e3234e01cc4896f5bc4b2c56772
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

3 files changed, 49 insertions, 24 deletions

diff --git a/src/widgets/widgets/qsplashscreen.cpp b/src/widgets/widgets/qsplashscreen.cpp
index 040b7424cf..fcd09908cd 100644
--- a/src/widgets/widgets/qsplashscreen.cpp
+++ b/src/widgets/widgets/qsplashscreen.cpp
@@ -32,6 +32,8 @@ public:
     int currAlign;
 
     inline QSplashScreenPrivate();
+
+    void handlePaintEvent();
 };
 
 /*!
@@ -67,9 +69,9 @@ public:
    \snippet qsplashscreen/main.cpp 1
 
    The user can hide the splash screen by clicking on it with the
-   mouse. Since the splash screen is typically displayed before the
-   event loop has started running, it is necessary to periodically
-   call QCoreApplication::processEvents() to receive the mouse clicks.
+   mouse. For mouse handling to work, call QApplication::processEvents()
+   periodically during startup.
+
 
    It is sometimes useful to update the splash screen with messages,
    for example, announcing connections established or modules loaded
@@ -201,18 +203,21 @@ void QSplashScreen::clearMessage()
     repaint();
 }
 
-// A copy of Qt Test's qWaitForWindowExposed() and qSleep().
-inline static bool waitForWindowExposed(QWindow *window, int timeout = 1000)
+static bool waitForWidgetMapped(QWidget *widget, int timeout = 1000)
 {
     enum { TimeOutMs = 10 };
+    auto isMapped = [widget](){
+        return widget->windowHandle() && widget->windowHandle()->isVisible();
+    };
+
     QElapsedTimer timer;
     timer.start();
-    while (!window->isExposed()) {
+    while (!isMapped()) {
         const int remaining = timeout - int(timer.elapsed());
         if (remaining <= 0)
             break;
         QCoreApplication::processEvents(QEventLoop::AllEvents, remaining);
-        QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
+        QCoreApplication::sendPostedEvents();
 #if defined(Q_OS_WIN)
         Sleep(uint(TimeOutMs));
 #else
@@ -220,7 +225,7 @@ inline static bool waitForWindowExposed(QWindow *window, int timeout = 1000)
         nanosleep(&ts, nullptr);
 #endif
     }
-    return window->isExposed();
+    return isMapped();
 }
 
 /*!
@@ -233,7 +238,7 @@ void QSplashScreen::finish(QWidget *mainWin)
     if (mainWin) {
         if (!mainWin->windowHandle())
             mainWin->createWinId();
-        waitForWindowExposed(mainWin->windowHandle());
+        waitForWidgetMapped(mainWin);
     }
     close();
 }
@@ -311,18 +316,33 @@ void QSplashScreen::drawContents(QPainter *painter)
     }
 }
 
+void QSplashScreenPrivate::handlePaintEvent()
+{
+    Q_Q(QSplashScreen);
+    QPainter painter(q);
+    painter.setRenderHints(QPainter::SmoothPixmapTransform);
+    painter.setLayoutDirection(q->layoutDirection());
+    if (!pixmap.isNull())
+        painter.drawPixmap(QPoint(), pixmap);
+    q->drawContents(&painter);
+}
+
 /*! \reimp */
 bool QSplashScreen::event(QEvent *e)
 {
-    if (e->type() == QEvent::Paint) {
-        Q_D(QSplashScreen);
-        QPainter painter(this);
-        painter.setRenderHints(QPainter::SmoothPixmapTransform);
-        painter.setLayoutDirection(layoutDirection());
-        if (!d->pixmap.isNull())
-            painter.drawPixmap(QPoint(), d->pixmap);
-        drawContents(&painter);
+    Q_D(QSplashScreen);
+
+    switch (e->type()) {
+    case QEvent::Paint:
+        d->handlePaintEvent();
+        break;
+    case QEvent::Show:
+        waitForWidgetMapped(this);
+        break;
+    default:
+        break;
     }
+
     return QWidget::event(e);
 }
 
diff --git a/tests/auto/widgets/widgets/CMakeLists.txt b/tests/auto/widgets/widgets/CMakeLists.txt
index c6c940a40c..eb44f3d103 100644
--- a/tests/auto/widgets/widgets/CMakeLists.txt
+++ b/tests/auto/widgets/widgets/CMakeLists.txt
@@ -29,6 +29,7 @@ add_subdirectory(qscrollbar)
 add_subdirectory(qsizegrip)
 add_subdirectory(qslider)
 add_subdirectory(qspinbox)
+add_subdirectory(qsplashscreen)
 add_subdirectory(qsplitter)
 add_subdirectory(qstackedwidget)
 add_subdirectory(qstatusbar)
diff --git a/tests/auto/widgets/widgets/qsplashscreen/tst_qsplashscreen.cpp b/tests/auto/widgets/widgets/qsplashscreen/tst_qsplashscreen.cpp
index f397777065..45a05480a2 100644
--- a/tests/auto/widgets/widgets/qsplashscreen/tst_qsplashscreen.cpp
+++ b/tests/auto/widgets/widgets/qsplashscreen/tst_qsplashscreen.cpp
@@ -4,6 +4,7 @@
 
 #include <QTest>
 #include <QSplashScreen>
+#include <QTimer>
 
 class tst_QSplashScreen : public QObject
 {
@@ -11,7 +12,7 @@ class tst_QSplashScreen : public QObject
 
 private slots:
     void checkCloseTime();
-    void checkScreenConstructor();
+    void checkConstructorAndShow();
 };
 
 class CloseEventSplash : public QSplashScreen
@@ -20,7 +21,7 @@ public:
     CloseEventSplash(const QPixmap &pix) : QSplashScreen(pix), receivedCloseEvent(false) {}
     bool receivedCloseEvent;
 protected:
-    void closeEvent(QCloseEvent *event)
+    void closeEvent(QCloseEvent *event) override
     {
         receivedCloseEvent = true;
         QSplashScreen::closeEvent(event);
@@ -35,23 +36,26 @@ void tst_QSplashScreen::checkCloseTime()
     QVERIFY(!splash.receivedCloseEvent);
     QWidget w;
     splash.show();
-    QTimer::singleShot(500, &w, SLOT(show()));
+    QTimer::singleShot(10, &w, &QWidget::show);
     QVERIFY(!splash.receivedCloseEvent);
     splash.finish(&w);
     QVERIFY(splash.receivedCloseEvent);
     // We check the window handle because if this is not valid, then
     // it can't have been exposed
     QVERIFY(w.windowHandle());
-    QVERIFY(w.windowHandle()->isExposed());
+    QVERIFY(w.windowHandle()->isVisible());
 }
 
-void tst_QSplashScreen::checkScreenConstructor()
+void tst_QSplashScreen::checkConstructorAndShow()
 {
-    for (const auto screen : QGuiApplication::screens()) {
-        QSplashScreen splash(screen);
+    QPixmap pix(100, 100);
+    pix.fill(Qt::red);
+    for (auto *screen : QGuiApplication::screens()) {
+        QSplashScreen splash(screen, pix);
         splash.show();
         QCOMPARE(splash.screen(), screen);
         QVERIFY(splash.windowHandle());
+        QVERIFY(splash.windowHandle()->isVisible());
         QCOMPARE(splash.windowHandle()->screen(), screen);
     }
 }