From 5515faa197c40c6b4bc853df14a8da9f1f3f1b80 Mon Sep 17 00:00:00 2001 From: emersion Date: Tue, 4 Dec 2018 13:42:29 +0100 Subject: [PATCH] xdg-shell: emit xdg_surface destroy when role object is destroyed Fixes https://github.com/swaywm/wlroots/issues/1407 --- include/types/wlr_xdg_shell.h | 1 + types/xdg_shell/wlr_xdg_popup.c | 32 ++++++---------- types/xdg_shell/wlr_xdg_surface.c | 61 +++++++++++++++++++----------- types/xdg_shell/wlr_xdg_toplevel.c | 51 ++++++++++--------------- 4 files changed, 71 insertions(+), 74 deletions(-) diff --git a/include/types/wlr_xdg_shell.h b/include/types/wlr_xdg_shell.h index 93fdc670..08a691bd 100644 --- a/include/types/wlr_xdg_shell.h +++ b/include/types/wlr_xdg_shell.h @@ -18,6 +18,7 @@ struct wlr_xdg_surface *create_xdg_surface( struct wlr_xdg_client *client, struct wlr_surface *surface, uint32_t id); void unmap_xdg_surface(struct wlr_xdg_surface *surface); +void reset_xdg_surface(struct wlr_xdg_surface *xdg_surface); void destroy_xdg_surface(struct wlr_xdg_surface *surface); void handle_xdg_surface_commit(struct wlr_surface *wlr_surface); void handle_xdg_surface_precommit(struct wlr_surface *wlr_surface); diff --git a/types/xdg_shell/wlr_xdg_popup.c b/types/xdg_shell/wlr_xdg_popup.c index d23d7dac..874329f9 100644 --- a/types/xdg_shell/wlr_xdg_popup.c +++ b/types/xdg_shell/wlr_xdg_popup.c @@ -207,9 +207,7 @@ static const struct xdg_popup_interface xdg_popup_implementation = { static void xdg_popup_handle_resource_destroy(struct wl_resource *resource) { struct wlr_xdg_surface *surface = wlr_xdg_surface_from_popup_resource(resource); - if (surface != NULL) { - destroy_xdg_popup(surface); - } + destroy_xdg_popup(surface); } const struct wlr_surface_role xdg_popup_surface_role = { @@ -241,14 +239,13 @@ void create_xdg_popup(struct wlr_xdg_surface *xdg_surface, 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; + assert(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, @@ -280,18 +277,11 @@ void create_xdg_popup(struct wlr_xdg_surface *xdg_surface, } void destroy_xdg_popup(struct wlr_xdg_surface *xdg_surface) { + if (xdg_surface == NULL) { + return; + } assert(xdg_surface->role == WLR_XDG_SURFACE_ROLE_POPUP); - unmap_xdg_surface(xdg_surface); - - // 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; - - wl_list_remove(&xdg_surface->popup->link); - - xdg_surface->role = WLR_XDG_SURFACE_ROLE_NONE; + reset_xdg_surface(xdg_surface); } 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 0b1d599f..7046d5f0 100644 --- a/types/xdg_shell/wlr_xdg_surface.c +++ b/types/xdg_shell/wlr_xdg_surface.c @@ -439,12 +439,43 @@ struct wlr_xdg_surface *create_xdg_surface( return xdg_surface; } -void destroy_xdg_surface(struct wlr_xdg_surface *surface) { - if (surface->role != WLR_XDG_SURFACE_ROLE_NONE) { - unmap_xdg_surface(surface); +void reset_xdg_surface(struct wlr_xdg_surface *xdg_surface) { + if (xdg_surface->role != WLR_XDG_SURFACE_ROLE_NONE) { + unmap_xdg_surface(xdg_surface); } - wlr_signal_emit_safe(&surface->events.destroy, surface); + if (xdg_surface->added) { + wlr_signal_emit_safe(&xdg_surface->events.destroy, xdg_surface); + xdg_surface->added = false; + } + + switch (xdg_surface->role) { + case WLR_XDG_SURFACE_ROLE_TOPLEVEL: + wl_resource_set_user_data(xdg_surface->toplevel->resource, NULL); + xdg_surface->toplevel->resource = NULL; + + free(xdg_surface->toplevel); + xdg_surface->toplevel = NULL; + break; + case WLR_XDG_SURFACE_ROLE_POPUP: + wl_resource_set_user_data(xdg_surface->popup->resource, NULL); + xdg_surface->toplevel->resource = NULL; + + wl_list_remove(&xdg_surface->popup->link); + + free(xdg_surface->popup); + xdg_surface->popup = NULL; + break; + case WLR_XDG_SURFACE_ROLE_NONE: + // This space is intentionally left blank + break; + } + + xdg_surface->role = WLR_XDG_SURFACE_ROLE_NONE; +} + +void destroy_xdg_surface(struct wlr_xdg_surface *surface) { + reset_xdg_surface(surface); struct wlr_xdg_popup *popup_state, *next; wl_list_for_each_safe(popup_state, next, &surface->popups, link) { @@ -452,26 +483,9 @@ void destroy_xdg_surface(struct wlr_xdg_surface *surface) { destroy_xdg_popup(popup_state->base); } - switch (surface->role) { - case WLR_XDG_SURFACE_ROLE_TOPLEVEL: - destroy_xdg_toplevel(surface); - break; - case WLR_XDG_SURFACE_ROLE_POPUP: - destroy_xdg_popup(surface); - break; - case WLR_XDG_SURFACE_ROLE_NONE: - // This space is intentionally left blank - 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); wl_list_remove(&surface->surface_destroy.link); wl_list_remove(&surface->surface_commit.link); @@ -625,7 +639,8 @@ void wlr_xdg_surface_for_each_popup(struct wlr_xdg_surface *surface, xdg_surface_for_each_popup(surface, 0, 0, iterator, user_data); } -void wlr_xdg_surface_get_geometry(struct wlr_xdg_surface *surface, struct wlr_box *box) { +void wlr_xdg_surface_get_geometry(struct wlr_xdg_surface *surface, + struct wlr_box *box) { wlr_surface_get_extends(surface->surface, box); /* The client never set the geometry */ if (!surface->geometry.width) { diff --git a/types/xdg_shell/wlr_xdg_toplevel.c b/types/xdg_shell/wlr_xdg_toplevel.c index d3220994..d3ee0cb7 100644 --- a/types/xdg_shell/wlr_xdg_toplevel.c +++ b/types/xdg_shell/wlr_xdg_toplevel.c @@ -444,9 +444,7 @@ static const struct xdg_toplevel_interface xdg_toplevel_implementation = { static void xdg_toplevel_handle_resource_destroy(struct wl_resource *resource) { struct wlr_xdg_surface *surface = wlr_xdg_surface_from_toplevel_resource(resource); - if (surface != NULL) { - destroy_xdg_toplevel(surface); - } + destroy_xdg_toplevel(surface); } const struct wlr_surface_role xdg_toplevel_surface_role = { @@ -469,28 +467,24 @@ void create_xdg_toplevel(struct wlr_xdg_surface *xdg_surface, return; } + assert(xdg_surface->toplevel == NULL); + xdg_surface->toplevel = calloc(1, sizeof(struct wlr_xdg_toplevel)); 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; + wl_resource_post_no_memory(xdg_surface->resource); + return; } + xdg_surface->toplevel->base = xdg_surface; - xdg_surface->role = WLR_XDG_SURFACE_ROLE_TOPLEVEL; + 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); - 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); @@ -502,19 +496,16 @@ void create_xdg_toplevel(struct wlr_xdg_surface *xdg_surface, wl_resource_set_implementation(xdg_surface->toplevel->resource, &xdg_toplevel_implementation, xdg_surface, xdg_toplevel_handle_resource_destroy); + + xdg_surface->role = WLR_XDG_SURFACE_ROLE_TOPLEVEL; } void destroy_xdg_toplevel(struct wlr_xdg_surface *xdg_surface) { + if (xdg_surface == NULL) { + return; + } 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; + reset_xdg_surface(xdg_surface); } uint32_t wlr_xdg_toplevel_set_size(struct wlr_xdg_surface *surface,