[PATCH weston v4] libweston-desktop/xdg-shell: Properly handle ack_configure
Quentin Glidic
sardemff7+wayland at sardemff7.net
Thu Jul 20 08:53:44 UTC 2017
On 7/20/17 10:49 AM, Jonas Ådahl wrote:
> On Thu, Jul 20, 2017 at 10:45:38AM +0200, Quentin Glidic wrote:
>> From: Quentin Glidic <sardemff7+git at sardemff7.net>
>>
>> Now we keep track of serial->state association and we discard the states
>> that the client ignored.
>>
>> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
>
> Reviewed-by: Jonas Ådahl <jadahl at gmail.com>
Thanks. And pushed:
19d1f6ef..749637a8 master -> master
Cheers,
>> ---
>>
>> On 7/20/17 10:24 AM, Jonas Ådahl wrote:
>>> Noticed one more thing I missed in the previous review: cleanup of the
>>> pending configure allocations on surface destruction.
>>
>> Oh, good catch, thanks.
>>
>> v3:
>> Fixed a tiny style issue
>> Now send an error on wrong serial
>> v4:
>> Free configure_list on destroy
>>
>> libweston-desktop/xdg-shell-v5.c | 60 ++++++++++++++++++++++++++----
>> libweston-desktop/xdg-shell-v6.c | 79 +++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 122 insertions(+), 17 deletions(-)
>>
>> diff --git a/libweston-desktop/xdg-shell-v5.c b/libweston-desktop/xdg-shell-v5.c
>> index b32b7812..c91c2590 100644
>> --- a/libweston-desktop/xdg-shell-v5.c
>> +++ b/libweston-desktop/xdg-shell-v5.c
>> @@ -46,6 +46,13 @@ struct weston_desktop_xdg_surface_state {
>> bool activated;
>> };
>>
>> +struct weston_desktop_xdg_surface_configure {
>> + struct wl_list link; /* weston_desktop_xdg_surface::configure_list */
>> + uint32_t serial;
>> + struct weston_desktop_xdg_surface_state state;
>> + struct weston_size size;
>> +};
>> +
>> struct weston_desktop_xdg_surface {
>> struct wl_resource *resource;
>> struct weston_desktop_surface *surface;
>> @@ -53,7 +60,7 @@ struct weston_desktop_xdg_surface {
>> bool added;
>> struct wl_event_source *add_idle;
>> struct wl_event_source *configure_idle;
>> - uint32_t configure_serial;
>> + struct wl_list configure_list; /* weston_desktop_xdg_surface_configure::link */
>> struct {
>> struct weston_desktop_xdg_surface_state state;
>> struct weston_size size;
>> @@ -94,13 +101,26 @@ static void
>> weston_desktop_xdg_surface_send_configure(void *data)
>> {
>> struct weston_desktop_xdg_surface *surface = data;
>> + struct weston_desktop_xdg_surface_configure *configure;
>> uint32_t *s;
>> struct wl_array states;
>>
>> surface->configure_idle = NULL;
>>
>> - surface->configure_serial =
>> + configure = zalloc(sizeof(struct weston_desktop_xdg_surface_configure));
>> + if (configure == NULL) {
>> + struct weston_desktop_client *client =
>> + weston_desktop_surface_get_client(surface->surface);
>> + struct wl_client *wl_client =
>> + weston_desktop_client_get_client(client);
>> + wl_client_post_no_memory(wl_client);
>> + return;
>> + }
>> + wl_list_insert(surface->configure_list.prev, &configure->link);
>> + configure->serial =
>> wl_display_next_serial(weston_desktop_get_display(surface->desktop));
>> + configure->state = surface->pending.state;
>> + configure->size = surface->pending.size;
>>
>> wl_array_init(&states);
>> if (surface->pending.state.maximized) {
>> @@ -124,7 +144,7 @@ weston_desktop_xdg_surface_send_configure(void *data)
>> surface->pending.size.width,
>> surface->pending.size.height,
>> &states,
>> - surface->configure_serial);
>> + configure->serial);
>>
>> wl_array_release(&states);
>> };
>> @@ -325,6 +345,7 @@ weston_desktop_xdg_surface_destroy(struct weston_desktop_surface *dsurface,
>> void *user_data)
>> {
>> struct weston_desktop_xdg_surface *surface = user_data;
>> + struct weston_desktop_xdg_surface_configure *configure, *temp;
>>
>> if (surface->added)
>> weston_desktop_api_surface_removed(surface->desktop,
>> @@ -336,6 +357,9 @@ weston_desktop_xdg_surface_destroy(struct weston_desktop_surface *dsurface,
>> if (surface->configure_idle != NULL)
>> wl_event_source_remove(surface->configure_idle);
>>
>> + wl_list_for_each_safe(configure, temp, &surface->configure_list, link)
>> + free(configure);
>> +
>> free(surface);
>> }
>>
>> @@ -443,12 +467,34 @@ weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>> wl_resource_get_user_data(resource);
>> struct weston_desktop_xdg_surface *surface =
>> weston_desktop_surface_get_implementation_data(dsurface);
>> -
>> - if (surface->configure_serial != serial)
>> + struct weston_desktop_xdg_surface_configure *configure, *temp;
>> + bool found = false;
>> +
>> + wl_list_for_each_safe(configure, temp, &surface->configure_list, link) {
>> + if (configure->serial < serial) {
>> + wl_list_remove(&configure->link);
>> + free(configure);
>> + } else if (configure->serial == serial) {
>> + wl_list_remove(&configure->link);
>> + found = true;
>> + }
>> + break;
>> + }
>> + if (!found) {
>> + struct weston_desktop_client *client =
>> + weston_desktop_surface_get_client(dsurface);
>> + struct wl_resource *client_resource =
>> + weston_desktop_client_get_resource(client);
>> + wl_resource_post_error(client_resource,
>> + XDG_SHELL_ERROR_DEFUNCT_SURFACES,
>> + "Wrong configure serial: %u", serial);
>> return;
>> + }
>> +
>> + surface->next.state = configure->state;
>> + surface->next.size = configure->size;
>>
>> - surface->next.state = surface->pending.state;
>> - surface->next.size = surface->pending.size;
>> + free(configure);
>> }
>>
>> static void
>> diff --git a/libweston-desktop/xdg-shell-v6.c b/libweston-desktop/xdg-shell-v6.c
>> index f5e46daa..de5d3e05 100644
>> --- a/libweston-desktop/xdg-shell-v6.c
>> +++ b/libweston-desktop/xdg-shell-v6.c
>> @@ -69,7 +69,7 @@ struct weston_desktop_xdg_surface {
>> struct weston_desktop_surface *desktop_surface;
>> bool configured;
>> struct wl_event_source *configure_idle;
>> - uint32_t configure_serial;
>> + struct wl_list configure_list; /* weston_desktop_xdg_surface_configure::link */
>>
>> bool has_next_geometry;
>> struct weston_geometry next_geometry;
>> @@ -77,6 +77,11 @@ struct weston_desktop_xdg_surface {
>> enum weston_desktop_xdg_surface_role role;
>> };
>>
>> +struct weston_desktop_xdg_surface_configure {
>> + struct wl_list link; /* weston_desktop_xdg_surface::configure_list */
>> + uint32_t serial;
>> +};
>> +
>> struct weston_desktop_xdg_toplevel_state {
>> bool maximized;
>> bool fullscreen;
>> @@ -84,6 +89,12 @@ struct weston_desktop_xdg_toplevel_state {
>> bool activated;
>> };
>>
>> +struct weston_desktop_xdg_toplevel_configure {
>> + struct weston_desktop_xdg_surface_configure base;
>> + struct weston_desktop_xdg_toplevel_state state;
>> + struct weston_size size;
>> +};
>> +
>> struct weston_desktop_xdg_toplevel {
>> struct weston_desktop_xdg_surface base;
>>
>> @@ -115,6 +126,7 @@ struct weston_desktop_xdg_popup {
>> };
>>
>> #define weston_desktop_surface_role_biggest_size (sizeof(struct weston_desktop_xdg_toplevel))
>> +#define weston_desktop_surface_configure_biggest_size (sizeof(struct weston_desktop_xdg_toplevel))
>>
>>
>> static struct weston_geometry
>> @@ -422,10 +434,11 @@ weston_desktop_xdg_toplevel_protocol_resize(struct wl_client *wl_client,
>> }
>>
>> static void
>> -weston_desktop_xdg_toplevel_ack_configure(struct weston_desktop_xdg_toplevel *toplevel)
>> +weston_desktop_xdg_toplevel_ack_configure(struct weston_desktop_xdg_toplevel *toplevel,
>> + struct weston_desktop_xdg_toplevel_configure *configure)
>> {
>> - toplevel->next.state = toplevel->pending.state;
>> - toplevel->next.size = toplevel->pending.size;
>> + toplevel->next.state = configure->state;
>> + toplevel->next.size = configure->size;
>> }
>>
>> static void
>> @@ -529,11 +542,15 @@ weston_desktop_xdg_toplevel_protocol_set_minimized(struct wl_client *wl_client,
>> }
>>
>> static void
>> -weston_desktop_xdg_toplevel_send_configure(struct weston_desktop_xdg_toplevel *toplevel)
>> +weston_desktop_xdg_toplevel_send_configure(struct weston_desktop_xdg_toplevel *toplevel,
>> + struct weston_desktop_xdg_toplevel_configure *configure)
>> {
>> uint32_t *s;
>> struct wl_array states;
>>
>> + configure->state = toplevel->pending.state;
>> + configure->size = toplevel->pending.size;
>> +
>> wl_array_init(&states);
>> if (toplevel->pending.state.maximized) {
>> s = wl_array_add(&states, sizeof(uint32_t));
>> @@ -848,9 +865,21 @@ static void
>> weston_desktop_xdg_surface_send_configure(void *user_data)
>> {
>> struct weston_desktop_xdg_surface *surface = user_data;
>> + struct weston_desktop_xdg_surface_configure *configure;
>>
>> surface->configure_idle = NULL;
>> - surface->configure_serial =
>> +
>> + configure = zalloc(weston_desktop_surface_configure_biggest_size);
>> + if (configure == NULL) {
>> + struct weston_desktop_client *client =
>> + weston_desktop_surface_get_client(surface->desktop_surface);
>> + struct wl_client *wl_client =
>> + weston_desktop_client_get_client(client);
>> + wl_client_post_no_memory(wl_client);
>> + return;
>> + }
>> + wl_list_insert(surface->configure_list.prev, &configure->link);
>> + configure->serial =
>> wl_display_next_serial(weston_desktop_get_display(surface->desktop));
>>
>> switch (surface->role) {
>> @@ -858,14 +887,15 @@ weston_desktop_xdg_surface_send_configure(void *user_data)
>> assert(0 && "not reached");
>> break;
>> case WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL:
>> - weston_desktop_xdg_toplevel_send_configure((struct weston_desktop_xdg_toplevel *) surface);
>> + weston_desktop_xdg_toplevel_send_configure((struct weston_desktop_xdg_toplevel *) surface,
>> + (struct weston_desktop_xdg_toplevel_configure *) configure);
>> break;
>> case WESTON_DESKTOP_XDG_SURFACE_ROLE_POPUP:
>> weston_desktop_xdg_popup_send_configure((struct weston_desktop_xdg_popup *) surface);
>> break;
>> }
>>
>> - zxdg_surface_v6_send_configure(surface->resource, surface->configure_serial);
>> + zxdg_surface_v6_send_configure(surface->resource, configure->serial);
>> }
>>
>> static bool
>> @@ -1060,12 +1090,32 @@ weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>> wl_resource_get_user_data(resource);
>> struct weston_desktop_xdg_surface *surface =
>> weston_desktop_surface_get_implementation_data(dsurface);
>> + struct weston_desktop_xdg_surface_configure *configure, *temp;
>> + bool found = false;
>>
>> if (!weston_desktop_xdg_surface_check_role(surface))
>> return;
>>
>> - if (surface->configure_serial != serial)
>> + wl_list_for_each_safe(configure, temp, &surface->configure_list, link) {
>> + if (configure->serial < serial) {
>> + wl_list_remove(&configure->link);
>> + free(configure);
>> + } else if (configure->serial == serial) {
>> + wl_list_remove(&configure->link);
>> + found = true;
>> + }
>> + break;
>> + }
>> + if (!found) {
>> + struct weston_desktop_client *client =
>> + weston_desktop_surface_get_client(dsurface);
>> + struct wl_resource *client_resource =
>> + weston_desktop_client_get_resource(client);
>> + wl_resource_post_error(client_resource,
>> + ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE,
>> + "Wrong configure serial: %u", serial);
>> return;
>> + }
>>
>> surface->configured = true;
>>
>> @@ -1074,11 +1124,14 @@ weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>> assert(0 && "not reached");
>> break;
>> case WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL:
>> - weston_desktop_xdg_toplevel_ack_configure((struct weston_desktop_xdg_toplevel *) surface);
>> + weston_desktop_xdg_toplevel_ack_configure((struct weston_desktop_xdg_toplevel *) surface,
>> + (struct weston_desktop_xdg_toplevel_configure *) configure);
>> break;
>> case WESTON_DESKTOP_XDG_SURFACE_ROLE_POPUP:
>> break;
>> }
>> +
>> + free(configure);
>> }
>>
>> static void
>> @@ -1153,6 +1206,7 @@ weston_desktop_xdg_surface_destroy(struct weston_desktop_surface *dsurface,
>> void *user_data)
>> {
>> struct weston_desktop_xdg_surface *surface = user_data;
>> + struct weston_desktop_xdg_surface_configure *configure, *temp;
>>
>> switch (surface->role) {
>> case WESTON_DESKTOP_XDG_SURFACE_ROLE_NONE:
>> @@ -1168,6 +1222,9 @@ weston_desktop_xdg_surface_destroy(struct weston_desktop_surface *dsurface,
>> if (surface->configure_idle != NULL)
>> wl_event_source_remove(surface->configure_idle);
>>
>> + wl_list_for_each_safe(configure, temp, &surface->configure_list, link)
>> + free(configure);
>> +
>> free(surface);
>> }
>>
>> @@ -1290,6 +1347,8 @@ weston_desktop_xdg_shell_protocol_get_xdg_surface(struct wl_client *wl_client,
>> "xdg_surface must not have a buffer at creation");
>> return;
>> }
>> +
>> + wl_list_init(&surface->configure_list);
>> }
>>
>> static void
>> --
>> 2.13.3
>>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
--
Quentin “Sardem FF7” Glidic
More information about the wayland-devel
mailing list