[PATCH weston 02/17] xdg-shell: Require proper object tree destruction

[Pekka Paalanen]

On Tue,  7 Apr 2015 17:01:17 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> Require all child objects to be destroyed before the parent. In other
> words, all popups and surfaces created by one xdg_shell instance needs
> to be destroyed before the xdg_shell object, otherwise a protocol error
> is raised.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>  desktop-shell/shell.c  | 26 ++++++++++++++++++++++++++
>  protocol/xdg-shell.xml |  6 +++---
>  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 826692f..778c120 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -107,6 +107,7 @@ struct shell_surface {
>  	struct wl_resource *resource;
>  	struct wl_signal destroy_signal;
>  	struct shell_client *owner;
> +	struct wl_resource *owner_resource;
>  
>  	struct weston_surface *surface;
>  	struct weston_view *view;
> @@ -239,6 +240,7 @@ struct shell_client {
>  	struct wl_event_source *ping_timer;
>  	uint32_t ping_serial;
>  	int unresponsive;
> +	struct wl_list surface_list;
>  };
>  
>  static struct desktop_shell *
> @@ -3554,6 +3556,7 @@ shell_destroy_shell_surface(struct wl_resource *resource)
>  
>  	if (!wl_list_empty(&shsurf->popup.grab_link))
>  		remove_popup_grab(shsurf);
> +	wl_list_remove(wl_resource_get_link(shsurf->resource));
>  	shsurf->resource = NULL;
>  }
>  
> @@ -3725,6 +3728,7 @@ shell_get_shell_surface(struct wl_client *client,
>  	wl_resource_set_implementation(shsurf->resource,
>  				       &shell_surface_implementation,
>  				       shsurf, shell_destroy_shell_surface);
> +	wl_list_init(wl_resource_get_link(shsurf->resource));
>  }
>  
>  static bool
> @@ -3988,6 +3992,21 @@ static void
>  xdg_shell_destroy(struct wl_client *client,
>  		  struct wl_resource *resource)
>  {
> +	struct shell_client *sc = wl_resource_get_user_data(resource);
> +	struct wl_resource *shsurf_resource;
> +	struct shell_surface *shsurf;
> +
> +	wl_resource_for_each(shsurf_resource, &sc->surface_list) {
> +		shsurf = wl_resource_get_user_data(shsurf_resource);
> +		if (shsurf->owner_resource == resource) {
> +			wl_resource_post_error(
> +				resource,
> +				XDG_SHELL_ERROR_DEFUNCT_SURFACES,
> +				"not all child surface objects destroyed");
> +			return;
> +		}
> +	}
> +
>  	wl_resource_destroy(resource);
>  }
>  
> @@ -4056,6 +4075,9 @@ xdg_get_xdg_surface(struct wl_client *client,
>  	wl_resource_set_implementation(shsurf->resource,
>  				       &xdg_surface_implementation,
>  				       shsurf, shell_destroy_shell_surface);
> +	shsurf->owner_resource = resource;
> +	wl_list_insert(&sc->surface_list,
> +		       wl_resource_get_link(shsurf->resource));
>  }
>  
>  static bool
> @@ -4181,6 +4203,9 @@ xdg_get_xdg_popup(struct wl_client *client,
>  	wl_resource_set_implementation(shsurf->resource,
>  				       &xdg_popup_implementation,
>  				       shsurf, shell_destroy_shell_surface);
> +	shsurf->owner_resource = resource;
> +	wl_list_insert(&sc->surface_list,
> +		       wl_resource_get_link(shsurf->resource));
>  }
>  
>  static void
> @@ -5798,6 +5823,7 @@ shell_client_create(struct wl_client *client, struct desktop_shell *shell,
>  	sc->shell = shell;
>  	sc->destroy_listener.notify = handle_shell_client_destroy;
>  	wl_client_add_destroy_listener(client, &sc->destroy_listener);
> +	wl_list_init(&sc->surface_list);
>  
>  	return sc;
>  }
> diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml
> index d1b47d1..09ce019 100644
> --- a/protocol/xdg-shell.xml
> +++ b/protocol/xdg-shell.xml
> @@ -49,6 +49,7 @@
>  
>      <enum name="error">
>        <entry name="role" value="0" summary="given wl_surface has another role"/>
> +      <entry name="defunct_surfaces" value="1" summary="xdg_shell was destroyed before children"/>
>      </enum>
>  
>      <request name="destroy" type="destructor">
> @@ -56,9 +57,8 @@
>          Destroy this xdg_shell object.
>  
>          Destroying a bound xdg_shell object while there are surfaces
> -        still alive with roles from this interface is illegal and will
> -        result in a protocol error. Make sure to destroy all surfaces
> -        before destroying this object.
> +        still alive created by this xdg_shell object instance is illegal
> +        and will result in a protocol error.
>        </description>
>      </request>
>  

Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

FYI, I do not think this patch is reason to bump the experimental
version. If you can build, the protocol is still compatible. An error
code being unknown or not does not really change anything.


Thanks,
pq