xwayland: Better document/assert view/surface association

Each XWayland view is paired with a particular wlr_xwayland_surface and
its lifetime is tied to that surface.  This condition in handle_map():

    if (xsurface != view->xwayland_surface)

could never be true since the view is only registered to receive the
"map" signal from view->xwayland_surface, and no other.  So the code
updating view->xwayland_surface in handle_map() was dead.

So let's clean things up a little:

- Remove the dead code
- Add some comments, and slightly rearrange code to match
- Add/update assert()s in signal handlers for consistency
- Pass xsurface as <data> when calling handle_unmap() and
  handle_destroy() explicitly, to be consistent

diff --git a/src/xwayland.c b/src/xwayland.c
index 4dc33444438f4bfeee001da26616598e474b67a9..b360b1c7aff3d9d94519fda0f1d17759a588e98c 100644 (file)
--- a/src/xwayland.c
+++ b/src/xwayland.c
@@ -10,7 +10,7 @@ static void
 handle_commit(struct wl_listener *listener, void *data)
 {
        struct view *view = wl_container_of(listener, view, commit);
-       assert(view->surface);
+       assert(data && data == view->surface);
 
        /* Must receive commit signal before accessing surface->current* */
        struct wlr_surface_state *state = &view->surface->current;
@@ -69,11 +69,8 @@ static void
 handle_map(struct wl_listener *listener, void *data)
 {
        struct view *view = wl_container_of(listener, view, map);
-       struct wlr_xwayland_surface *xsurface = data;
-       if (xsurface != view->xwayland_surface) {
-               xsurface->data = view;
-               view->xwayland_surface = xsurface;
-       }
+       assert(data && data == view->xwayland_surface);
+
        view->impl->map(view);
 }
 
@@ -81,6 +78,8 @@ static void
 handle_unmap(struct wl_listener *listener, void *data)
 {
        struct view *view = wl_container_of(listener, view, unmap);
+       assert(data && data == view->xwayland_surface);
+
        view->impl->unmap(view);
 
        /*
@@ -100,9 +99,7 @@ static void
 handle_surface_destroy(struct wl_listener *listener, void *data)
 {
        struct view *view = wl_container_of(listener, view, surface_destroy);
-       assert(view->type == LAB_XWAYLAND_VIEW);
-       struct wlr_surface *surface = data;
-       assert(surface == view->surface);
+       assert(data && data == view->surface);
 
        view->surface = NULL;
        wl_list_remove(&view->surface_destroy.link);
@@ -112,9 +109,9 @@ static void
 handle_destroy(struct wl_listener *listener, void *data)
 {
        struct view *view = wl_container_of(listener, view, destroy);
-       assert(view->type == LAB_XWAYLAND_VIEW);
+       assert(data && data == view->xwayland_surface);
+       assert(view->xwayland_surface->data == view);
 
-       /* Reset XWayland specific surface for good measure */
        if (view->surface) {
                /*
                 * We got the destroy signal from
@@ -124,6 +121,13 @@ handle_destroy(struct wl_listener *listener, void *data)
                wl_list_remove(&view->surface_destroy.link);
        }
        view->surface = NULL;
+
+       /*
+        * Break view <-> xsurface association.  Note that the xsurface
+        * may not actually be destroyed at this point; it may become an
+        * "unmanaged" surface instead.
+        */
+       view->xwayland_surface->data = NULL;
        view->xwayland_surface = NULL;
 
        /* Remove XWayland specific handlers */
@@ -289,13 +293,14 @@ handle_override_redirect(struct wl_listener *listener, void *data)
 {
        struct view *view = wl_container_of(listener, view, override_redirect);
        struct wlr_xwayland_surface *xsurface = data;
+       assert(xsurface && xsurface == view->xwayland_surface);
        struct server *server = view->server;
        bool mapped = xsurface->mapped;
        if (mapped) {
-               handle_unmap(&view->unmap, NULL);
+               handle_unmap(&view->unmap, xsurface);
        }
-       handle_destroy(&view->destroy, view);
-       xsurface->data = NULL;
+       handle_destroy(&view->destroy, xsurface);
+       /* view is invalid after this point */
        struct xwayland_unmanaged *unmanaged =
                xwayland_unmanaged_create(server, xsurface);
        if (mapped) {
@@ -488,13 +493,22 @@ xwayland_surface_new(struct wl_listener *listener, void *data)
        view->server = server;
        view->type = LAB_XWAYLAND_VIEW;
        view->impl = &xwl_view_impl;
+
+       /*
+        * Set two-way view <-> xsurface association.  Usually the
+        * association remains until the xsurface is destroyed (which
+        * also destroys the view).  The only exception is caused by
+        * setting override-redirect on the xsurface, which removes it
+        * from the view (destroying the view) and makes it an
+        * "unmanaged" surface.
+        */
        view->xwayland_surface = xsurface;
+       xsurface->data = view;
 
        view->workspace = server->workspace_current;
        view->scene_tree = wlr_scene_tree_create(view->workspace->tree);
        node_descriptor_create(&view->scene_tree->node,
                LAB_NODE_DESC_VIEW, view);
-       xsurface->data = view;
 
        view->map.notify = handle_map;
        wl_signal_add(&xsurface->events.map, &view->map);