From 24002ac6cbd01dbde4944b63c1f7c87ed2bd72b5 Mon Sep 17 00:00:00 2001 From: David Redondo Date: Fri, 22 Nov 2024 10:56:41 +0100 Subject: [PATCH] client: Redo management of tablet object proxies Since 5af836aea3bb91a9f388e565415dc33b2fde4577 tools and pads can sometimes be parented to tablets. When a tablet is unplugged sometimes the remove event for the tablet can be sent before the remove events for the pad. Deleting the tablet will also delete the pad but not the pad proxy, resulting in a crash when the pad remove event is received. This moves destruction of the wayland proxies which makes everything much simpler as not every location that deletes those objects needs to call destroy and fixes the described problem. Change-Id: I0aaeda3d3996251e411229b5e97aa1ce0bce43ac Reviewed-by: David Edmundson (cherry picked from commit 1f76835d1805d9b1c25c136a19c1101f19cc2259) Reviewed-by: Qt Cherry-pick Bot --- src/client/qwaylandtabletv2.cpp | 24 +++++---- src/client/qwaylandtabletv2_p.h | 4 +- tests/auto/client/tabletv2/tst_tabletv2.cpp | 60 +++++++++++++++++---- 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/src/client/qwaylandtabletv2.cpp b/src/client/qwaylandtabletv2.cpp index ea2b225..67cee3b 100644 --- a/src/client/qwaylandtabletv2.cpp +++ b/src/client/qwaylandtabletv2.cpp @@ -187,12 +187,6 @@ QWaylandTabletSeatV2::QWaylandTabletSeatV2(QWaylandTabletManagerV2 *manager, QWa QWaylandTabletSeatV2::~QWaylandTabletSeatV2() { - for (auto *tablet : m_tablets) - tablet->destroy(); - for (auto *tool : m_tools) - tool->destroy(); - for (auto *pad : m_pads) - pad->destroy(); qDeleteAll(m_tablets); qDeleteAll(m_tools); qDeleteAll(m_deadTools); @@ -254,6 +248,11 @@ QWaylandTabletV2::QWaylandTabletV2(::zwp_tablet_v2 *tablet, const QString &seatN d->seatName = seatName; } +QWaylandTabletV2::~QWaylandTabletV2() +{ + destroy(); +} + void QWaylandTabletV2::zwp_tablet_v2_name(const QString &name) { QPointingDevicePrivate *d = QPointingDevicePrivate::get(this); @@ -292,7 +291,6 @@ void QWaylandTabletSeatV2::toolRemoved(QWaylandTabletToolV2 *tool) void QWaylandTabletV2::zwp_tablet_v2_removed() { - destroy(); deleteLater(); } @@ -316,7 +314,10 @@ QWaylandTabletToolV2::QWaylandTabletToolV2(QWaylandTabletSeatV2 *tabletSeat, ::z #endif } -QWaylandTabletToolV2::~QWaylandTabletToolV2() = default; +QWaylandTabletToolV2::~QWaylandTabletToolV2() +{ + destroy(); +} void QWaylandTabletToolV2::zwp_tablet_tool_v2_type(uint32_t tool_type) { @@ -410,7 +411,6 @@ void QWaylandTabletToolV2::zwp_tablet_tool_v2_done() void QWaylandTabletToolV2::zwp_tablet_tool_v2_removed() { - destroy(); m_tabletSeat->toolRemoved(this); } @@ -602,6 +602,11 @@ QWaylandTabletPadV2::QWaylandTabletPadV2(::zwp_tablet_pad_v2 *pad) { } +QWaylandTabletPadV2::~QWaylandTabletPadV2() +{ + destroy(); +} + void QWaylandTabletPadV2::zwp_tablet_pad_v2_path(const QString &path) { QPointingDevicePrivate *d = QPointingDevicePrivate::get(this); @@ -621,7 +626,6 @@ void QWaylandTabletPadV2::zwp_tablet_pad_v2_done() void QWaylandTabletPadV2::zwp_tablet_pad_v2_removed() { - destroy(); delete this; } diff --git a/src/client/qwaylandtabletv2_p.h b/src/client/qwaylandtabletv2_p.h index 94b687e..f0d7cd1 100644 --- a/src/client/qwaylandtabletv2_p.h +++ b/src/client/qwaylandtabletv2_p.h @@ -83,6 +83,7 @@ class Q_WAYLANDCLIENT_EXPORT QWaylandTabletV2 : public QPointingDevice, public Q Q_OBJECT public: explicit QWaylandTabletV2(::zwp_tablet_v2 *tablet, const QString &seatName); + ~QWaylandTabletV2(); protected: // callbacks which act as setters @@ -98,7 +99,7 @@ class Q_WAYLANDCLIENT_EXPORT QWaylandTabletToolV2 : public QPointingDevice, publ Q_OBJECT public: QWaylandTabletToolV2(QWaylandTabletSeatV2 *tabletSeat, ::zwp_tablet_tool_v2 *tool); - ~QWaylandTabletToolV2() override; + ~QWaylandTabletToolV2(); void updateCursor(); @@ -181,6 +182,7 @@ class Q_WAYLANDCLIENT_EXPORT QWaylandTabletPadV2 : public QPointingDevice, publi Q_OBJECT public: explicit QWaylandTabletPadV2(::zwp_tablet_pad_v2 *pad); + ~QWaylandTabletPadV2(); protected: // void zwp_tablet_pad_v2_group(struct ::zwp_tablet_pad_group_v2 *pad_group) override; diff --git a/tests/auto/client/tabletv2/tst_tabletv2.cpp b/tests/auto/client/tabletv2/tst_tabletv2.cpp index 85df099..d5c2ccb 100644 --- a/tests/auto/client/tabletv2/tst_tabletv2.cpp +++ b/tests/auto/client/tabletv2/tst_tabletv2.cpp @@ -186,9 +186,9 @@ public: QList m_tablets; QList m_tabletsWaitingForDestroy; QList m_tools; - QList m_toolsWaitingForDestroy; + QList m_toolsWaitingForDestroy; QList m_pads; - QList m_padsWaitingForDestroy; + QList m_padsWaitingForDestroy; protected: void zwp_tablet_seat_v2_bind_resource(Resource *resource) override @@ -274,11 +274,12 @@ void TabletV2::zwp_tablet_v2_destroy(QtWaylandServer::zwp_tablet_v2::Resource *r void TabletToolV2::sendRemoved() { - for (auto *resource : resourceMap()) + for (auto *resource : resourceMap()) { zwp_tablet_tool_v2_send_removed(resource->handle); + m_tabletSeat->m_toolsWaitingForDestroy.append(resource); + } bool removed = m_tabletSeat->m_tools.removeOne(this); QVERIFY(removed); - m_tabletSeat->m_toolsWaitingForDestroy.append(this); } uint TabletToolV2::sendProximityIn(TabletV2 *tablet, Surface *surface) @@ -333,26 +334,25 @@ uint TabletToolV2::sendFrame() void TabletToolV2::zwp_tablet_tool_v2_destroy(QtWaylandServer::zwp_tablet_tool_v2::Resource *resource) { if (m_tabletSeat) { - bool removed = m_tabletSeat->m_toolsWaitingForDestroy.removeOne(this); - QVERIFY(removed); + m_tabletSeat->m_toolsWaitingForDestroy.removeOne(resource); } wl_resource_destroy(resource->handle); } void TabletPadV2::sendRemoved() { - for (auto *resource : resourceMap()) + for (auto *resource : resourceMap()) { zwp_tablet_pad_v2_send_removed(resource->handle); + m_tabletSeat->m_padsWaitingForDestroy.append(resource); + } bool removed = m_tabletSeat->m_pads.removeOne(this); QVERIFY(removed); - m_tabletSeat->m_padsWaitingForDestroy.append(this); } void TabletPadV2::zwp_tablet_pad_v2_destroy(QtWaylandServer::zwp_tablet_pad_v2::Resource *resource) { if (m_tabletSeat) { - bool removed = m_tabletSeat->m_padsWaitingForDestroy.removeOne(this); - QVERIFY(removed); + m_tabletSeat->m_padsWaitingForDestroy.removeOne(resource); } wl_resource_destroy(resource->handle); } @@ -405,6 +405,8 @@ private slots: void destroysTablet(); void destroysTool(); void destroysPad(); + void removeTabletBeforeTool(); + void removeTabletBeforePad(); void proximityEvents(); void moveEvent(); void pointerType_data(); @@ -502,12 +504,14 @@ void tst_tabletv2::destroysTool() { QCOMPOSITOR_TRY_VERIFY(tabletSeat()); exec([&] { + tabletSeat()->addTablet(); tabletSeat()->addTool(); }); QCOMPOSITOR_TRY_VERIFY(tabletTool()); exec([&] { tabletTool()->sendRemoved(); + tablet()->sendRemoved(); }); QCOMPOSITOR_TRY_VERIFY(!tabletTool()); @@ -530,6 +534,42 @@ void tst_tabletv2::destroysPad() QCOMPOSITOR_TRY_VERIFY(tabletSeat()->m_padsWaitingForDestroy.empty()); } +void tst_tabletv2::removeTabletBeforeTool() +{ + QCOMPOSITOR_TRY_VERIFY(tabletSeat()); + exec([&] { + tabletSeat()->addTablet(); + tabletSeat()->addTool(); + }); + QCOMPOSITOR_TRY_VERIFY(tablet()); + QCOMPOSITOR_TRY_VERIFY(tabletTool()); + + exec([&] { tablet()->sendRemoved(); }); + QCOMPOSITOR_TRY_VERIFY(tabletSeat()->m_tabletsWaitingForDestroy.empty()); + + exec([&] { tabletTool()->sendRemoved(); }); + QCOMPOSITOR_TRY_VERIFY(!tabletTool()); + QCOMPOSITOR_TRY_VERIFY(tabletSeat()->m_toolsWaitingForDestroy.empty()); +} + +void tst_tabletv2::removeTabletBeforePad() +{ + QCOMPOSITOR_TRY_VERIFY(tabletSeat()); + exec([&] { + tabletSeat()->addTablet(); + tabletSeat()->addPad(); + }); + QCOMPOSITOR_TRY_VERIFY(tablet()); + QCOMPOSITOR_TRY_VERIFY(tabletPad()); + + exec([&] { tablet()->sendRemoved(); }); + QCOMPOSITOR_TRY_VERIFY(tabletSeat()->m_tabletsWaitingForDestroy.empty()); + + exec([&] { tabletPad()->sendRemoved(); }); + QCOMPOSITOR_TRY_VERIFY(!tabletPad()); + QCOMPOSITOR_TRY_VERIFY(tabletSeat()->m_padsWaitingForDestroy.empty()); +} + void tst_tabletv2::proximityEvents() { ProximityFilter filter;