On Thu, Feb 05, 2015 at 04:58:05PM +0800, Jonas Ådahl wrote:
> 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

s/wold/would/

> 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 <[email protected]>

I'll ditto daniels' comments, so with those fixed, otherwise LGTM:

Reviewed-by: Bryce Harrington <[email protected]>

> ---
>  desktop-shell/shell.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index a7514f7..cc345b5 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;
>  
> +     if ((shsurf = get_shell_surface(surface)) &&
> +         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;
>  
> +     if ((shsurf = get_shell_surface(surface)) &&
> +         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;
> -- 
> 2.1.0
> 
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to