[PATCH] shell: Destroy the shell_surface when the xdg_shell interface gets destroyed

[Sjoerd Simons]

On Wed, 2014-06-04 at 13:44 +0200, Sjoerd Simons wrote:
> When running gtk3-demo under weston opening comboboxes a second time
> causes the program to bail due to weston returning an error. The
> relevant client trace in this case is:
>  -> xdg_shell at 15.get_xdg_popup(new id xdg_popup at 12, wl_surface at 28, ....
>  -> xdg_popup at 12.destroy()
>  -> xdg_shell at 15.get_xdg_popup(new id xdg_popup at 19, wl_surface at 28, ....
>   wl_display at 1.error(wl_surface at 28, 0,
>     "xdg_shell::get_xdg_popup already requested")
> 
> However, according to the xdg_shell protocol this should be valid as
> calling destroy should remove the xdg_popup interface from the
> wl_surface object.
> 
> Fix this by ensuring the internal shell surface also gets destroyed when
> the relevant xdg interface is destroyed and not just when the underlying
> wl_surface gets destroyed.
> 
> This patch applies both to Weston 1.5 and master, please apply to both
> if correct.

Turns out this patch is, in fact, not correct. The desktop-shell code
seems to have a bigger assumption on the shell surface having a lifetime
bound to the underlying wl_surface then i first thought. The patch
causes weston to crash if the fade-out animation is running when the xdg
shell surface is destroyed.

One solution here is to rework the code so the shell surface can be
destroyed & recreated seperately. Another one would be to get xdg_shell
more in line with wl_shell when it comes to life-time rules. 

I've put together an initial patch for GTK+ such that it doesn't try to
re-create xdg_shell surfaces anymore and essentially assumes the xdg
surfaces are once-only per wl_surface:
  https://bugzilla.gnome.org/show_bug.cgi?id=727021#c3



> ---
>  desktop-shell/shell.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 46c6e18..45a3321 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -3153,6 +3153,7 @@ destroy_shell_surface(struct shell_surface *shsurf)
>  	/* As destroy_resource() use wl_list_for_each_safe(),
>  	 * we can always remove the listener.
>  	 */
> +	wl_list_remove(&shsurf->resource_destroy_listener.link);
>  	wl_list_remove(&shsurf->surface_destroy_listener.link);
>  	shsurf->surface->configure = NULL;
>  	free(shsurf->title);
> @@ -3175,6 +3176,7 @@ shell_destroy_shell_surface(struct wl_resource *resource)
>  	if (!wl_list_empty(&shsurf->popup.grab_link))
>  		remove_popup_grab(shsurf);
>  	shsurf->resource = NULL;
> +	destroy_shell_surface(shsurf);
>  }
>  
>  static void


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140612/6518d75e/attachment-0001.bin>