From 7882407bb6bab6c042fd20592a63a875b74eb581 Mon Sep 17 00:00:00 2001
From: Giulio Camuffo <giulio.camuffo@jollamobile.com>
Date: Thu, 23 Oct 2014 18:37:16 +0300
Subject: [PATCH] Be more careful when destroying wl_resources

The compositor must not destroy wl_resources when it wants to, else it
breaks the contract with the client.

Change-Id: Ic0d298072cdf0954d2504c04bff2bcc99733e621
Reviewed-by: Laszlo Agocs <laszlo.agocs@digia.com>
---
 .../compositor_api/qwaylandsurface.cpp         |  9 +--------
 .../compositor_api/qwaylandsurface.h           |  1 -
 .../wayland_wrapper/qwlextendedsurface.cpp     | 18 +++++++++++++-----
 .../wayland_wrapper/qwlextendedsurface_p.h     |  1 +
 .../wayland_wrapper/qwlshellsurface.cpp        | 14 +-------------
 .../wayland_wrapper/qwlshellsurface_p.h        |  1 -
 src/compositor/wayland_wrapper/qwlsurface.cpp  |  3 +--
 7 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/src/compositor/compositor_api/qwaylandsurface.cpp b/src/compositor/compositor_api/qwaylandsurface.cpp
index a4f4e1fdd..205cea7a3 100644
--- a/src/compositor/compositor_api/qwaylandsurface.cpp
+++ b/src/compositor/compositor_api/qwaylandsurface.cpp
@@ -330,14 +330,7 @@ void QWaylandSurface::destroySurface()
 {
     QWaylandSurfaceOp op(QWaylandSurfaceOp::Close);
     if (!sendInterfaceOp(op))
-        destroySurfaceByForce();
-}
-
-void QWaylandSurface::destroySurfaceByForce()
-{
-    Q_D(QWaylandSurface);
-   wl_resource *surface_resource = d->resource()->handle;
-   wl_resource_destroy(surface_resource);
+        emit surfaceDestroyed();
 }
 
 /*!
diff --git a/src/compositor/compositor_api/qwaylandsurface.h b/src/compositor/compositor_api/qwaylandsurface.h
index 77e871365..7c7ec4d76 100644
--- a/src/compositor/compositor_api/qwaylandsurface.h
+++ b/src/compositor/compositor_api/qwaylandsurface.h
@@ -170,7 +170,6 @@ public:
 
     Q_INVOKABLE void destroy();
     Q_INVOKABLE void destroySurface();
-    Q_INVOKABLE void destroySurfaceByForce();
     Q_INVOKABLE void ping();
 
     void ref();
diff --git a/src/compositor/wayland_wrapper/qwlextendedsurface.cpp b/src/compositor/wayland_wrapper/qwlextendedsurface.cpp
index ee68ed196..55878295d 100644
--- a/src/compositor/wayland_wrapper/qwlextendedsurface.cpp
+++ b/src/compositor/wayland_wrapper/qwlextendedsurface.cpp
@@ -72,7 +72,8 @@ ExtendedSurface::ExtendedSurface(struct wl_client *client, uint32_t id, int vers
 
 ExtendedSurface::~ExtendedSurface()
 {
-    m_surface->setExtendedSurface(0);
+    if (m_surface)
+        m_surface->setExtendedSurface(0);
 }
 
 void ExtendedSurface::sendGenericProperty(const QString &name, const QVariant &variant)
@@ -91,6 +92,11 @@ void ExtendedSurface::setVisibility(QWindow::Visibility visibility, bool updateC
         send_onscreen_visibility(visibility);
 }
 
+void ExtendedSurface::setParentSurface(Surface *surface)
+{
+    m_surface = surface;
+}
+
 bool ExtendedSurface::runOperation(QWaylandSurfaceOp *op)
 {
     switch (op->type()) {
@@ -141,7 +147,7 @@ void ExtendedSurface::extended_surface_set_content_orientation_mask(Resource *re
     Qt::ScreenOrientations oldMask = m_contentOrientationMask;
     m_contentOrientationMask = mask;
 
-    if (mask != oldMask)
+    if (m_surface && mask != oldMask)
         emit m_surface->waylandSurface()->orientationUpdateMaskChanged();
 }
 
@@ -168,7 +174,7 @@ void ExtendedSurface::extended_surface_set_window_flags(Resource *resource, int3
 {
     Q_UNUSED(resource);
     QWaylandSurface::WindowFlags windowFlags(flags);
-    if (windowFlags== m_windowFlags)
+    if (m_surface || windowFlags == m_windowFlags)
         return;
     m_windowFlags = windowFlags;
     emit m_surface->waylandSurface()->windowFlagsChanged(windowFlags);
@@ -181,12 +187,14 @@ void ExtendedSurface::extended_surface_destroy_resource(Resource *)
 
 void ExtendedSurface::extended_surface_raise(Resource *)
 {
-    emit m_surface->waylandSurface()->raiseRequested();
+    if (m_surface)
+        emit m_surface->waylandSurface()->raiseRequested();
 }
 
 void ExtendedSurface::extended_surface_lower(Resource *)
 {
-    emit m_surface->waylandSurface()->lowerRequested();
+    if (m_surface)
+        emit m_surface->waylandSurface()->lowerRequested();
 }
 
 }
diff --git a/src/compositor/wayland_wrapper/qwlextendedsurface_p.h b/src/compositor/wayland_wrapper/qwlextendedsurface_p.h
index c64456666..9bcb28272 100644
--- a/src/compositor/wayland_wrapper/qwlextendedsurface_p.h
+++ b/src/compositor/wayland_wrapper/qwlextendedsurface_p.h
@@ -87,6 +87,7 @@ public:
     ExtendedSurface *parent() const;
     void setParent(ExtendedSurface *parent);
     QLinkedList<QWaylandSurface *> subSurfaces() const;
+    void setParentSurface(Surface *s);
 
     Qt::ScreenOrientations contentOrientationMask() const;
 
diff --git a/src/compositor/wayland_wrapper/qwlshellsurface.cpp b/src/compositor/wayland_wrapper/qwlshellsurface.cpp
index 97380feba..80304f3cb 100644
--- a/src/compositor/wayland_wrapper/qwlshellsurface.cpp
+++ b/src/compositor/wayland_wrapper/qwlshellsurface.cpp
@@ -91,7 +91,6 @@ ShellSurface::ShellSurface(Shell *shell, wl_client *client, uint32_t id, Surface
     , wl_shell_surface(client, id, 1)
     , m_shell(shell)
     , m_surface(surface)
-    , m_deleting(false)
     , m_resizeGrabber(0)
     , m_moveGrabber(0)
     , m_popupGrabber(0)
@@ -104,13 +103,6 @@ ShellSurface::ShellSurface(Shell *shell, wl_client *client, uint32_t id, Surface
 
 ShellSurface::~ShellSurface()
 {
-    // We must destroy the wl_resource here, but be careful not to do it
-    // if we're here from shell_surface_destroy_resource(), i.e. if the
-    // wl_resource was destroyed already
-    if (!m_deleting) {
-        m_deleting = true;
-        wl_resource_destroy(resource()->handle);
-    }
 }
 
 void ShellSurface::sendConfigure(uint32_t edges, int32_t width, int32_t height)
@@ -200,11 +192,7 @@ void ShellSurface::shell_surface_destroy_resource(Resource *)
     if (m_popupGrabber)
         m_popupGrabber->removePopup(this);
 
-    // If we're here from the destructor don't delete this again
-    if (!m_deleting) {
-        m_deleting = true;
-        delete this;
-    }
+    delete this;
 }
 
 void ShellSurface::shell_surface_move(Resource *resource,
diff --git a/src/compositor/wayland_wrapper/qwlshellsurface_p.h b/src/compositor/wayland_wrapper/qwlshellsurface_p.h
index f724ef290..92405bbc9 100644
--- a/src/compositor/wayland_wrapper/qwlshellsurface_p.h
+++ b/src/compositor/wayland_wrapper/qwlshellsurface_p.h
@@ -112,7 +112,6 @@ private:
     Shell *m_shell;
     Surface *m_surface;
     QWaylandSurfaceView *m_view;
-    bool m_deleting;
 
     ShellSurfaceResizeGrabber *m_resizeGrabber;
     ShellSurfaceMoveGrabber *m_moveGrabber;
diff --git a/src/compositor/wayland_wrapper/qwlsurface.cpp b/src/compositor/wayland_wrapper/qwlsurface.cpp
index e63fcff25..5cd8bb506 100644
--- a/src/compositor/wayland_wrapper/qwlsurface.cpp
+++ b/src/compositor/wayland_wrapper/qwlsurface.cpp
@@ -338,8 +338,7 @@ Qt::ScreenOrientation Surface::contentOrientation() const
 void Surface::surface_destroy_resource(Resource *)
 {
     if (m_extendedSurface) {
-        if (m_extendedSurface->resource())
-            wl_resource_destroy(m_extendedSurface->resource()->handle);
+        m_extendedSurface->setParentSurface(Q_NULLPTR);
         m_extendedSurface = 0;
     }
 
-- 
GitLab