From 89a4c2f0e1a05e4d9aae64367a2a50fa59e4c316 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Tue, 28 Jun 2016 18:57:08 +0200
Subject: [PATCH] Properly handle destroying a Pointer resource

Summary:
On client side the newer wl_pointer_release is used which is a
destructor call. On server side the shared destroy callback is used
and it's ensured that KWayland doesn't crash if called into the
PointerInterface between unbound and destroyed.

Test Plan:
Test case extended to cover the condition of an unbound
PointerInterface.

Reviewers: #plasma_on_wayland

Subscribers: plasma-devel

Tags: #plasma_on_wayland

Differential Revision: https://phabricator.kde.org/D2037
---
 .../autotests/client/test_wayland_seat.cpp    | 47 ++++++++++++++++++-
 src/wayland/pointer_interface.cpp             |  8 +---
 src/wayland/pointer_interface_p.h             |  2 -
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/src/wayland/autotests/client/test_wayland_seat.cpp b/src/wayland/autotests/client/test_wayland_seat.cpp
index d8e6722d78..cdae94456d 100644
--- a/src/wayland/autotests/client/test_wayland_seat.cpp
+++ b/src/wayland/autotests/client/test_wayland_seat.cpp
@@ -466,11 +466,56 @@ void TestWaylandSeat::testPointer()
     QCOMPARE(p->enteredSurface(), s);
     QCOMPARE(cp.enteredSurface(), s);
 
+    // destroy the focused pointer
+    QSignalSpy unboundSpy(serverPointer, &Resource::unbound);
+    QVERIFY(unboundSpy.isValid());
+    QSignalSpy destroyedSpy(serverPointer, &Resource::destroyed);
+    QVERIFY(destroyedSpy.isValid());
+    delete p;
+    QVERIFY(unboundSpy.wait());
+    QCOMPARE(unboundSpy.count(), 1);
+    QCOMPARE(destroyedSpy.count(), 0);
+    // now test that calling into the methods in Seat does not crash
+    QCOMPARE(m_seatInterface->focusedPointer(), serverPointer);
+    QCOMPARE(m_seatInterface->focusedPointerSurface(), serverSurface);
+    m_seatInterface->setTimestamp(8);
+    m_seatInterface->setPointerPos(QPoint(10, 15));
+    m_seatInterface->setTimestamp(9);
+    m_seatInterface->pointerButtonPressed(1);
+    m_seatInterface->setTimestamp(10);
+    m_seatInterface->pointerButtonReleased(1);
+    m_seatInterface->setTimestamp(11);
+    m_seatInterface->pointerAxis(Qt::Horizontal, 10);
+    m_seatInterface->setTimestamp(12);
+    m_seatInterface->pointerAxis(Qt::Vertical, 20);
+    m_seatInterface->setFocusedPointerSurface(nullptr);
+    QCOMPARE(focusedPointerChangedSpy.count(), 7);
+    m_seatInterface->setFocusedPointerSurface(serverSurface);
+    QCOMPARE(focusedPointerChangedSpy.count(), 8);
+    QCOMPARE(m_seatInterface->focusedPointerSurface(), serverSurface);
+    QVERIFY(!m_seatInterface->focusedPointer());
+
+    // and now destroy
+    QVERIFY(destroyedSpy.wait());
+    QCOMPARE(unboundSpy.count(), 1);
+    QCOMPARE(destroyedSpy.count(), 1);
+    QCOMPARE(m_seatInterface->focusedPointerSurface(), serverSurface);
+    QVERIFY(!m_seatInterface->focusedPointer());
+
+    // create a pointer again
+    p = m_seat->createPointer(m_seat);
+    QVERIFY(focusedPointerChangedSpy.wait());
+    QCOMPARE(focusedPointerChangedSpy.count(), 9);
+    QCOMPARE(m_seatInterface->focusedPointerSurface(), serverSurface);
+    serverPointer = m_seatInterface->focusedPointer();
+    QVERIFY(serverPointer);
+
     delete s;
     wl_display_flush(m_connection->display());
     QVERIFY(focusedPointerChangedSpy.wait());
-    QCOMPARE(focusedPointerChangedSpy.count(), 7);
+    QCOMPARE(focusedPointerChangedSpy.count(), 10);
     QVERIFY(!m_seatInterface->focusedPointerSurface());
+    QVERIFY(!m_seatInterface->focusedPointer());
 }
 
 void TestWaylandSeat::testPointerTransformation_data()
diff --git a/src/wayland/pointer_interface.cpp b/src/wayland/pointer_interface.cpp
index 8a689c2f70..27f97f52e4 100644
--- a/src/wayland/pointer_interface.cpp
+++ b/src/wayland/pointer_interface.cpp
@@ -100,7 +100,7 @@ void PointerInterface::Private::sendEnter(SurfaceInterface *surface, const QPoin
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
 const struct wl_pointer_interface PointerInterface::Private::s_interface = {
     setCursorCallback,
-    releaseCallback
+    resourceDestroyedCallback
 };
 #endif
 
@@ -205,12 +205,6 @@ void PointerInterface::Private::setCursorCallback(wl_client *client, wl_resource
     p->setCursor(serial, SurfaceInterface::get(surface), QPoint(hotspot_x, hotspot_y));
 }
 
-void PointerInterface::Private::releaseCallback(wl_client *client, wl_resource *resource)
-{
-    Q_UNUSED(client)
-    unbind(resource);
-}
-
 Cursor *PointerInterface::cursor() const
 {
     Q_D();
diff --git a/src/wayland/pointer_interface_p.h b/src/wayland/pointer_interface_p.h
index a44a03134d..bbfd7828be 100644
--- a/src/wayland/pointer_interface_p.h
+++ b/src/wayland/pointer_interface_p.h
@@ -51,8 +51,6 @@ private:
     // interface
     static void setCursorCallback(wl_client *client, wl_resource *resource, uint32_t serial,
                                   wl_resource *surface, int32_t hotspot_x, int32_t hotspot_y);
-    // since version 3
-    static void releaseCallback(wl_client *client, wl_resource *resource);
 
     static const struct wl_pointer_interface s_interface;
 };