From 9b05557a6325497485b582c3891367aa7dfee245 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 6 Feb 2019 09:30:09 +0100 Subject: 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 --- src/client/qwaylandinputdevice.cpp | 23 +++++++++++++++++++-- src/client/qwaylandinputdevice_p.h | 10 +++++++-- tests/auto/client/seatv4/tst_seatv4.cpp | 34 ++++++++++++++++++++++++++++++- tests/auto/client/shared/coreprotocol.cpp | 6 +++--- 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/client/qwaylandinputdevice.cpp b/src/client/qwaylandinputdevice.cpp index 65267869f..76000e542 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 cb382da31..50b1af385 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 surface; diff --git a/tests/auto/client/seatv4/tst_seatv4.cpp b/tests/auto/client/seatv4/tst_seatv4.cpp index 771307d7e..7dc2e727a 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 c4dc3f341..729d481f8 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; -- cgit v1.2.3