<div dir="ltr">The issue isn't "there's already a role". The issue is that we don't allow two xdg_surface proxies for the same surface (a currently-undocumented but sane limitation). For pretty much any other surface role, allowing the surface to re-gain its role is a feature, not a bug. </div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Feb 5, 2015 at 1:46 PM, Bill Spitzak <<a href="mailto:spitzak@gmail.com" target="_blank">spitzak@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">How about a wrapper function around weston_surface_set_role that always fails if there is a non-zero role, and then call that? You can then gradually transition all the other calls to this new one, and if they are all changed then remove the old function.<div class="HOEnZb"><div class="h5"> On 02/05/2015 05:21 AM, Jonas Ådahl wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> If a client calls xdg_shell.get_xdg_surface on a surface that is already an xdg_surface wold, prior to this patch, succeed, but cause weston to crash later when trying to configure. This patch instead sends a role error to the client complaining that it already is an xdg_surface. Note that .._set_role() only fails when changing roles, not when setting the same role twice. The same is done for xdg_popup. Signed-off-by: Jonas Ådahl <<a href="mailto:jadahl@gmail.com" target="_blank">jadahl@gmail.com</a>> Reviewed-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>> --- Changed since v1: Put the assignments on their own lines. desktop-shell/shell.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index a7514f7..d4407d0 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -3959,6 +3959,14 @@ xdg_get_xdg_surface(struct wl_client *client, struct desktop_shell *shell = sc->shell; struct shell_surface *shsurf; + shsurf = get_shell_surface(surface); + if (shsurf && shell_surface_is_xdg_surface(shsurf)) { + wl_resource_post_error(resource, XDG_SHELL_ERROR_ROLE, + "This wl_surface is already an " + "xdg_surface"); + return; + } + if (weston_surface_set_role(surface, "xdg_surface", resource, XDG_SHELL_ERROR_ROLE) < 0) return; @@ -4052,6 +4060,14 @@ xdg_get_xdg_popup(struct wl_client *client, struct weston_surface *parent; struct shell_seat *seat; + shsurf = get_shell_surface(surface); + if (shsurf && shell_surface_is_xdg_popup(shsurf)) { + wl_resource_post_error(resource, XDG_SHELL_ERROR_ROLE, + "This wl_surface is already an " + "xdg_popup"); + return; + } + if (weston_surface_set_role(surface, "xdg_popup", resource, XDG_SHELL_ERROR_ROLE) < 0) return; </blockquote></div></div><div class="HOEnZb"><div class="h5"> _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </div></div></blockquote></div> -- <div class="gmail_signature"> Jasper </div> </div>