Client: Don't send illegal wl_pointer.set_cursor requests

When the cursor focus' wl_surface is destroyed, or the pointer leaves a
surface, we have to reset the enter serial to avoid sending illegal set_cursor
requests.

Change-Id: I0c886e4123acb4aebd325b07bf15b9d3fa8589da
Reviewed-by: Giulio Camuffo <giulio.camuffo@kdab.com>

4 files changed, 65 insertions, 8 deletions

diff --git a/src/client/qwaylandinputdevice.cpp b/src/client/qwaylandinputdevice.cpp
index 65267869..76000e54 100644
--- a/src/client/qwaylandinputdevice.cpp
+++ b/src/client/qwaylandinputdevice.cpp
@@ -209,7 +209,9 @@ public:
 
     void hide()
     {
-        m_pointer->set_cursor(m_pointer->mEnterSerial, nullptr, 0, 0);
+        uint serial = m_pointer->mEnterSerial;
+        Q_ASSERT(serial);
+        m_pointer->set_cursor(serial, nullptr, 0, 0);
         m_setSerial = 0;
     }
 
@@ -581,7 +583,16 @@ void QWaylandInputDevice::Pointer::pointer_enter(uint32_t serial, struct wl_surf
         return;
 
     QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface);
+
+    if (mFocus) {
+        qWarning(lcQpaWayland) << "The compositor sent a wl_pointer.enter event before sending a"
+                               << "leave event first, this is not allowed by the wayland protocol"
+                               << "attempting to work around it by invalidating the current focus";
+        invalidateFocus();
+    }
     mFocus = window;
+    connect(mFocus, &QWaylandWindow::wlSurfaceDestroyed, this, &Pointer::handleFocusDestroyed);
+
     mSurfacePos = QPointF(wl_fixed_to_double(sx), wl_fixed_to_double(sy));
     mGlobalPos = window->window()->mapToGlobal(mSurfacePos.toPoint());
 
@@ -611,7 +622,8 @@ void QWaylandInputDevice::Pointer::pointer_leave(uint32_t time, struct wl_surfac
         QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface);
         window->handleMouseLeave(mParent);
     }
-    mFocus = nullptr;
+
+    invalidateFocus();
     mButtons = Qt::NoButton;
 
     mParent->mTime = time;
@@ -714,6 +726,13 @@ void QWaylandInputDevice::Pointer::pointer_button(uint32_t serial, uint32_t time
     }
 }
 
+void QWaylandInputDevice::Pointer::invalidateFocus()
+{
+    disconnect(mFocus, &QWaylandWindow::wlSurfaceDestroyed, this, &Pointer::handleFocusDestroyed);
+    mFocus = nullptr;
+    mEnterSerial = 0;
+}
+
 void QWaylandInputDevice::Pointer::releaseButtons()
 {
     mButtons = Qt::NoButton;
diff --git a/src/client/qwaylandinputdevice_p.h b/src/client/qwaylandinputdevice_p.h
index cb382da3..50b1af38 100644
--- a/src/client/qwaylandinputdevice_p.h
+++ b/src/client/qwaylandinputdevice_p.h
@@ -250,8 +250,9 @@ private:
 
 };
 
-class Q_WAYLAND_CLIENT_EXPORT QWaylandInputDevice::Pointer : public QtWayland::wl_pointer
+class Q_WAYLAND_CLIENT_EXPORT QWaylandInputDevice::Pointer : public QObject, public QtWayland::wl_pointer
 {
+    Q_OBJECT
 public:
     explicit Pointer(QWaylandInputDevice *seat);
     ~Pointer() override;
@@ -277,6 +278,12 @@ protected:
                       uint32_t axis,
                       wl_fixed_t value) override;
 
+private slots:
+    void handleFocusDestroyed() { invalidateFocus(); }
+
+private:
+    void invalidateFocus();
+
 public:
     void releaseButtons();
 
@@ -285,7 +292,6 @@ public:
     uint32_t mEnterSerial = 0;
 #if QT_CONFIG(cursor)
     struct {
-        uint32_t serial = 0;
         QWaylandCursorTheme *theme = nullptr;
         int themeBufferScale = 0;
         QScopedPointer<CursorSurface> surface;
diff --git a/tests/auto/client/seatv4/tst_seatv4.cpp b/tests/auto/client/seatv4/tst_seatv4.cpp
index 771307d7..7dc2e727 100644
--- a/tests/auto/client/seatv4/tst_seatv4.cpp
+++ b/tests/auto/client/seatv4/tst_seatv4.cpp
@@ -70,6 +70,7 @@ private slots:
     void createsPointer();
     void setsCursorOnEnter();
     void usesEnterSerial();
+    void focusDestruction();
     void mousePress();
     void simpleAxis_data();
     void simpleAxis();
@@ -147,12 +148,43 @@ void tst_seatv4::usesEnterSerial()
     uint enterSerial = exec([=] {
         return pointer()->sendEnter(xdgSurface()->m_surface, {32, 32});
     });
-    QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface());
+    QCOMPOSITOR_TRY_VERIFY(cursorSurface());
 
     QTRY_COMPARE(setCursorSpy.count(), 1);
     QCOMPARE(setCursorSpy.takeFirst().at(0).toUInt(), enterSerial);
 }
 
+void tst_seatv4::focusDestruction()
+{
+    QSignalSpy setCursorSpy(exec([=] { return pointer(); }), &Pointer::setCursor);
+    QRasterWindow window;
+    window.resize(64, 64);
+    window.show();
+    QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
+    // Setting a cursor now is not allowed since there has been no enter event
+    QCOMPARE(setCursorSpy.count(), 0);
+
+    uint enterSerial = exec([=] {
+        return pointer()->sendEnter(xdgSurface()->m_surface, {32, 32});
+    });
+    QCOMPOSITOR_TRY_VERIFY(cursorSurface());
+    QTRY_COMPARE(setCursorSpy.count(), 1);
+    QCOMPARE(setCursorSpy.takeFirst().at(0).toUInt(), enterSerial);
+
+    // Destroy the focus
+    window.close();
+
+    QRasterWindow window2;
+    window2.resize(64, 64);
+    window2.show();
+    window2.setCursor(Qt::WaitCursor);
+    QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
+
+    // Setting a cursor now is not allowed since there has been no enter event
+    xdgPingAndWaitForPong();
+    QCOMPARE(setCursorSpy.count(), 0);
+}
+
 void tst_seatv4::mousePress()
 {
     class Window : public QRasterWindow {
diff --git a/tests/auto/client/shared/coreprotocol.cpp b/tests/auto/client/shared/coreprotocol.cpp
index c4dc3f34..729d481f 100644
--- a/tests/auto/client/shared/coreprotocol.cpp
+++ b/tests/auto/client/shared/coreprotocol.cpp
@@ -261,6 +261,7 @@ uint Pointer::sendEnter(Surface *surface, const QPointF &position)
 
     uint serial = m_seat->m_compositor->nextSerial();
     m_enterSerials << serial;
+    m_cursorRole = nullptr; // According to the protocol, the pointer image is undefined after enter
 
     wl_client *client = surface->resource()->client();
     const auto pointerResources = resourceMap().values(client);
@@ -320,9 +321,8 @@ void Pointer::pointer_set_cursor(Resource *resource, uint32_t serial, wl_resourc
     QVERIFY(s);
 
     if (s->m_role) {
-        auto *cursorRole = CursorRole::fromSurface(s);
-        QVERIFY(cursorRole);
-        QVERIFY(cursorRole == m_cursorRole);
+        m_cursorRole = CursorRole::fromSurface(s);
+        QVERIFY(m_cursorRole);
     } else {
         m_cursorRole = new CursorRole(s); //TODO: make sure we don't leak CursorRole
         s->m_role = m_cursorRole;