From 6d529abf8e0484e9c560adf770d2ed01d0d967f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Thu, 24 Mar 2016 12:45:46 +0100
Subject: [PATCH] [server] Remove sub-surface from parent when it's surface is
 destroyed

Summary:
From spec:
If the wl_surface associated with the wl_subsurface is destroyed,
the wl_subsurface object becomes inert. Note, that destroying either
object takes effect immediately.

Reviewers: #plasma

Subscribers: plasma-devel

Projects: #plasma

Differential Revision: https://phabricator.kde.org/D1211
---
 .../client/test_wayland_subsurface.cpp        | 34 +++++++++++++++++++
 src/wayland/subcompositor_interface.cpp       | 12 +++++++
 src/wayland/surface_interface.cpp             |  8 +++--
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/src/wayland/autotests/client/test_wayland_subsurface.cpp b/src/wayland/autotests/client/test_wayland_subsurface.cpp
index 97a7641abc..7ddf1fd627 100644
--- a/src/wayland/autotests/client/test_wayland_subsurface.cpp
+++ b/src/wayland/autotests/client/test_wayland_subsurface.cpp
@@ -55,6 +55,7 @@ private Q_SLOTS:
     void testSyncMode();
     void testDeSyncMode();
     void testMainSurfaceFromTree();
+    void testRemoveSurface();
 
 private:
     KWayland::Server::Display *m_display;
@@ -748,5 +749,38 @@ void TestSubSurface::testMainSurfaceFromTree()
     QCOMPARE(child3->surface()->childSubSurfaces().count(), 0);
 }
 
+void TestSubSurface::testRemoveSurface()
+{
+    // this test verifies that removing the surface also removes the sub-surface from the parent
+    using namespace KWayland::Client;
+    using namespace KWayland::Server;
+
+    QSignalSpy surfaceCreatedSpy(m_compositorInterface, &CompositorInterface::surfaceCreated);
+    QVERIFY(surfaceCreatedSpy.isValid());
+
+    QScopedPointer<Surface> parentSurface(m_compositor->createSurface());
+    QVERIFY(surfaceCreatedSpy.wait());
+    auto parentServerSurface = surfaceCreatedSpy.last().first().value<SurfaceInterface*>();
+    QVERIFY(parentServerSurface);
+    QScopedPointer<Surface> childSurface(m_compositor->createSurface());
+    QVERIFY(surfaceCreatedSpy.wait());
+    auto childServerSurface = surfaceCreatedSpy.last().first().value<SurfaceInterface*>();
+    QVERIFY(childServerSurface);
+
+    QSignalSpy subSurfaceTreeChangedSpy(parentServerSurface, &SurfaceInterface::subSurfaceTreeChanged);
+    QVERIFY(subSurfaceTreeChangedSpy.isValid());
+
+    m_subCompositor->createSubSurface(childSurface.data(), parentSurface.data());
+    parentSurface->commit(Surface::CommitFlag::None);
+    QVERIFY(subSurfaceTreeChangedSpy.wait());
+
+    QCOMPARE(parentServerSurface->childSubSurfaces().count(), 1);
+
+    // destroy surface, takes place immediately
+    childSurface.reset();
+    QVERIFY(subSurfaceTreeChangedSpy.wait());
+    QCOMPARE(parentServerSurface->childSubSurfaces().count(), 0);
+}
+
 QTEST_GUILESS_MAIN(TestSubSurface)
 #include "test_wayland_subsurface.moc"
diff --git a/src/wayland/subcompositor_interface.cpp b/src/wayland/subcompositor_interface.cpp
index dbdce927da..a701e1687d 100644
--- a/src/wayland/subcompositor_interface.cpp
+++ b/src/wayland/subcompositor_interface.cpp
@@ -176,6 +176,18 @@ void SubSurfaceInterface::Private::create(ClientConnection *client, quint32 vers
     surface->d_func()->subSurfacePending.shadowIsSet = false;
     surface->d_func()->subSurfacePending.slideIsSet = false;
     parent->d_func()->addChild(QPointer<SubSurfaceInterface>(q));
+
+    QObject::connect(surface, &QObject::destroyed, q,
+        [this] {
+            // from spec: "If the wl_surface associated with the wl_subsurface is destroyed,
+            // the wl_subsurface object becomes inert. Note, that destroying either object
+            // takes effect immediately."
+            if (parent) {
+                Q_Q(SubSurfaceInterface);
+                reinterpret_cast<SurfaceInterface::Private*>(parent->d.data())->removeChild(QPointer<SubSurfaceInterface>(q));
+            }
+        }
+    );
 }
 
 void SubSurfaceInterface::Private::commit()
diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp
index 82d48469fe..e9b4ffff4c 100644
--- a/src/wayland/surface_interface.cpp
+++ b/src/wayland/surface_interface.cpp
@@ -68,9 +68,11 @@ void SurfaceInterface::Private::removeChild(QPointer< SubSurfaceInterface > chil
     Q_Q(SurfaceInterface);
     emit q->subSurfaceTreeChanged();
     QObject::disconnect(child.data(), &SubSurfaceInterface::positionChanged, q, &SurfaceInterface::subSurfaceTreeChanged);
-    QObject::disconnect(child->surface().data(), &SurfaceInterface::damaged, q, &SurfaceInterface::subSurfaceTreeChanged);
-    QObject::disconnect(child->surface().data(), &SurfaceInterface::unmapped, q, &SurfaceInterface::subSurfaceTreeChanged);
-    QObject::disconnect(child->surface().data(), &SurfaceInterface::subSurfaceTreeChanged, q, &SurfaceInterface::subSurfaceTreeChanged);
+    if (!child->surface().isNull()) {
+        QObject::disconnect(child->surface().data(), &SurfaceInterface::damaged, q, &SurfaceInterface::subSurfaceTreeChanged);
+        QObject::disconnect(child->surface().data(), &SurfaceInterface::unmapped, q, &SurfaceInterface::subSurfaceTreeChanged);
+        QObject::disconnect(child->surface().data(), &SurfaceInterface::subSurfaceTreeChanged, q, &SurfaceInterface::subSurfaceTreeChanged);
+    }
 }
 
 bool SurfaceInterface::Private::raiseChild(QPointer<SubSurfaceInterface> subsurface, SurfaceInterface *sibling)