From 45c93c6cae1f9b5ea487046b86487e067a50f392 Mon Sep 17 00:00:00 2001
From: Vlad Zagorodniy <vladzzag@gmail.com>
Date: Tue, 10 Sep 2019 10:40:36 +0300
Subject: [PATCH] Destroy ShellClient before Workspace is gone

Summary:
Managing lifetime of objects during tear down is a bit clunky in KWin
mostly because the wayland server outlives the workspace.

3f4e7334684 tried to tackle one aspect of this problem, but the proposed
solution is good only in short term. If a ShellClient wants to discard
force temporarily rules, it needs to access RuleBook, whose lifetime is
bounded to the workspace, no matter what happens. Otherwise, the force
temporarily rule will be applied again on the next startup.

It's worth to mention that there was another attempt to address this
problem, see commit 826b9742e95. It was reverted because some internal
clients may need to destroy Wayland resources during tear down.

This change takes another approach. In order to ensure that ShellClient
can access RuleBook during tear down, we manually destroy Wayland clients
in destructor of the Workspace class. Something is done already for X11
clients.

Reviewers: #kwin

Subscribers: romangg, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D22986
---
 shell_client.cpp | 37 +++++++++++++++++--------------------
 shell_client.h   |  2 ++
 workspace.cpp    | 13 +++++++++++++
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/shell_client.cpp b/shell_client.cpp
index ebd92b208c..391430e1ce 100644
--- a/shell_client.cpp
+++ b/shell_client.cpp
@@ -423,11 +423,10 @@ void ShellClient::destroyClient()
     if (isMoveResize()) {
         leaveMoveResize();
     }
-    Deleted *del = nullptr;
-    if (workspace()) {
-        del = Deleted::create(this);
-    }
-    emit windowClosed(this, del);
+
+    // Replace ShellClient with an instance of Deleted in the stacking order.
+    Deleted *deleted = Deleted::create(this);
+    emit windowClosed(this, deleted);
 
     // Remove Force Temporarily rules.
     RuleBook::self()->discardUsed(this, true);
@@ -435,25 +434,23 @@ void ShellClient::destroyClient()
     destroyWindowManagementInterface();
     destroyDecoration();
 
-    if (workspace()) {
-        StackingUpdatesBlocker blocker(workspace());
-        if (transientFor()) {
-            transientFor()->removeTransient(this);
-        }
-        for (auto it = transients().constBegin(); it != transients().constEnd();) {
-            if ((*it)->transientFor() == this) {
-                removeTransient(*it);
-                it = transients().constBegin(); // restart, just in case something more has changed with the list
-            } else {
-                ++it;
-            }
+    StackingUpdatesBlocker blocker(workspace());
+    if (transientFor()) {
+        transientFor()->removeTransient(this);
+    }
+    for (auto it = transients().constBegin(); it != transients().constEnd();) {
+        if ((*it)->transientFor() == this) {
+            removeTransient(*it);
+            it = transients().constBegin(); // restart, just in case something more has changed with the list
+        } else {
+            ++it;
         }
     }
+
     waylandServer()->removeClient(this);
 
-    if (del) {
-        del->unrefWindow();
-    }
+    deleted->unrefWindow();
+
     m_shellSurface = nullptr;
     m_xdgShellSurface = nullptr;
     m_xdgShellPopup = nullptr;
diff --git a/shell_client.h b/shell_client.h
index f25625bc1e..920ef2b8b5 100644
--- a/shell_client.h
+++ b/shell_client.h
@@ -300,6 +300,8 @@ private:
 
     bool m_compositingSetup = false;
     bool m_isInitialized = false;
+
+    friend class Workspace;
 };
 
 }
diff --git a/workspace.cpp b/workspace.cpp
index 59367a5f95..ec5c8006bb 100644
--- a/workspace.cpp
+++ b/workspace.cpp
@@ -556,6 +556,19 @@ Workspace::~Workspace()
         desktops.removeAll(c);
     }
     Client::cleanupX11();
+
+    if (waylandServer()) {
+        const QList<ShellClient *> shellClients = waylandServer()->clients();
+        for (ShellClient *shellClient : shellClients) {
+            shellClient->destroyClient();
+        }
+
+        const QList<ShellClient *> internalClients = waylandServer()->internalClients();
+        for (ShellClient *internalClient : internalClients) {
+            internalClient->destroyClient();
+        }
+    }
+
     for (UnmanagedList::iterator it = unmanaged.begin(), end = unmanaged.end(); it != end; ++it)
         (*it)->release(ReleaseReason::KWinShutsDown);