From 95dfbe2962ff6f30d02143f00b037baa6b812d06 Mon Sep 17 00:00:00 2001 From: emersion Date: Mon, 12 Nov 2018 19:37:22 +0100 Subject: [PATCH] xdg-shell: don't destroy xdg role state on role destroy ie. don't destroy surface->toplevel on xdg_toplevel destroy. Instead do this on xdg_surface destroy. This allows compositors to add toplevel listeners when the surface appears and remove them when the surface is destroyed. --- types/xdg_shell/wlr_xdg_popup.c | 38 +++++++++++++------- types/xdg_shell/wlr_xdg_surface.c | 18 ++++------ types/xdg_shell/wlr_xdg_toplevel.c | 56 +++++++++++++++++++++--------- 3 files changed, 71 insertions(+), 41 deletions(-) diff --git a/types/xdg_shell/wlr_xdg_popup.c b/types/xdg_shell/wlr_xdg_popup.c index b3409261..d23d7dac 100644 --- a/types/xdg_shell/wlr_xdg_popup.c +++ b/types/xdg_shell/wlr_xdg_popup.c @@ -234,12 +234,22 @@ void create_xdg_popup(struct wlr_xdg_surface *xdg_surface, return; } - xdg_surface->popup = calloc(1, sizeof(struct wlr_xdg_popup)); - if (!xdg_surface->popup) { - wl_resource_post_no_memory(xdg_surface->resource); + if (xdg_surface->role != WLR_XDG_SURFACE_ROLE_NONE) { + wl_resource_post_error(xdg_surface->resource, + XDG_SURFACE_ERROR_ALREADY_CONSTRUCTED, + "xdg-surface has already been constructed"); return; } + if (xdg_surface->popup == NULL) { + xdg_surface->popup = calloc(1, sizeof(struct wlr_xdg_popup)); + if (!xdg_surface->popup) { + wl_resource_post_no_memory(xdg_surface->resource); + return; + } + xdg_surface->popup->base = xdg_surface; + } + xdg_surface->popup->resource = wl_resource_create( xdg_surface->client->client, &xdg_popup_interface, wl_resource_get_version(xdg_surface->resource), id); @@ -253,7 +263,6 @@ void create_xdg_popup(struct wlr_xdg_surface *xdg_surface, xdg_popup_handle_resource_destroy); xdg_surface->role = WLR_XDG_SURFACE_ROLE_POPUP; - xdg_surface->popup->base = xdg_surface; // positioner properties memcpy(&xdg_surface->popup->positioner, &positioner->attrs, @@ -265,19 +274,24 @@ void create_xdg_popup(struct wlr_xdg_surface *xdg_surface, xdg_surface->popup->parent = parent->surface; wl_list_insert(&parent->popups, &xdg_surface->popup->link); wlr_signal_emit_safe(&parent->events.new_popup, xdg_surface->popup); + } else { + wl_list_init(&xdg_surface->popup->link); } } -void destroy_xdg_popup(struct wlr_xdg_surface *surface) { - assert(surface->role == WLR_XDG_SURFACE_ROLE_POPUP); - unmap_xdg_surface(surface); +void destroy_xdg_popup(struct wlr_xdg_surface *xdg_surface) { + assert(xdg_surface->role == WLR_XDG_SURFACE_ROLE_POPUP); + unmap_xdg_surface(xdg_surface); - wl_resource_set_user_data(surface->popup->resource, NULL); - wl_list_remove(&surface->popup->link); - free(surface->popup); - surface->popup = NULL; + // Don't destroy the popup state yet, the compositor might have some + // listeners set up. Anyway the client can only re-create another xdg-popup + // with this xdg-surface because of role restrictions. + wl_resource_set_user_data(xdg_surface->popup->resource, NULL); + xdg_surface->toplevel->resource = NULL; - surface->role = WLR_XDG_SURFACE_ROLE_NONE; + wl_list_remove(&xdg_surface->popup->link); + + xdg_surface->role = WLR_XDG_SURFACE_ROLE_NONE; } void wlr_xdg_popup_get_anchor_point(struct wlr_xdg_popup *popup, diff --git a/types/xdg_shell/wlr_xdg_surface.c b/types/xdg_shell/wlr_xdg_surface.c index e32b7f18..0b1d599f 100644 --- a/types/xdg_shell/wlr_xdg_surface.c +++ b/types/xdg_shell/wlr_xdg_surface.c @@ -83,18 +83,6 @@ void unmap_xdg_surface(struct wlr_xdg_surface *surface) { memset(&surface->next_geometry, 0, sizeof(struct wlr_box)); } -void destroy_xdg_toplevel(struct wlr_xdg_surface *surface) { - assert(surface->role == WLR_XDG_SURFACE_ROLE_TOPLEVEL); - unmap_xdg_surface(surface); - - wl_resource_set_user_data(surface->toplevel->resource, NULL); - free(surface->toplevel); - surface->toplevel = NULL; - - surface->role = WLR_XDG_SURFACE_ROLE_NONE; -} - - static void xdg_surface_handle_ack_configure(struct wl_client *client, struct wl_resource *resource, uint32_t serial) { @@ -476,6 +464,12 @@ void destroy_xdg_surface(struct wlr_xdg_surface *surface) { break; } + if (surface->surface->role == &xdg_toplevel_surface_role) { + free(surface->toplevel); + } else if (surface->surface->role == &xdg_popup_surface_role) { + free(surface->popup); + } + wl_resource_set_user_data(surface->resource, NULL); surface->surface->role_data = NULL; wl_list_remove(&surface->link); diff --git a/types/xdg_shell/wlr_xdg_toplevel.c b/types/xdg_shell/wlr_xdg_toplevel.c index 77398c7e..d3220994 100644 --- a/types/xdg_shell/wlr_xdg_toplevel.c +++ b/types/xdg_shell/wlr_xdg_toplevel.c @@ -462,37 +462,59 @@ void create_xdg_toplevel(struct wlr_xdg_surface *xdg_surface, return; } - xdg_surface->toplevel = calloc(1, sizeof(struct wlr_xdg_toplevel)); - if (xdg_surface->toplevel == NULL) { - wl_resource_post_no_memory(xdg_surface->resource); + if (xdg_surface->role != WLR_XDG_SURFACE_ROLE_NONE) { + wl_resource_post_error(xdg_surface->resource, + XDG_SURFACE_ERROR_ALREADY_CONSTRUCTED, + "xdg-surface has already been constructed"); return; } - wl_signal_init(&xdg_surface->toplevel->events.request_maximize); - wl_signal_init(&xdg_surface->toplevel->events.request_fullscreen); - wl_signal_init(&xdg_surface->toplevel->events.request_minimize); - wl_signal_init(&xdg_surface->toplevel->events.request_move); - wl_signal_init(&xdg_surface->toplevel->events.request_resize); - wl_signal_init(&xdg_surface->toplevel->events.request_show_window_menu); - wl_signal_init(&xdg_surface->toplevel->events.set_parent); - wl_signal_init(&xdg_surface->toplevel->events.set_title); - wl_signal_init(&xdg_surface->toplevel->events.set_app_id); + + if (xdg_surface->toplevel == NULL) { + xdg_surface->toplevel = calloc(1, sizeof(struct wlr_xdg_toplevel)); + if (xdg_surface->toplevel == NULL) { + wl_resource_post_no_memory(xdg_surface->resource); + return; + } + wl_signal_init(&xdg_surface->toplevel->events.request_maximize); + wl_signal_init(&xdg_surface->toplevel->events.request_fullscreen); + wl_signal_init(&xdg_surface->toplevel->events.request_minimize); + wl_signal_init(&xdg_surface->toplevel->events.request_move); + wl_signal_init(&xdg_surface->toplevel->events.request_resize); + wl_signal_init(&xdg_surface->toplevel->events.request_show_window_menu); + wl_signal_init(&xdg_surface->toplevel->events.set_parent); + wl_signal_init(&xdg_surface->toplevel->events.set_title); + wl_signal_init(&xdg_surface->toplevel->events.set_app_id); + + xdg_surface->toplevel->base = xdg_surface; + } xdg_surface->role = WLR_XDG_SURFACE_ROLE_TOPLEVEL; - xdg_surface->toplevel->base = xdg_surface; - struct wl_resource *toplevel_resource = wl_resource_create( + assert(xdg_surface->toplevel->resource == NULL); + xdg_surface->toplevel->resource = wl_resource_create( xdg_surface->client->client, &xdg_toplevel_interface, wl_resource_get_version(xdg_surface->resource), id); - if (toplevel_resource == NULL) { + if (xdg_surface->toplevel->resource == NULL) { free(xdg_surface->toplevel); wl_resource_post_no_memory(xdg_surface->resource); return; } - wl_resource_set_implementation(toplevel_resource, + wl_resource_set_implementation(xdg_surface->toplevel->resource, &xdg_toplevel_implementation, xdg_surface, xdg_toplevel_handle_resource_destroy); +} - xdg_surface->toplevel->resource = toplevel_resource; +void destroy_xdg_toplevel(struct wlr_xdg_surface *xdg_surface) { + assert(xdg_surface->role == WLR_XDG_SURFACE_ROLE_TOPLEVEL); + unmap_xdg_surface(xdg_surface); + + // Don't destroy the toplevel state yet, the compositor might have some + // listeners set up. Anyway the client can only re-create another + // xdg-toplevel with this xdg-surface because of role restrictions. + wl_resource_set_user_data(xdg_surface->toplevel->resource, NULL); + xdg_surface->toplevel->resource = NULL; + + xdg_surface->role = WLR_XDG_SURFACE_ROLE_NONE; } uint32_t wlr_xdg_toplevel_set_size(struct wlr_xdg_surface *surface,