[x11] Fix crash during tear down

Summary:
Any call made to a virtual method in constructor/destructor of a base
class won't go to a derived class because the base class may access
uninitialized or destroyed resources.

For example, let's consider the following two classes

    class Base {
    public:
        Base() { foo()->bar(); }
        virtual ~Base() { foo()->bar(); }

        virtual Foo* foo() const { return nullptr; }
    };

    class Derived : public Base {
    public:
        Derived() : mFoo(new Foo) {}
        ~Derived() override { delete mFoo; }

        Foo* foo() const override { return mFoo; }

    private:
        Foo* mFoo;
    };

When an instance of Derived class is created, constructors will run in
the following order:

    Base()
    Derived()

It's not safe to dispatch foo() method call to Derived class because
constructor of Derived hasn't initialized yet mFoo.

Same story with destructors, they'll run in the following order:

    ~Derived()
    ~Base()

It's not safe to dispatch foo() method call in the destructor of Base
class to Derived class because mFoo was deleted.

So, what does that weird C++ behavior has something to do with KWin? Well,
recently Compositor class was split into two classes - WaylandCompositor,
and X11Compositor. Some functionality from X11 doesn't make sense on
Wayland. Therefore methods that implement that stuff were "purified," i.e.
they became pure virtual methods. Unfortunately, when Compositor tears
down it may call pure virtual methods on itself. Given that those calls
cannot be dispatched to X11Compositor or WaylandCompositor, the only
choice that C++ runtime has is to throw an exception.

The fix for this very delicate problem is very simple - do not call virtual
methods from constructors and the destructor. Avoid doing that if you can!

This change moves Compositor::updateClientCompositeBlocking to X11Compositor
so it longer has to be a virtual method. Also, it kind of doesn't make sense
to keep it in base Compositor class because compositing can be blocked only
on X11.

BUG: 411049

Test Plan: KWin no longer crashes when running kwin_x11 --replace command.

Reviewers: #kwin, romangg

Reviewed By: #kwin, romangg

Subscribers: anthonyfieroni, kwin

Tags: #kwin

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

composite.cpp

@@ -890,17 +890,6 @@ int WaylandCompositor::refreshRate() const
     return KWin::currentRefreshRate();
 }
 
-void WaylandCompositor::updateCompositeBlocking()
-{
-    // Composite blocking not possible on Wayland.
-}
-
-void WaylandCompositor::updateClientCompositeBlocking(Client *c)
-{
-    Q_UNUSED(c)
-    // Composite blocking not possible on Wayland.
-}
-
 X11Compositor::X11Compositor(QObject *parent)
     : Compositor(parent)
     , m_suspended(options->isUseCompositing() ? NoReasonSuspend : UserSuspend)
@@ -1029,11 +1018,6 @@ int X11Compositor::refreshRate() const
     return m_xrrRefreshRate;
 }
 
-void X11Compositor::updateCompositeBlocking()
-{
-    updateClientCompositeBlocking(NULL);
-}
-
 void X11Compositor::updateClientCompositeBlocking(Client *c)
 {
     if (c) {
@@ -1063,6 +1047,11 @@ void X11Compositor::updateClientCompositeBlocking(Client *c)
     }
 }
 
+X11Compositor *X11Compositor::self()
+{
+    return qobject_cast<X11Compositor *>(Compositor::self());
+}
+
 }
 
 // included for CompositorSelectionOwner

composite.h

@@ -113,10 +113,6 @@ public:
         return s_compositor != NULL && s_compositor->isActive();
     }
 
-
-    virtual void updateCompositeBlocking() = 0;
-    virtual void updateClientCompositeBlocking(KWin::Client *c) = 0;
-
     // for delayed supportproperty management of effects
     void keepSupportProperty(xcb_atom_t atom);
     void removeSupportProperty(xcb_atom_t atom);
@@ -197,9 +193,6 @@ public:
 
     bool checkForOverlayWindow(WId w) const override;
 
-    void updateCompositeBlocking() override;
-    void updateClientCompositeBlocking(KWin::Client* c) override;
-
 protected:
     void start() override;
 
@@ -267,8 +260,9 @@ public:
 
     int refreshRate() const override;
 
-    void updateCompositeBlocking() override;
-    void updateClientCompositeBlocking(KWin::Client* c) override;
+    void updateClientCompositeBlocking(Client *client = nullptr);
+
+    static X11Compositor *self();
 
 protected:
     void start() override;

workspace.cpp

@@ -596,7 +596,9 @@ Client* Workspace::createClient(xcb_window_t w, bool is_mapped)
     StackingUpdatesBlocker blocker(this);
     Client* c = new Client();
     setupClientConnections(c);
-    connect(c, &Client::blockingCompositingChanged, m_compositor, &Compositor::updateClientCompositeBlocking);
+    if (X11Compositor *compositor = X11Compositor::self()) {
+        connect(c, &Client::blockingCompositingChanged, compositor, &X11Compositor::updateClientCompositeBlocking);
+    }
     connect(c, SIGNAL(clientFullScreenSet(KWin::Client*,bool,bool)), ScreenEdges::self(), SIGNAL(checkBlocking()));
     if (!c->manage(w, is_mapped)) {
         Client::deleteClient(c);
@@ -749,8 +751,11 @@ void Workspace::removeDeleted(Deleted* c)
     unconstrained_stacking_order.removeAll(c);
     stacking_order.removeAll(c);
     markXStackingOrderAsDirty();
-    if (c->wasClient() && m_compositor) {
-        m_compositor->updateCompositeBlocking();
+    if (!c->wasClient()) {
+        return;
+    }
+    if (X11Compositor *compositor = X11Compositor::self()) {
+        compositor->updateClientCompositeBlocking();
     }
 }