From 576c6671245c85dd1e3417992d9f99b53120b661 Mon Sep 17 00:00:00 2001
From: David Edmundson <kde@davidedmundson.co.uk>
Date: Thu, 6 Dec 2018 12:54:27 +0000
Subject: [PATCH] Avoid deleting ourselves midway through updateShadow

Summary:
If updating a shadow means that there should be no shadow, shadow ends
up deleting itself midway through the method.

It's cleaner and safer to leave that to the caller.

This new change matches the existing documentation for
Shadow::updateShadow which states

> In case the Property has been withdrawn the method returns @c false.
> In that case the owner should delete the Shadow.

Test Plan:
Created an ASAN of kwin
Relevant testMaximised no longer fails

Reviewers: #kwin, zzag

Reviewed By: #kwin, zzag

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D17380
---
 shadow.cpp   | 14 --------------
 toplevel.cpp |  5 ++++-
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/shadow.cpp b/shadow.cpp
index 562106cba2..2cbabb01bd 100644
--- a/shadow.cpp
+++ b/shadow.cpp
@@ -331,15 +331,6 @@ void Shadow::buildQuads()
 
 bool Shadow::updateShadow()
 {
-    auto clear = [this] {
-        if (m_topLevel && m_topLevel->shadow()) {
-            auto w = m_topLevel->effectWindow();
-            // this also deletes the shadow
-            w->sceneWindow()->updateShadow(nullptr);
-            emit m_topLevel->shadowChanged();
-        }
-    };
-
     if (!m_topLevel) {
         return false;
     }
@@ -348,12 +339,10 @@ bool Shadow::updateShadow()
         if (AbstractClient *c = qobject_cast<AbstractClient*>(m_topLevel)) {
             if (c->decoration()) {
                 if (init(c->decoration())) {
-                    emit m_topLevel->shadowChanged();
                     return true;
                 }
             }
         }
-        clear();
         return false;
     }
 
@@ -361,7 +350,6 @@ bool Shadow::updateShadow()
         if (m_topLevel && m_topLevel->surface()) {
             if (const auto &s = m_topLevel->surface()->shadow()) {
                 if (init(s)) {
-                    emit m_topLevel->shadowChanged();
                     return true;
                 }
             }
@@ -370,12 +358,10 @@ bool Shadow::updateShadow()
 
     auto data = Shadow::readX11ShadowProperty(m_topLevel->window());
     if (data.isEmpty()) {
-        clear();
         return false;
     }
 
     init(data);
-    emit m_topLevel->shadowChanged();
 
     return true;
 }
diff --git a/toplevel.cpp b/toplevel.cpp
index bff868f6fe..0b83e621f8 100644
--- a/toplevel.cpp
+++ b/toplevel.cpp
@@ -336,7 +336,10 @@ void Toplevel::getShadow()
     const QRect oldVisibleRect = visibleRect();
     if (hasShadow()) {
         dirtyRect = shadow()->shadowRegion().boundingRect();
-        effectWindow()->sceneWindow()->shadow()->updateShadow();
+        if (!effectWindow()->sceneWindow()->shadow()->updateShadow()) {
+            effectWindow()->sceneWindow()->updateShadow(nullptr);
+        }
+        emit shadowChanged();
     } else {
         Shadow::createShadow(this);
     }