xwayland/xwm: replace role with addon

Closes: https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3545

include/wlr/xwayland/xwayland.h

@@ -13,6 +13,7 @@
 #include <wayland-server-core.h>
 #include <xcb/xcb.h>
 #include <xcb/xcb_icccm.h>
+#include <wlr/util/addon.h>
 
 struct wlr_xwm;
 struct wlr_data_source;
@@ -89,6 +90,10 @@ struct wlr_xwayland_surface {
 	struct wl_list unpaired_link;
 
 	struct wlr_surface *surface;
+	struct wlr_addon surface_addon;
+	struct wl_listener surface_commit;
+	struct wl_listener surface_precommit;
+
 	int16_t x, y;
 	uint16_t width, height;
 	uint16_t saved_width, saved_height;
@@ -227,15 +232,17 @@ void wlr_xwayland_set_seat(struct wlr_xwayland *xwayland,
 	struct wlr_seat *seat);
 
 /**
- * Returns true if the surface has the xwayland surface role.
+ * Check whether a surface is an Xwayland surface.
+ *
+ * As an edge case, if the surface has been created by Xwayland but has no X11
+ * window associated, false is returned.
  */
 bool wlr_surface_is_xwayland_surface(struct wlr_surface *surface);
 
 /**
  * Get a struct wlr_xwayland_surface from a struct wlr_surface.
- * Asserts that the surface has the xwayland surface role.
+ *
- * May return NULL even if the surface has the xwayland surface role if the
+ * This function asserts that the surface is an Xwayland surface.
- * corresponding xwayland surface has been unmapped or destroyed.
  */
 struct wlr_xwayland_surface *wlr_xwayland_surface_from_wlr_surface(
 	struct wlr_surface *surface);

xwayland/xwm.c

@@ -99,16 +99,18 @@ struct pending_startup_id {
 	struct wl_list link;
 };
 
-static const struct wlr_surface_role xwayland_surface_role;
+static const struct wlr_addon_interface surface_addon_impl;
 
 bool wlr_surface_is_xwayland_surface(struct wlr_surface *surface) {
-	return surface->role == &xwayland_surface_role;
+	return wlr_addon_find(&surface->addons, NULL, &surface_addon_impl) != NULL;
 }
 
 struct wlr_xwayland_surface *wlr_xwayland_surface_from_wlr_surface(
 		struct wlr_surface *surface) {
-	assert(wlr_surface_is_xwayland_surface(surface));
+	struct wlr_addon *addon = wlr_addon_find(&surface->addons, NULL, &surface_addon_impl);
-	return (struct wlr_xwayland_surface *)surface->role_data;
+	assert(addon != NULL);
+	struct wlr_xwayland_surface *xsurface = wl_container_of(addon, xsurface, surface_addon);
+	return xsurface;
 }
 
 // TODO: replace this with hash table?
@@ -386,10 +388,26 @@ static void xsurface_set_net_wm_state(struct wlr_xwayland_surface *xsurface) {
 		i, property);
 }
 
-static void xwayland_surface_destroy(
+static void xwayland_surface_set_mapped(struct wlr_xwayland_surface *xsurface, bool mapped);
-		struct wlr_xwayland_surface *xsurface) {
+
+static void xwayland_surface_dissociate(struct wlr_xwayland_surface *xsurface) {
+	xwayland_surface_set_mapped(xsurface, false);
+
+	// Make sure we're not on the unpaired surface list or we
+	// could be assigned a surface during surface creation that
+	// was mapped before this unmap request.
+	wl_list_remove(&xsurface->unpaired_link);
+	wl_list_init(&xsurface->unpaired_link);
+	wl_list_remove(&xsurface->surface_commit.link);
+	wl_list_remove(&xsurface->surface_precommit.link);
+	wlr_addon_finish(&xsurface->surface_addon);
+	xsurface->surface_id = 0;
+	xsurface->surface = NULL;
+}
+
+static void xwayland_surface_destroy(struct wlr_xwayland_surface *xsurface) {
 	if (xsurface->surface != NULL) {
-		wlr_surface_destroy_role_object(xsurface->surface);
+		xwayland_surface_dissociate(xsurface);
 	}
 
 	wl_signal_emit_mutable(&xsurface->events.destroy, xsurface);
@@ -823,71 +841,61 @@ static void read_surface_property(struct wlr_xwm *xwm,
 	free(reply);
 }
 
-static void xwayland_surface_role_commit(struct wlr_surface *wlr_surface) {
+static void xwayland_surface_set_mapped(struct wlr_xwayland_surface *xsurface, bool mapped) {
-	assert(wlr_surface->role == &xwayland_surface_role);
+	if (xsurface->mapped == mapped) {
-	struct wlr_xwayland_surface *surface = wlr_surface->role_data;
+		return;
+	}
+
+	xsurface->mapped = mapped;
 
-	if (!surface->mapped && wlr_surface_has_buffer(surface->surface)) {
+	if (mapped) {
-		surface->mapped = true;
+		wl_signal_emit_mutable(&xsurface->events.map, xsurface);
-		wl_signal_emit_mutable(&surface->events.map, surface);
+	} else {
-		xwm_set_net_client_list(surface->xwm);
+		wl_signal_emit_mutable(&xsurface->events.unmap, xsurface);
 	}
+
+	xwm_set_net_client_list(xsurface->xwm);
 }
 
-static void xwayland_surface_role_precommit(struct wlr_surface *wlr_surface,
+static void xwayland_surface_handle_commit(struct wl_listener *listener, void *data) {
-		const struct wlr_surface_state *state) {
+	struct wlr_xwayland_surface *xsurface = wl_container_of(listener, xsurface, surface_commit);
-	assert(wlr_surface->role == &xwayland_surface_role);
+	bool mapped = wlr_surface_has_buffer(xsurface->surface);
-	struct wlr_xwayland_surface *surface = wlr_surface->role_data;
+	xwayland_surface_set_mapped(xsurface, mapped);
+}
 
+static void xwayland_surface_handle_precommit(struct wl_listener *listener, void *data) {
+	struct wlr_xwayland_surface *xsurface = wl_container_of(listener, xsurface, surface_precommit);
+	const struct wlr_surface_state *state = data;
 	if (state->committed & WLR_SURFACE_STATE_BUFFER && state->buffer == NULL) {
 		// This is a NULL commit
-		if (surface->mapped) {
+		xwayland_surface_set_mapped(xsurface, false);
-			surface->mapped = false;
-			wl_signal_emit_mutable(&surface->events.unmap, surface);
-			xwm_set_net_client_list(surface->xwm);
-		}
-	}
 	}
-
-static void xwayland_surface_role_destroy(struct wlr_surface *wlr_surface) {
-	assert(wlr_surface->role == &xwayland_surface_role);
-	struct wlr_xwayland_surface *surface = wlr_surface->role_data;
-
-	if (surface->mapped) {
-		wl_signal_emit_mutable(&surface->events.unmap, surface);
-		surface->mapped = false;
-		xwm_set_net_client_list(surface->xwm);
 }
 
-	// Make sure we're not on the unpaired surface list or we
+static void xwayland_surface_handle_addon_destroy(struct wlr_addon *addon) {
-	// could be assigned a surface during surface creation that
+	struct wlr_xwayland_surface *xsurface = wl_container_of(addon, xsurface, surface_addon);
-	// was mapped before this unmap request.
+	xwayland_surface_dissociate(xsurface);
-	wl_list_remove(&surface->unpaired_link);
-	wl_list_init(&surface->unpaired_link);
-	surface->surface_id = 0;
-	surface->surface = NULL;
 }
 
-static const struct wlr_surface_role xwayland_surface_role = {
+static const struct wlr_addon_interface surface_addon_impl = {
 	.name = "wlr_xwayland_surface",
-	.commit = xwayland_surface_role_commit,
+	.destroy = xwayland_surface_handle_addon_destroy,
-	.precommit = xwayland_surface_role_precommit,
-	.destroy = xwayland_surface_role_destroy,
 };
 
 static void xwayland_surface_associate(struct wlr_xwm *xwm,
 		struct wlr_xwayland_surface *xsurface, struct wlr_surface *surface) {
-	if (!wlr_surface_set_role(surface, &xwayland_surface_role, xsurface,
-			NULL, 0)) {
-		wlr_log(WLR_ERROR, "Failed to set xwayland surface role");
-		return;
-	}
-
 	wl_list_remove(&xsurface->unpaired_link);
 	wl_list_init(&xsurface->unpaired_link);
 	xsurface->surface_id = 0;
 
 	xsurface->surface = surface;
+	wlr_addon_init(&xsurface->surface_addon, &surface->addons, NULL, &surface_addon_impl);
+
+	xsurface->surface_commit.notify = xwayland_surface_handle_commit;
+	wl_signal_add(&surface->events.commit, &xsurface->surface_commit);
+
+	xsurface->surface_precommit.notify = xwayland_surface_handle_precommit;
+	wl_signal_add(&surface->events.commit, &xsurface->surface_precommit);
 
 	// read all surface properties
 	const xcb_atom_t props[] = {
@@ -1080,7 +1088,7 @@ static void xwm_handle_unmap_notify(struct wlr_xwm *xwm,
 	}
 
 	if (xsurface->surface != NULL) {
-		wlr_surface_destroy_role_object(xsurface->surface);
+		xwayland_surface_dissociate(xsurface);
 	}
 
 	xsurface_set_wm_state(xsurface, XCB_ICCCM_WM_STATE_WITHDRAWN);