[mutter/wip/wayland-work: 42/42] MetaWaylandSurface: fix attach(NULL) to mean unmap

[Giovanni Campagna <gcampagna src gnome org>]

commit 6f433bbd89acd73acfbedd5a71120bf467f13de3
Author: Giovanni Campagna <gcampagn redhat com>
Date:   Thu Sep 19 16:08:11 2013 +0200

    MetaWaylandSurface: fix attach(NULL) to mean unmap
    
    The wayland protocol describes a commit after attaching null as
    meaning unmap, so must unmanage the window and not crash.
    This is complicated by the fact the the flow for X11 and wayland
    clients is different, in that we want to listen for X events
    to map/unmap X clients.

 src/core/window.c                  |   10 +++-
 src/wayland/meta-wayland-surface.c |   98 ++++++++++++++++++++++--------------
 src/wayland/meta-wayland-surface.h |    1 +
 3 files changed, 70 insertions(+), 39 deletions(-)
---
diff --git a/src/core/window.c b/src/core/window.c
index a99650b..959e54d 100644
--- a/src/core/window.c
+++ b/src/core/window.c
@@ -2050,7 +2050,15 @@ meta_window_unmanage (MetaWindow  *window,
     meta_display_unregister_wayland_window (window->display, window);
 
   if (window->surface)
-    meta_wayland_surface_free (window->surface);
+    {
+      if (window->client_type == META_WINDOW_CLIENT_TYPE_X11)
+        meta_wayland_surface_free (window->surface);
+      else
+        {
+          window->surface->window = NULL;
+          window->surface = NULL;
+        }
+    }
 
   meta_prefs_remove_listener (prefs_changed_callback, window);
 
diff --git a/src/wayland/meta-wayland-surface.c b/src/wayland/meta-wayland-surface.c
index f77f9cb..27ed66f 100644
--- a/src/wayland/meta-wayland-surface.c
+++ b/src/wayland/meta-wayland-surface.c
@@ -249,37 +249,59 @@ meta_wayland_surface_commit (struct wl_client *client,
   if (surface->pending.newly_attached &&
       surface->buffer_ref.buffer != surface->pending.buffer)
     {
-      MetaWaylandBuffer *buffer = surface->pending.buffer;
-      CoglContext *ctx =
-        clutter_backend_get_cogl_context (clutter_get_default_backend ());
-      CoglError *catch_error = NULL;
-      CoglTexture *texture =
-        COGL_TEXTURE (cogl_wayland_texture_2d_new_from_buffer (ctx,
-                                                               buffer->resource,
-                                                               &catch_error));
-      if (!texture)
-        {
-          cogl_error_free (catch_error);
-         meta_warning ("Could not import pending buffer, ignoring commit\n");
-         return;
-        }
+      if (surface->pending.buffer)
+       {
+         MetaWaylandBuffer *buffer = surface->pending.buffer;
+         CoglContext *ctx =
+           clutter_backend_get_cogl_context (clutter_get_default_backend ());
+         CoglError *catch_error = NULL;
+         CoglTexture *texture =
+           COGL_TEXTURE (cogl_wayland_texture_2d_new_from_buffer (ctx,
+                                                                  buffer->resource,
+                                                                  &catch_error));
+         if (!texture)
+           {
+             cogl_error_free (catch_error);
+             meta_warning ("Could not import pending buffer, ignoring commit\n");
+             return;
+           }
+         else
+           {
+             buffer->texture = texture;
+             buffer->width = cogl_texture_get_width (texture);
+             buffer->height = cogl_texture_get_height (texture);
+           }
+
+         /* Note: we set this before informing any window-actor since the
+          * window actor will expect to find the new buffer within the
+          * surface. */
+         meta_wayland_buffer_reference (&surface->buffer_ref, buffer);
+       }
       else
-        {
-         buffer->texture = texture;
-          buffer->width = cogl_texture_get_width (texture);
-          buffer->height = cogl_texture_get_height (texture);
-        }
+       {
+         /* An attach without a buffer is equivalent to unmap, for wayland clients.
+            For X11 clients, instead we wait for the UnmapNotify, and just ignore
+            the commit (we also keep the previous buffer, which we use for the
+            unmap animation).
+
+            We also ignore the commit if we were never mapped in the first place.
+         */
+         if (surface->window && surface->window->client_type == META_WINDOW_CLIENT_TYPE_WAYLAND)
+           {
+             MetaDisplay *display = meta_get_display ();
+             guint32 timestamp = meta_display_get_current_time_roundtrip (display);
 
-      /* Note: we set this before informing any window-actor since the
-       * window actor will expect to find the new buffer within the
-       * surface. */
-      meta_wayland_buffer_reference (&surface->buffer_ref,
-                                     surface->pending.buffer);
-    }
+             meta_window_unmanage (surface->window, timestamp);
+             meta_wayland_buffer_reference (&surface->buffer_ref, NULL);
+           }
 
-  if (!surface->buffer_ref.buffer)
+         return;
+       }
+    }
+  else if (surface->buffer_ref.buffer == NULL)
     {
-      meta_warning ("Commit without a buffer? Ignoring\n");
+      /* A commit without attach while unmapped doesn't make sense, so warn */
+      meta_warning ("Spurious commit from client while unmapped\n");
       return;
     }
 
@@ -422,31 +444,31 @@ meta_wayland_surface_resource_destroy_cb (struct wl_resource *resource)
 {
   MetaWaylandSurface *surface = wl_resource_get_user_data (resource);
 
-  /* There are four cases here:
+  /* There are three cases here:
      - An X11 unmanaged window -> surface is NULL, nothing to do
      - An X11 unmanaged window, but we got the wayland event first ->
        just clear the resource pointer
-     - A wayland surface without window (destroyed before set_toplevel) ->
-       need to free the surface itself
-     - A wayland window -> need to unmanage
+     - A wayland surface -> need to free the surface and possibly
+       unmanage the window
+
+       NB: If the surface corresponds to an X window then we will be
+       sure to free the MetaWindow according to some X event (UnmapNotify
+       usually).
   */
 
   if (surface)
     {
       surface->resource = NULL;
 
-      /* NB: If the surface corresponds to an X window then we will be
-       * sure to free the MetaWindow according to some X event. */
-      if (surface->window &&
-         surface->window->client_type == META_WINDOW_CLIENT_TYPE_WAYLAND)
+      if (surface->client_type == META_WINDOW_CLIENT_TYPE_WAYLAND)
        {
          MetaDisplay *display = meta_get_display ();
          guint32 timestamp = meta_display_get_current_time_roundtrip (display);
 
-         meta_window_unmanage (surface->window, timestamp);
+         if (surface->window)
+           meta_window_unmanage (surface->window, timestamp);
+         meta_wayland_surface_free (surface);
        }
-      else if (!surface->window)
-       meta_wayland_surface_free (surface);
     }
 }
 
diff --git a/src/wayland/meta-wayland-surface.h b/src/wayland/meta-wayland-surface.h
index 1a1b3ad..05034d8 100644
--- a/src/wayland/meta-wayland-surface.h
+++ b/src/wayland/meta-wayland-surface.h
@@ -103,6 +103,7 @@ struct _MetaWaylandSurface
   MetaWaylandCompositor *compositor;
   MetaWaylandBufferReference buffer_ref;
   MetaWindow *window;
+  MetaWindowClientType client_type;
   MetaWaylandSurfaceExtension *shell_surface;
   MetaWaylandSurfaceExtension *gtk_surface;