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.

3f4e733468 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 826b9742e9. 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

@@ -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;

shell_client.h

@@ -300,6 +300,8 @@ private:
 
     bool m_compositingSetup = false;
     bool m_isInitialized = false;
+
+    friend class Workspace;
 };
 
 }

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);