Client: Don't attach buffers to unexposed windows

QBackingStore::flush is sometimes called with an unxeposed window, in that
case, don't attach the buffer to the wl_surface immediately, as that causes
protocol errors with xdg_shell.

Flushed buffers are instead stored until we get the first configure event.

[ChangeLog][QPA plugin][xdg-shell] Fixed a bug where buffers were sometimes
attached and committed before the first configure event, causing protocol
errors.

Fixes: QTBUG-71345
Change-Id: If9409d97bd25f6b13940c56141920a664c349c8e
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
Reviewed-by: David Edmundson <davidedmundson@kde.org>

7 files changed, 61 insertions, 10 deletions

diff --git a/src/client/qwaylandshmbackingstore.cpp b/src/client/qwaylandshmbackingstore.cpp
index ecb03c0d6..3fe2ce80c 100644
--- a/src/client/qwaylandshmbackingstore.cpp
+++ b/src/client/qwaylandshmbackingstore.cpp
@@ -234,8 +234,7 @@ void QWaylandShmBackingStore::flush(QWindow *window, const QRegion &region, cons
     mFrontBuffer = mBackBuffer;
 
     QMargins margins = windowDecorationMargins();
-
-    waylandWindow()->commit(mFrontBuffer, region.translated(margins.left(), margins.top()));
+    waylandWindow()->safeCommit(mFrontBuffer, region.translated(margins.left(), margins.top()));
 }
 
 void QWaylandShmBackingStore::resize(const QSize &size, const QRegion &)
diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
index 52bee6ae3..1801c2a4b 100644
--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -248,6 +248,7 @@ void QWaylandWindow::reset(bool sendDestroyEvent)
     }
 
     mMask = QRegion();
+    mQueuedBuffer = nullptr;
 }
 
 QWaylandWindow *QWaylandWindow::fromWlSurface(::wl_surface *surface)
@@ -566,8 +567,29 @@ void QWaylandWindow::damage(const QRect &rect)
     damage(rect.x(), rect.y(), rect.width(), rect.height());
 }
 
+void QWaylandWindow::safeCommit(QWaylandBuffer *buffer, const QRegion &damage)
+{
+    if (isExposed()) {
+        commit(buffer, damage);
+    } else {
+        mQueuedBuffer = buffer;
+        mQueuedBufferDamage = damage;
+    }
+}
+
+void QWaylandWindow::handleExpose(const QRegion &region)
+{
+    QWindowSystemInterface::handleExposeEvent(window(), region);
+    if (mQueuedBuffer) {
+        commit(mQueuedBuffer, mQueuedBufferDamage);
+        mQueuedBuffer = nullptr;
+        mQueuedBufferDamage = QRegion();
+    }
+}
+
 void QWaylandWindow::commit(QWaylandBuffer *buffer, const QRegion &damage)
 {
+    Q_ASSERT(isExposed());
     if (buffer->committed()) {
         qCDebug(lcWaylandBackingstore) << "Buffer already committed, ignoring.";
         return;
diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
index d11ed871b..ad24b7358 100644
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -116,6 +116,8 @@ public:
     using QtWayland::wl_surface::damage;
     void damage(const QRect &rect);
 
+    void safeCommit(QWaylandBuffer *buffer, const QRegion &damage);
+    void handleExpose(const QRegion &region);
     void commit(QWaylandBuffer *buffer, const QRegion &damage);
 
     void waitForFrameSync();
@@ -231,6 +233,8 @@ protected:
     Qt::WindowStates mLastReportedWindowStates = Qt::WindowNoState;
 
     QWaylandShmBackingStore *mBackingStore = nullptr;
+    QWaylandBuffer *mQueuedBuffer = nullptr;
+    QRegion mQueuedBufferDamage;
 
 private slots:
     void handleScreenRemoved(QScreen *qScreen);
diff --git a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp
index 447e8fb6a..3d3af6929 100644
--- a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp
+++ b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp
@@ -258,9 +258,14 @@ void QWaylandXdgSurfaceV6::setAppId(const QString &appId)
         m_toplevel->set_app_id(appId);
 }
 
+bool QWaylandXdgSurfaceV6::isExposed() const
+{
+    return m_configured || m_pendingConfigureSerial;
+}
+
 bool QWaylandXdgSurfaceV6::handleExpose(const QRegion &region)
 {
-    if (!m_configured && !region.isEmpty()) {
+    if (!isExposed() && !region.isEmpty()) {
         m_exposeRegion = region;
         return true;
     }
@@ -333,10 +338,18 @@ void QWaylandXdgSurfaceV6::setPopup(QWaylandWindow *parent, QWaylandInputDevice
 
 void QWaylandXdgSurfaceV6::zxdg_surface_v6_configure(uint32_t serial)
 {
-    m_window->applyConfigureWhenPossible();
     m_pendingConfigureSerial = serial;
+    if (!m_configured) {
+        // We have to do the initial applyConfigure() immediately, since that is the expose.
+        applyConfigure();
+    } else {
+        // Later configures are probably resizes, so we have to queue them up for a time when we
+        // are not painting to the window.
+        m_window->applyConfigureWhenPossible();
+    }
+
     if (!m_exposeRegion.isEmpty()) {
-        QWindowSystemInterface::handleExposeEvent(m_window->window(), m_exposeRegion);
+        m_window->handleExpose(m_exposeRegion);
         m_exposeRegion = QRegion();
     }
 }
diff --git a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h
index 38b711f88..874dba014 100644
--- a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h
+++ b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h
@@ -85,7 +85,7 @@ public:
     void setTitle(const QString &title) override;
     void setAppId(const QString &appId) override;
 
-    bool isExposed() const override { return m_configured; }
+    bool isExposed() const override;
     bool handleExpose(const QRegion &) override;
     bool handlesActiveState() const { return m_toplevel; }
     void applyConfigure() override;
diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp
index 0756e4d02..c723192c8 100644
--- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp
+++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp
@@ -292,9 +292,14 @@ void QWaylandXdgSurface::setWindowFlags(Qt::WindowFlags flags)
         m_toplevel->requestWindowFlags(flags);
 }
 
+bool QWaylandXdgSurface::isExposed() const
+{
+    return m_configured || m_pendingConfigureSerial;
+}
+
 bool QWaylandXdgSurface::handleExpose(const QRegion &region)
 {
-    if (!m_configured && !region.isEmpty()) {
+    if (!isExposed() && !region.isEmpty()) {
         m_exposeRegion = region;
         return true;
     }
@@ -367,10 +372,18 @@ void QWaylandXdgSurface::setPopup(QWaylandWindow *parent, QWaylandInputDevice *d
 
 void QWaylandXdgSurface::xdg_surface_configure(uint32_t serial)
 {
-    m_window->applyConfigureWhenPossible();
     m_pendingConfigureSerial = serial;
+    if (!m_configured) {
+        // We have to do the initial applyConfigure() immediately, since that is the expose.
+        applyConfigure();
+    } else {
+        // Later configures are probably resizes, so we have to queue them up for a time when we
+        // are not painting to the window.
+        m_window->applyConfigureWhenPossible();
+    }
+
     if (!m_exposeRegion.isEmpty()) {
-        QWindowSystemInterface::handleExposeEvent(m_window->window(), m_exposeRegion);
+        m_window->handleExpose(m_exposeRegion);
         m_exposeRegion = QRegion();
     }
 }
diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h
index 45d7d4b0e..5e97a34b3 100644
--- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h
+++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h
@@ -89,7 +89,7 @@ public:
     void setAppId(const QString &appId) override;
     void setWindowFlags(Qt::WindowFlags flags) override;
 
-    bool isExposed() const override { return m_configured; }
+    bool isExposed() const override;
     bool handleExpose(const QRegion &) override;
     bool handlesActiveState() const { return m_toplevel; }
     void applyConfigure() override;