[PATCH weston v2] desktop-shell: Fail if get_xdg_surface is called on an xdg_surface

[Jasper St. Pierre]

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.

On Thu, Feb 5, 2015 at 1:46 PM, Bill Spitzak <spitzak at gmail.com> wrote:

> 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.
>
>
> On 02/05/2015 05:21 AM, 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
>> 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 <jadahl at gmail.com>
>> Reviewed-by: Daniel Stone <daniels at collabora.com>
>> ---
>>
>> 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;
>>
>>  _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>



-- 
  Jasper
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150205/f598617e/attachment.html>