[mutter/wayland] wayland: refactor window destruction and focus

[Giovanni Campagna <gcampagna src gnome org>]

commit 9a5f243f73f4f93cce5d69bd1d8fccd674ac1f77
Author: Giovanni Campagna <gcampagn redhat com>
Date:   Mon Aug 12 18:04:34 2013 +0200

    wayland: refactor window destruction and focus
    
    The previous code was leaving focus fields dirty in MetaWaylandPointer
    and MetaWaylandKeyboard at time (which could crash the X server
    because of invalid object IDs)
    The new code is more tighly integrated in the normal X11 code
    for handling keyboard focus (meaning that the core idea of input
    focus is also correct now), so that meta_window_unmanage() can
    do the right thing. As a side benefit, clicking on wayland clients
    now unfocus X11 clients.
    For the mouse focus, we need to clear the surface pointer when
    the metawindowactor is destroyed (even if the actual actor is
    kept alive for effects), so that a repick finds a different pointer
    focus.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=705859

 src/compositor/meta-window-actor.c |    7 ++-
 src/core/display.c                 |   37 ++++++++----
 src/core/window.c                  |    9 +++
 src/wayland/meta-wayland-pointer.c |    8 ++-
 src/wayland/meta-wayland-private.h |    2 +
 src/wayland/meta-wayland.c         |  110 ++++++++++++++++++++----------------
 6 files changed, 108 insertions(+), 65 deletions(-)
---
diff --git a/src/compositor/meta-window-actor.c b/src/compositor/meta-window-actor.c
index d738947..6949fcc 100644
--- a/src/compositor/meta-window-actor.c
+++ b/src/compositor/meta-window-actor.c
@@ -1320,6 +1320,11 @@ meta_window_actor_destroy (MetaWindowActor *self)
 
   priv = self->priv;
 
+#ifdef HAVE_WAYLAND
+  if (meta_is_wayland_compositor ())
+    meta_shaped_texture_set_wayland_surface (META_SHAPED_TEXTURE (priv->actor), NULL);
+#endif
+
   window = priv->window;
   window_type = meta_window_get_window_type (window);
   meta_window_set_compositor_private (window, NULL);
@@ -2466,7 +2471,7 @@ meta_window_actor_set_wayland_surface (MetaWindowActor *self,
 
   meta_shaped_texture_set_wayland_surface (META_SHAPED_TEXTURE (priv->actor),
                                            surface);
-  if (surface->buffer_ref.buffer)
+  if (surface && surface->buffer_ref.buffer)
     maybe_emit_size_changed (self, surface->buffer_ref.buffer);
 }
 
diff --git a/src/core/display.c b/src/core/display.c
index 5369368..d633af3 100644
--- a/src/core/display.c
+++ b/src/core/display.c
@@ -1902,9 +1902,14 @@ update_focus_window (MetaDisplay *display,
                      Window       xwindow,
                      gulong       serial)
 {
+#ifdef HAVE_WAYLAND
+  MetaWaylandCompositor *compositor;
+#endif
+
   display->focus_serial = serial;
 
-  if (display->focus_xwindow == xwindow)
+  if (display->focus_xwindow == xwindow &&
+      display->focus_window == window)
     return;
 
   if (display->focus_window)
@@ -1938,6 +1943,20 @@ update_focus_window (MetaDisplay *display,
   else
     meta_topic (META_DEBUG_FOCUS, "* Focus --> NULL with serial %lu\n", serial);
 
+#ifdef HAVE_WAYLAND
+  if (meta_is_wayland_compositor ())
+    {
+      compositor = meta_wayland_compositor_get_default ();
+
+      if (meta_display_xwindow_is_a_no_focus_window (display, xwindow))
+        meta_wayland_compositor_set_input_focus (compositor, NULL);
+      else if (window && window->surface)
+        meta_wayland_compositor_set_input_focus (compositor, window);
+      else
+        meta_topic (META_DEBUG_FOCUS, "Focus change has no effect, because there is no matching wayland 
surface");
+     }
+#endif
+
   g_object_notify (G_OBJECT (display), "focus-window");
   meta_display_update_active_window_hint (display);
 }
@@ -1974,17 +1993,15 @@ timestamp_too_old (MetaDisplay *display,
 static void
 request_xserver_input_focus_change (MetaDisplay *display,
                                     MetaScreen  *screen,
+                                    MetaWindow  *meta_window,
                                     Window       xwindow,
                                     guint32      timestamp)
 {
-  MetaWindow *meta_window;
   gulong serial;
 
   if (timestamp_too_old (display, &timestamp))
     return;
 
-  meta_window = meta_display_lookup_x_window (display, xwindow);
-
   meta_error_trap_push (display);
 
   /* In order for mutter to know that the focus request succeeded, we track
@@ -2651,15 +2668,6 @@ meta_display_handle_event (MetaDisplay *display,
             }
           break;
         case XI_FocusIn:
-#ifdef HAVE_WAYLAND
-          if (meta_is_wayland_compositor ())
-            {
-              MetaWaylandCompositor *compositor =
-                meta_wayland_compositor_get_default ();
-              meta_wayland_compositor_set_input_focus (compositor, window);
-            }
-#endif
-          /* fall through */
         case XI_FocusOut:
           /* libXi does not properly copy the serial to the XIEnterEvent, so pull it
            * from the parent XAnyEvent.
@@ -5884,6 +5892,7 @@ meta_display_set_input_focus_window (MetaDisplay *display,
 {
   request_xserver_input_focus_change (display,
                                       window->screen,
+                                      window,
                                       focus_frame ? window->frame->xwindow : window->xwindow,
                                       timestamp);
 }
@@ -5931,6 +5940,7 @@ meta_display_set_input_focus_xwindow (MetaDisplay *display,
 {
   request_xserver_input_focus_change (display,
                                       screen,
+                                      NULL,
                                       window,
                                       timestamp);
 }
@@ -5942,6 +5952,7 @@ meta_display_focus_the_no_focus_window (MetaDisplay *display,
 {
   request_xserver_input_focus_change (display,
                                       screen,
+                                      NULL,
                                       screen->no_focus_window,
                                       timestamp);
 }
diff --git a/src/core/window.c b/src/core/window.c
index 599965a..efb4f74 100644
--- a/src/core/window.c
+++ b/src/core/window.c
@@ -62,6 +62,10 @@
 
 #include <X11/extensions/Xcomposite.h>
 
+#ifdef HAVE_WAYLAND
+#include "meta-wayland-private.h"
+#endif
+
 /* Windows that unmaximize to a size bigger than that fraction of the workarea
  * will be scaled down to that size (while maintaining aspect ratio).
  * Windows that cover an area greater then this size are automaximized on map.
@@ -2103,6 +2107,11 @@ meta_window_unmanage (MetaWindow  *window,
       meta_error_trap_pop (window->display);
     }
 
+#ifdef HAVE_WAYLAND
+  if (window->surface)
+    meta_wayland_surface_free (window->surface);
+#endif
+
   meta_prefs_remove_listener (prefs_changed_callback, window);
 
   meta_screen_queue_check_fullscreen (window->screen);
diff --git a/src/wayland/meta-wayland-pointer.c b/src/wayland/meta-wayland-pointer.c
index 52a81ad..fd0efed 100644
--- a/src/wayland/meta-wayland-pointer.c
+++ b/src/wayland/meta-wayland-pointer.c
@@ -60,6 +60,7 @@ lose_pointer_focus (struct wl_listener *listener, void *data)
     wl_container_of (listener, pointer, focus_listener);
 
   pointer->focus_resource = NULL;
+  pointer->focus = NULL;
 }
 
 static void
@@ -138,6 +139,9 @@ meta_wayland_pointer_release (MetaWaylandPointer *pointer)
   /* XXX: What about pointer->resource_list? */
   if (pointer->focus_resource)
     wl_list_remove (&pointer->focus_listener.link);
+
+  pointer->focus = NULL;
+  pointer->focus_resource = NULL;
 }
 
 static struct wl_resource *
@@ -148,9 +152,7 @@ find_resource_for_surface (struct wl_list *list, MetaWaylandSurface *surface)
   if (!surface)
     return NULL;
 
-  if (!surface->resource)
-    return NULL;
-
+  g_assert (surface->resource);
   client = wl_resource_get_client (surface->resource);
 
   return wl_resource_find_for_client (list, client);
diff --git a/src/wayland/meta-wayland-private.h b/src/wayland/meta-wayland-private.h
index b4ba57d..92f5d83 100644
--- a/src/wayland/meta-wayland-private.h
+++ b/src/wayland/meta-wayland-private.h
@@ -356,4 +356,6 @@ void                    meta_wayland_compositor_repick          (MetaWaylandComp
 void                    meta_wayland_compositor_set_input_focus (MetaWaylandCompositor *compositor,
                                                                  MetaWindow            *window);
 
+void                    meta_wayland_surface_free               (MetaWaylandSurface    *surface);
+
 #endif /* META_WAYLAND_PRIVATE_H */
diff --git a/src/wayland/meta-wayland.c b/src/wayland/meta-wayland.c
index b01c60b..7c41c21 100644
--- a/src/wayland/meta-wayland.c
+++ b/src/wayland/meta-wayland.c
@@ -247,6 +247,10 @@ meta_wayland_surface_attach (struct wl_client *wayland_client,
     wl_resource_get_user_data (wayland_surface_resource);
   MetaWaylandBuffer *buffer;
 
+  /* X11 unmanaged window */
+  if (!surface)
+    return;
+
   if (wayland_buffer_resource)
     buffer = meta_wayland_buffer_from_resource (wayland_buffer_resource);
   else
@@ -277,6 +281,10 @@ meta_wayland_surface_damage (struct wl_client *client,
   MetaWaylandSurface *surface = wl_resource_get_user_data (surface_resource);
   cairo_rectangle_int_t rectangle = { x, y, width, height };
 
+  /* X11 unmanaged window */
+  if (!surface)
+    return;
+
   cairo_region_union_rectangle (surface->pending.damage, &rectangle);
 }
 
@@ -298,6 +306,10 @@ meta_wayland_surface_frame (struct wl_client *client,
   MetaWaylandFrameCallback *callback;
   MetaWaylandSurface *surface = wl_resource_get_user_data (surface_resource);
 
+  /* X11 unmanaged window */
+  if (!surface)
+    return;
+
   callback = g_slice_new0 (MetaWaylandFrameCallback);
   callback->compositor = surface->compositor;
   callback->resource = wl_resource_create (client,
@@ -337,7 +349,13 @@ meta_wayland_surface_commit (struct wl_client *client,
                              struct wl_resource *resource)
 {
   MetaWaylandSurface *surface = wl_resource_get_user_data (resource);
-  MetaWaylandCompositor *compositor = surface->compositor;
+  MetaWaylandCompositor *compositor;
+
+  /* X11 unmanaged window */
+  if (!surface)
+    return;
+
+  compositor = surface->compositor;
 
   /* wl_surface.attach */
   if (surface->pending.newly_attached &&
@@ -432,14 +450,6 @@ meta_wayland_compositor_set_input_focus (MetaWaylandCompositor *compositor,
   meta_wayland_data_device_set_keyboard_focus (compositor->seat);
 }
 
-static void
-window_destroyed_cb (void *user_data, GObject *old_object)
-{
-  MetaWaylandSurface *surface = user_data;
-
-  surface->window = NULL;
-}
-
 void
 meta_wayland_compositor_repick (MetaWaylandCompositor *compositor)
 {
@@ -448,7 +458,7 @@ meta_wayland_compositor_repick (MetaWaylandCompositor *compositor)
                             NULL);
 }
 
-static void
+void
 meta_wayland_surface_free (MetaWaylandSurface *surface)
 {
   MetaWaylandCompositor *compositor = surface->compositor;
@@ -458,21 +468,6 @@ meta_wayland_surface_free (MetaWaylandSurface *surface)
 
   meta_wayland_buffer_reference (&surface->buffer_ref, NULL);
 
-  if (surface->window)
-    g_object_weak_unref (G_OBJECT (surface->window),
-                         window_destroyed_cb,
-                         surface);
-
-  /* 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)
-    {
-      MetaDisplay *display = meta_get_display ();
-      guint32 timestamp = meta_display_get_current_time_roundtrip (display);
-      meta_window_unmanage (surface->window, timestamp);
-    }
-
   if (surface->pending.buffer)
     wl_list_remove (&surface->pending.buffer_destroy_listener.link);
 
@@ -482,19 +477,51 @@ meta_wayland_surface_free (MetaWaylandSurface *surface)
                          &surface->pending.frame_callback_list, link)
     wl_resource_destroy (cb->resource);
 
-  g_slice_free (MetaWaylandSurface, surface);
-
   meta_wayland_compositor_repick (compositor);
 
+  /* Check that repick didn't pick the freed surface */
+  g_assert (surface != compositor->seat->pointer.focus);
+  g_assert (surface != compositor->seat->keyboard.focus);
+
  if (compositor->implicit_grab_surface == surface)
    compositor->implicit_grab_surface = compositor->seat->pointer.current;
+
+  if (surface->resource)
+    wl_resource_set_user_data (surface->resource, NULL);
+  g_slice_free (MetaWaylandSurface, surface);
 }
 
 static void
 meta_wayland_surface_resource_destroy_cb (struct wl_resource *resource)
 {
   MetaWaylandSurface *surface = wl_resource_get_user_data (resource);
-  meta_wayland_surface_free (surface);
+
+  /* There are four 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
+  */
+
+  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)
+       {
+         MetaDisplay *display = meta_get_display ();
+         guint32 timestamp = meta_display_get_current_time_roundtrip (display);
+
+         meta_window_unmanage (surface->window, timestamp);
+       }
+      else
+       meta_wayland_surface_free (surface);
+    }
 }
 
 static void
@@ -929,13 +956,6 @@ ensure_surface_window (MetaWaylandSurface *surface)
       surface->window =
         meta_window_new_for_wayland (display, width, height, surface);
 
-      /* If the MetaWindow becomes unmanaged (surface->window will be
-       * freed in this case) we need to make sure to clear our
-       * ->window pointers. */
-      g_object_weak_ref (G_OBJECT (surface->window),
-                         window_destroyed_cb,
-                         surface);
-
       meta_window_calc_showing (surface->window);
     }
 }
@@ -1149,13 +1169,6 @@ xserver_set_window_id (struct wl_client *client,
       surface->window = window;
       window->surface = surface;
 
-      /* If the MetaWindow becomes unmanaged (surface->window will be
-       * freed in this case) we need to make sure to clear our
-       * ->window pointers in this case. */
-      g_object_weak_ref (G_OBJECT (surface->window),
-                         window_destroyed_cb,
-                         surface);
-
       /* If the window is already meant to have focus then the
        * original attempt to call this in response to the FocusIn
        * event will have been lost because there was no surface
@@ -1342,7 +1355,7 @@ event_cb (ClutterActor *stage,
   meta_wayland_seat_handle_event (compositor->seat, event);
 
   /* HACK: for now, the surfaces from Wayland clients aren't
-     integrated into Mutter's stacking and Mutter won't give them
+     integrated into Mutter's event handling and Mutter won't give them
      focus on mouse clicks. As a hack to work around this we can just
      give them input focus on mouse clicks so we can at least test the
      keyboard support */
@@ -1350,12 +1363,13 @@ event_cb (ClutterActor *stage,
     {
       surface = pointer->current;
 
-      /* Only focus surfaces that wouldn't be handled by the
-         corresponding X events */
-      if (surface && surface->xid == 0)
+      if (surface && surface->window &&
+         surface->window->client_type == META_WINDOW_CLIENT_TYPE_WAYLAND)
         {
-          meta_wayland_keyboard_set_focus (&seat->keyboard, surface);
-          meta_wayland_data_device_set_keyboard_focus (seat);
+         MetaDisplay *display = meta_get_display ();
+         guint32 timestamp = meta_display_get_current_time_roundtrip (display);
+
+         meta_window_focus (surface->window, timestamp);
         }
     }