Add Instance::unregisterSurface() in the destroy funct of the create …

…surface

... method of the `struct wl_compositor_interface`

This commits prevents crashes by accessing to already deleted Surfaces
destroyed by a premature wl_client_destroy method call. For example,
trying to load a non-allowed URI filtered by a Content-Filter policy.

Changes:

* The Instance class implements an Instance::unregisterSurface().

* The destroy callback in the wl_resource_set_implementation for the
  created Surface now includes a Instance::unregisterSurface(). For this
  removes the surface from the ViewBackendMap avoiding dispatch operations
  over a already destroyed Surface.

* The Instance::dispatchFrameCallbacks(uint32_t bridgeId) still checks if the
  bridgeId has an associated Surface in the ViewBackendMap but now just
  logs a warning message and avoids to fail when associated element is not
  found. Instead of that the logic now just skips the dispatchFrameCallbacks
  if the surface is there.

Co-authored-by: Adrian Perez de Castro <aperez@igalia.com>
Acked-by: Adrian Perez de Castro <aperez@igalia.com>
Signed-off-by: Pablo Saavedra <psaavedra@igalia.com>

src/ws.cpp

@@ -28,6 +28,7 @@
 #include "wpe-audio-server-protocol.h"
 #include "wpe-bridge-server-protocol.h"
 #include "wpe-video-plane-display-dmabuf-server-protocol.h"
+#include <algorithm>
 #include <cassert>
 #include <sys/socket.h>
 #include <unistd.h>
@@ -153,6 +154,7 @@ static const struct wl_compositor_interface s_compositorInterface = {
             [](struct wl_resource* resource)
             {
                 auto* surface = static_cast<Surface*>(wl_resource_get_user_data(resource));
+                WS::Instance::singleton().unregisterSurface(surface);
                 delete surface;
             });
     },
@@ -537,15 +539,27 @@ void Instance::unregisterViewBackend(uint32_t bridgeId)
     }
 }
 
+void Instance::unregisterSurface(Surface* surface)
+{
+    auto it = std::find_if(m_viewBackendMap.begin(), m_viewBackendMap.end(),
+        [surface](const std::pair<uint32_t, Surface*>& value) -> bool {
+            return value.second == surface;
+        });
+    if (it != m_viewBackendMap.end())
+        m_viewBackendMap.erase(it);
+}
+
 void Instance::dispatchFrameCallbacks(uint32_t bridgeId)
 {
     auto it = m_viewBackendMap.find(bridgeId);
     if (it == m_viewBackendMap.end()) {
-        g_error("Instance::dispatchFrameCallbacks(): "
-                "Cannot find surface with bridgeId %" PRIu32 " in view backend map.", bridgeId);
+        g_warning("Instance::dispatchFrameCallbacks(): "
+                  "Cannot find surface with bridgeId %" PRIu32 " in view "
+                  "backend map. Probably the associated surface is gone "
+                  "due to a premature exit in the client side", bridgeId);
+    } else {
+        it->second->dispatchFrameCallbacks();
     }
-
-    it->second->dispatchFrameCallbacks();
 }
 
 } // namespace WS

src/ws.h

@@ -136,6 +136,7 @@ class Instance {
     int createClient();
 
     void registerSurface(uint32_t, Surface*);
+    void unregisterSurface(Surface*);
     void registerViewBackend(uint32_t, APIClient&);
     void unregisterViewBackend(uint32_t);
     void dispatchFrameCallbacks(uint32_t);