Don't crash when highlighted tabbox client is closed

Summary:
The compositor tries to switch to the next tabbox client when currently
highlighted client is closed. Though there is a small issue with that.
Because the switch happens too late, a dangling pointer can be inserted
into the unconstrained stacking order, which can lead to a crash later on.

There are two cases:
- compositing is on;
- compositing is off.

Compositing is on: TabBox will try to un-elevate currently highlighted
client, though by that time the client no longer owns EffectWindow, so
this is basically a no-op (that's why we haven't experienced this bug
before).

Compositing is off: TabBox will try to restack currently hightlighted
client under the next tabbox client. Given that the restack method
doesn't do any sanity checks(see Client::manage why), a client that is
about to be destroyed will be re-inserted back into the unconstrained
stacking order.

This change ensures that the switch happens before currently highlighted
client is removed from the stacking order.

BUG: 406784

Test Plan:
- Turn off compositing;
- Follow steps to reproduce in the bug report (see comment 2).

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D20916

client.cpp

@@ -32,6 +32,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "focuschain.h"
 #include "group.h"
 #include "shadow.h"
+#ifdef KWIN_BUILD_TABBOX
+#include "tabbox.h"
+#endif
 #include "workspace.h"
 #include "screenedge.h"
 #include "decorations/decorationbridge.h"
@@ -217,6 +220,12 @@ void Client::releaseWindow(bool on_shutdown)
 {
     assert(!deleting);
     deleting = true;
+#ifdef KWIN_BUILD_TABBOX
+    TabBox::TabBox *tabBox = TabBox::TabBox::self();
+    if (tabBox->isDisplayed() && tabBox->currentClient() == this) {
+        tabBox->nextPrev(true);
+    }
+#endif
     destroyWindowManagementInterface();
     Deleted* del = NULL;
     if (!on_shutdown) {
@@ -287,6 +296,12 @@ void Client::destroyClient()
 {
     assert(!deleting);
     deleting = true;
+#ifdef KWIN_BUILD_TABBOX
+    TabBox::TabBox *tabBox = TabBox::TabBox::self();
+    if (tabBox->isDisplayed() && tabBox->currentClient() == this) {
+        tabBox->nextPrev(true);
+    }
+#endif
     destroyWindowManagementInterface();
     Deleted* del = Deleted::create(this);
     if (isMoveResize())

workspace.cpp

@@ -679,12 +679,6 @@ void Workspace::removeClient(Client* c)
         clientShortcutUpdated(c);   // Needed, since this is otherwise delayed by setShortcut() and wouldn't run
     }
 
-#ifdef KWIN_BUILD_TABBOX
-    TabBox::TabBox *tabBox = TabBox::TabBox::self();
-    if (tabBox->isDisplayed() && tabBox->currentClient() == c)
-        tabBox->nextPrev(true);
-#endif
-
     Q_ASSERT(clients.contains(c) || desktops.contains(c));
     // TODO: if marked client is removed, notify the marked list
     clients.removeAll(c);
@@ -710,6 +704,7 @@ void Workspace::removeClient(Client* c)
     updateStackingOrder(true);
 
 #ifdef KWIN_BUILD_TABBOX
+    TabBox::TabBox *tabBox = TabBox::TabBox::self();
     if (tabBox->isDisplayed())
         tabBox->reset(true);
 #endif