From fef73b9baa50a5d83289c0adabbdd769950c12c4 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 bb151390240c1f20a6464adbefa497f3559554dd 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. Pick-to: 6.8 Change-Id: I0aaeda3d3996251e411229b5e97aa1ce0bce43ac Reviewed-by: David Edmundson --- .../platforms/wayland/qwaylandtabletv2.cpp | 24 ++++---- .../platforms/wayland/qwaylandtabletv2_p.h | 4 +- tests/auto/wayland/tabletv2/tst_tabletv2.cpp | 60 +++++++++++++++---- 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandtabletv2.cpp b/src/plugins/platforms/wayland/qwaylandtabletv2.cpp index ffd4abd5878..cca00ca7d35 100644 --- a/src/plugins/platforms/wayland/qwaylandtabletv2.cpp +++ b/src/plugins/platforms/wayland/qwaylandtabletv2.cpp @@ -185,12 +185,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); @@ -252,6 +246,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); @@ -290,7 +289,6 @@ void QWaylandTabletSeatV2::toolRemoved(QWaylandTabletToolV2 *tool) void QWaylandTabletV2::zwp_tablet_v2_removed() { - destroy(); deleteLater(); } @@ -314,7 +312,10 @@ QWaylandTabletToolV2::QWaylandTabletToolV2(QWaylandTabletSeatV2 *tabletSeat, ::z #endif } -QWaylandTabletToolV2::~QWaylandTabletToolV2() = default; +QWaylandTabletToolV2::~QWaylandTabletToolV2() +{ + destroy(); +} void QWaylandTabletToolV2::zwp_tablet_tool_v2_type(uint32_t tool_type) { @@ -408,7 +409,6 @@ void QWaylandTabletToolV2::zwp_tablet_tool_v2_done() void QWaylandTabletToolV2::zwp_tablet_tool_v2_removed() { - destroy(); m_tabletSeat->toolRemoved(this); } @@ -598,6 +598,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); @@ -617,7 +622,6 @@ void QWaylandTabletPadV2::zwp_tablet_pad_v2_done() void QWaylandTabletPadV2::zwp_tablet_pad_v2_removed() { - destroy(); delete this; } diff --git a/src/plugins/platforms/wayland/qwaylandtabletv2_p.h b/src/plugins/platforms/wayland/qwaylandtabletv2_p.h index 94b687ee3ea..f0d7cd189bb 100644 --- a/src/plugins/platforms/wayland/qwaylandtabletv2_p.h +++ b/src/plugins/platforms/wayland/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/wayland/tabletv2/tst_tabletv2.cpp b/tests/auto/wayland/tabletv2/tst_tabletv2.cpp index 85df099f90c..d5c2ccb3665 100644 --- a/tests/auto/wayland/tabletv2/tst_tabletv2.cpp +++ b/tests/auto/wayland/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;