From 1f48daea1ea469e19cca8e13b7f49f46bfa8b44c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Mon, 30 May 2016 11:21:12 +0200
Subject: [PATCH] [server] Don't destroy ShadowInterface when SurfaceInterface
 gets destroyed

Summary:
Unlike the other cases this one is not as dangerous as the shadow
protocol doesn't have a destructor request (yet).

Once that is added the problem would be the same: destroying the
ShadowInterface when the parent SurfaceInterface gets destroyed would
result in a protocol error on client side.

Reviewers: #plasma

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D1711
---
 src/wayland/autotests/client/test_shadow.cpp | 47 ++++++++++++++++++++
 src/wayland/shadow_interface.cpp             |  8 ----
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/src/wayland/autotests/client/test_shadow.cpp b/src/wayland/autotests/client/test_shadow.cpp
index ae2ec153e8..bab5938817 100644
--- a/src/wayland/autotests/client/test_shadow.cpp
+++ b/src/wayland/autotests/client/test_shadow.cpp
@@ -45,6 +45,7 @@ private Q_SLOTS:
 
     void testCreateShadow();
     void testShadowElements();
+    void testSurfaceDestroy();
 
 private:
     Display *m_display = nullptr;
@@ -266,5 +267,51 @@ void ShadowTest::testShadowElements()
     QVERIFY(!serverShadow->left());
 }
 
+void ShadowTest::testSurfaceDestroy()
+{
+    using namespace KWayland::Client;
+    using namespace KWayland::Server;
+    QSignalSpy serverSurfaceCreated(m_compositorInterface, &CompositorInterface::surfaceCreated);
+    QVERIFY(serverSurfaceCreated.isValid());
+
+    QScopedPointer<KWayland::Client::Surface> surface(m_compositor->createSurface());
+    QVERIFY(serverSurfaceCreated.wait());
+    auto serverSurface = serverSurfaceCreated.first().first().value<SurfaceInterface*>();
+    QSignalSpy shadowChangedSpy(serverSurface, &SurfaceInterface::shadowChanged);
+    QVERIFY(shadowChangedSpy.isValid());
+
+    QScopedPointer<Shadow> shadow(m_shadow->createShadow(surface.data()));
+    shadow->commit();
+    surface->commit(Surface::CommitFlag::None);
+    QVERIFY(shadowChangedSpy.wait());
+    auto serverShadow = serverSurface->shadow();
+    QVERIFY(serverShadow);
+
+    // destroy the parent surface
+    QSignalSpy surfaceDestroyedSpy(serverSurface, &QObject::destroyed);
+    QVERIFY(surfaceDestroyedSpy.isValid());
+    QSignalSpy shadowDestroyedSpy(serverShadow, &QObject::destroyed);
+    QVERIFY(shadowDestroyedSpy.isValid());
+    QSignalSpy clientDisconnectedSpy(serverSurface->client(), &ClientConnection::disconnected);
+    QVERIFY(clientDisconnectedSpy.isValid());
+    surface.reset();
+    QVERIFY(surfaceDestroyedSpy.wait());
+    QVERIFY(shadowDestroyedSpy.isEmpty());
+    // destroy the shadow
+    shadow.reset();
+    // shadow protocol doesn't have a destroy callback yet, so also disconnect
+    m_connection->deleteLater();
+    m_connection = nullptr;
+    QVERIFY(clientDisconnectedSpy.wait());
+    if (shadowDestroyedSpy.isEmpty()) {
+        QVERIFY(shadowDestroyedSpy.wait());
+    }
+    QCOMPARE(shadowDestroyedSpy.count(), 1);
+    m_shm->destroy();
+    m_compositor->destroy();
+    m_shadow->destroy();
+    m_queue->destroy();
+}
+
 QTEST_GUILESS_MAIN(ShadowTest)
 #include "test_shadow.moc"
diff --git a/src/wayland/shadow_interface.cpp b/src/wayland/shadow_interface.cpp
index 75d5333982..965a86ba93 100644
--- a/src/wayland/shadow_interface.cpp
+++ b/src/wayland/shadow_interface.cpp
@@ -105,14 +105,6 @@ void ShadowManagerInterface::Private::createShadow(wl_client *client, wl_resourc
         delete shadow;
         return;
     }
-    QObject::connect(s, &QObject::destroyed, shadow,
-        [shadow] {
-            if (shadow->resource()) {
-                wl_resource_destroy(shadow->resource());
-                delete shadow;
-            }
-        }
-    );
     s->d_func()->setShadow(QPointer<ShadowInterface>(shadow));
 }