view backend: Properly unregister surfaces

When a new surface is registered for an existing ViewBackend, unregister
the old one first, to avoid keeping stale registrations. This makes the
WS::Instance::m_viewBackendMap only contain one entry pointing for any
given ViewBackend instance, instead of growing continuosly.

To ensure that the same code path is taken when the wl_client for a
ViewBackend is destroyed, all the operations to be done are moved into
ViewBackend::clientDestroyNotify(), the callback of the wl_client
destruction listener, then ViewBackend::unregisterSurface() only needs
to call wl_client_destroy(). This way the ~ViewBackend destructor only
needs to call unregisterSurface() as well.

Also, avoid doing any operation in ViewBackend::dispatchFrameCallbacks()
and ViewBackend::releaseBuffer() if the client connection has been lost.

src/view-backend-private.cpp

@@ -36,14 +36,12 @@ ViewBackend::ViewBackend(ClientBundle* clientBundle, struct wpe_view_backend* ba
     m_clientBundle->viewBackend = this;
 
     wl_list_init(&m_frameCallbacks);
+    wl_list_init(&m_clientDestroy.link);
 }
 
 ViewBackend::~ViewBackend()
 {
-    clearFrameCallbacks();
-
     unregisterSurface(m_surfaceId);
-
     if (m_clientFd != -1)
         close(m_clientFd);
 }
@@ -105,44 +103,69 @@ void ViewBackend::exportEGLStreamProducer(struct wl_resource* bufferResource)
 
 void ViewBackend::dispatchFrameCallbacks()
 {
+    if (G_UNLIKELY(!m_client))
+        return;
+
     FrameCallbackResource* resource;
     wl_list_for_each(resource, &m_frameCallbacks, link) {
         wl_callback_send_done(resource->resource, 0);
     }
     clearFrameCallbacks();
 
-    if (m_client.object)
-        wl_client_flush(m_client.object);
+    wl_client_flush(m_client);
+
     wpe_view_backend_dispatch_frame_displayed(m_backend);
 }
 
 void ViewBackend::releaseBuffer(struct wl_resource* buffer_resource)
 {
+    if (G_UNLIKELY(!m_client))
+        return;
+
     wl_buffer_send_release(buffer_resource);
-    if (m_client.object)
-        wl_client_flush(m_client.object);
+    wl_client_flush(m_client);
+}
+
+void ViewBackend::clientDestroyNotify(struct wl_listener* listener, void*)
+{
+    ViewBackend* self = wl_container_of(listener, self, m_clientDestroy);
+
+    self->clearFrameCallbacks();
+    WS::Instance::singleton().unregisterViewBackend(self->m_surfaceId);
+    self->m_client = nullptr;
+    self->m_surfaceId = 0;
+
+    wl_list_remove(&self->m_clientDestroy.link);
 }
 
 void ViewBackend::registerSurface(uint32_t surfaceId)
 {
-    m_surfaceId = surfaceId;
-    m_client.object = WS::Instance::singleton().registerViewBackend(m_surfaceId, *this);
 
-    m_client.destroyListener.notify = Client::destroyNotify;
-    wl_client_add_destroy_listener(m_client.object, &m_client.destroyListener);
+    if (m_surfaceId == surfaceId)
+        return;
+
+    unregisterSurface(m_surfaceId);
+
+    m_surfaceId = surfaceId;
+    m_client = WS::Instance::singleton().registerViewBackend(m_surfaceId, *this);
+    wl_client_add_destroy_listener(m_client, &m_clientDestroy);
 }
 
 void ViewBackend::unregisterSurface(uint32_t surfaceId)
 {
     if (!surfaceId || m_surfaceId != surfaceId)
         return;
 
-    clearFrameCallbacks();
+    // If the surfaceId is valid, we cannot have an invalid wl_client.
+    g_assert(m_client != nullptr);
 
-    g_clear_pointer(&m_client.object, wl_client_destroy);
+    // Destroying the client triggers the m_clientDestroy callback,
+    // the rest of the teardown is done from there.
+    wl_client_destroy(m_client);
 
-    WS::Instance::singleton().unregisterViewBackend(m_surfaceId);
-    m_surfaceId = 0;
+    // After destroying the client, none of these can be valid.
+    g_assert(m_client == nullptr);
+    g_assert(m_surfaceId == 0);
 }
 
 void ViewBackend::didReceiveMessage(uint32_t messageId, uint32_t messageBody)
@@ -172,14 +195,6 @@ void ViewBackend::clearFrameCallbacks()
     wl_list_init(&m_frameCallbacks);
 }
 
-void ViewBackend::Client::destroyNotify(struct wl_listener* listener, void*)
-{
-    Client* client;
-    client = wl_container_of(listener, client, destroyListener);
-
-    client->object = nullptr;
-}
-
 void ViewBackend::FrameCallbackResource::destroyNotify(struct wl_listener* listener, void*)
 {
     FrameCallbackResource* resource;

src/view-backend-private.h

@@ -91,12 +91,10 @@ class ViewBackend final : public WS::APIClient, public FdoIPC::MessageReceiver {
     static gboolean s_socketCallback(GSocket*, GIOCondition, gpointer);
 
     uint32_t m_surfaceId { 0 };
-    struct Client {
-        struct wl_client* object { nullptr };
-        struct wl_listener destroyListener;
 
-        static void destroyNotify(struct wl_listener*, void*);
-    } m_client;
+    static void clientDestroyNotify(struct wl_listener*, void*);
+    struct wl_listener m_clientDestroy { {}, clientDestroyNotify };
+    struct wl_client* m_client { nullptr };
 
     ClientBundle* m_clientBundle;
     struct wpe_view_backend* m_backend;