[PATCH weston v2 3/7] libweston: Make weston_seat release safe
Quentin Glidic
sardemff7+wayland at sardemff7.net
Tue Feb 13 12:21:58 UTC 2018
On 2/13/18 11:58 AM, Pekka Paalanen wrote:
> On Thu, 8 Feb 2018 15:37:54 +0200
> Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:
>
>> Ensure the server can safely handle client requests for wl_seat resource
>> that have become inert due to weston_seat object release and subsequent
>> destruction.
>>
>> The clean-up involves, among other things, unsetting the destroyed
>> weston_seat object from the user data of wl_seat resources, and handling
>> this NULL user data case where required.
>>
>> The list of sites extracting and using weston_seat object from wl_seat
>> resources which were audited for this patch are:
>>
>> Legend:
>> N/A = Not Applicable (not implemented by weston)
>> FIXED = Fixed in the commit
>> OK = Already works correctly
>>
>> == keyboard_shortcuts_inhibit_unstable_v1 ==
>> [N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
>> == tablet_input_unstable_v{1,2} ==
>> [N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
>> == text_input_unstable_v1 ==
>> [FIXED] zwp_text_input_v1.activate
>> [FIXED] zwp_text_input_v1.deactivate
>> == wl_data_device ==
>> [FIXED] wl_data_device_manager.get_data_device
>> [OK] wl_data_device.start_drag
>> [FIXED] wl_data_device.set_selection
>> [OK] wl_data_device.release
>> == wl_shell ==
>> [FIXED] wl_shell_surface.move
>> [FIXED] wl_shell_surface.resize
>> [FIXED] wl_shell_surface.set_popup
>> == xdg_shell and xdg_shell_unstable_v6 ==
>> [FIXED] xdg_toplevel.show_window_menu
>> [FIXED] xdg_toplevel.move
>> [FIXED] xdg_toplevel.resize
>> [FIXED] xdg_popup.grab
>> == xdg_shell_unstable_v5 ==
>> [FIXED] xdg_shell.get_xdg_popup
>> [FIXED] xdg_surface.show_window_menu
>> [FIXED] xdg_surface.move
>> [FIXED] xdg_surface.resize
>>
>> Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
>> ---
>> Changes in v2:
>> - Properly create inert resources in seat_get_pointer/touch/keyboard.
>> - Ensure all sites which have a wl_seat input resource can deal
>> with inert resources.
>>
>> compositor/text-backend.c | 8 ++++--
>> libweston-desktop/wl-shell.c | 12 +++++++-
>> libweston-desktop/xdg-shell-v5.c | 16 ++++++++++-
>> libweston-desktop/xdg-shell-v6.c | 18 +++++++++++-
>> libweston/data-device.c | 15 ++++++----
>> libweston/input.c | 61 ++++++++++++++++++++++++++++------------
>> 6 files changed, 102 insertions(+), 28 deletions(-)
>
> Hi Alf,
>
> awesome work!
>
> The changes look good to me, however I did not review the
> libweston-desktop bits yet. I'm hoping Quentin would take a look at
> those.
>
> Therefore, apart from libweston-desktop bits, this patch is:
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
>
> Thanks,
> pq
>
>> diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
>> index 66553f45..3386d48b 100644
>> --- a/libweston-desktop/wl-shell.c
>> +++ b/libweston-desktop/wl-shell.c
>> @@ -220,6 +220,9 @@ weston_desktop_wl_shell_surface_protocol_move(struct wl_client *wl_client,
>> struct weston_desktop_wl_shell_surface *surface =
>> weston_desktop_surface_get_implementation_data(dsurface);
>>
>> + if (seat == NULL)
>> + return;
>> +
>> weston_desktop_api_move(surface->desktop, dsurface, seat, serial);
>> }
>>
>> @@ -238,6 +241,9 @@ weston_desktop_wl_shell_surface_protocol_resize(struct wl_client *wl_client,
>> enum weston_desktop_surface_edge surf_edges =
>> (enum weston_desktop_surface_edge) edges;
>>
>> + if (seat == NULL)
>> + return;
>> +
>> weston_desktop_api_resize(surface->desktop, dsurface, seat, serial, surf_edges);
>> }
>>
>> @@ -321,13 +327,17 @@ weston_desktop_wl_shell_surface_protocol_set_popup(struct wl_client *wl_client,
>> struct weston_desktop_surface *dsurface =
>> wl_resource_get_user_data(resource);
>> struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
>> - struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
>> + struct weston_desktop_seat *seat;
>> struct weston_surface *parent =
>> wl_resource_get_user_data(parent_resource);
>> struct weston_desktop_surface *parent_surface;
>> struct weston_desktop_wl_shell_surface *surface =
>> weston_desktop_surface_get_implementation_data(dsurface);
>>
>> + if (wseat == NULL)
>> + return;
See comment on v6.
>> +
>> + seat = weston_desktop_seat_from_seat(wseat);
>> if (seat == NULL) {
>> wl_client_post_no_memory(wl_client);
>> return;
>> diff --git a/libweston-desktop/xdg-shell-v5.c b/libweston-desktop/xdg-shell-v5.c
>> index ebe7940e..ac3de78a 100644
>> --- a/libweston-desktop/xdg-shell-v5.c
>> +++ b/libweston-desktop/xdg-shell-v5.c
>> @@ -421,6 +421,9 @@ weston_desktop_xdg_surface_protocol_show_window_menu(struct wl_client *wl_client
>> struct weston_desktop_xdg_surface *surface =
>> weston_desktop_surface_get_implementation_data(dsurface);
>>
>> + if (seat == NULL)
>> + return;
>> +
>> weston_desktop_xdg_surface_ensure_added(surface);
>> weston_desktop_api_show_window_menu(surface->desktop, dsurface, seat, x, y);
>> }
>> @@ -438,6 +441,9 @@ weston_desktop_xdg_surface_protocol_move(struct wl_client *wl_client,
>> struct weston_desktop_xdg_surface *surface =
>> weston_desktop_surface_get_implementation_data(dsurface);
>>
>> + if (seat == NULL)
>> + return;
>> +
>> weston_desktop_xdg_surface_ensure_added(surface);
>> weston_desktop_api_move(surface->desktop, dsurface, seat, serial);
>> }
>> @@ -458,6 +464,9 @@ weston_desktop_xdg_surface_protocol_resize(struct wl_client *wl_client,
>> enum weston_desktop_surface_edge surf_edges =
>> (enum weston_desktop_surface_edge) edges;
>>
>> + if (seat == NULL)
>> + return;
>> +
>> weston_desktop_xdg_surface_ensure_added(surface);
>> weston_desktop_api_resize(surface->desktop, dsurface, seat, serial, surf_edges);
>> }
>> @@ -766,11 +775,16 @@ weston_desktop_xdg_shell_protocol_get_xdg_popup(struct wl_client *wl_client,
>> struct weston_surface *wparent =
>> wl_resource_get_user_data(parent_resource);
>> struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
>> - struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
>> + struct weston_desktop_seat *seat;
>> struct weston_desktop_surface *parent, *topmost;
>> bool parent_is_popup, parent_is_xdg;
>> struct weston_desktop_xdg_popup *popup;
>>
>> + if (wseat == NULL)
>> + return;
See comment on v6.
>> +
>> + seat = weston_desktop_seat_from_seat(wseat);
>> +
>> if (weston_surface_set_role(wsurface, "xdg_popup", resource, XDG_SHELL_ERROR_ROLE) < 0)
>> return;
>>
>> diff --git a/libweston-desktop/xdg-shell-v6.c b/libweston-desktop/xdg-shell-v6.c
>> index 4db3748b..3bf15e7a 100644
>> --- a/libweston-desktop/xdg-shell-v6.c
>> +++ b/libweston-desktop/xdg-shell-v6.c
>> @@ -371,6 +371,9 @@ weston_desktop_xdg_toplevel_protocol_show_window_menu(struct wl_client *wl_clien
>> struct weston_desktop_xdg_toplevel *toplevel =
>> weston_desktop_surface_get_implementation_data(dsurface);
>>
>> + if (seat == NULL)
>> + return;
>> +
>> if (!toplevel->base.configured) {
>> wl_resource_post_error(toplevel->resource,
>> ZXDG_SURFACE_V6_ERROR_NOT_CONSTRUCTED,
>> @@ -395,6 +398,9 @@ weston_desktop_xdg_toplevel_protocol_move(struct wl_client *wl_client,
>> struct weston_desktop_xdg_toplevel *toplevel =
>> weston_desktop_surface_get_implementation_data(dsurface);
>>
>> + if (seat == NULL)
>> + return;
>> +
>> if (!toplevel->base.configured) {
>> wl_resource_post_error(toplevel->resource,
>> ZXDG_SURFACE_V6_ERROR_NOT_CONSTRUCTED,
>> @@ -421,6 +427,9 @@ weston_desktop_xdg_toplevel_protocol_resize(struct wl_client *wl_client,
>> enum weston_desktop_surface_edge surf_edges =
>> (enum weston_desktop_surface_edge) edges;
>>
>> + if (seat == NULL)
>> + return;
>> +
>> if (!toplevel->base.configured) {
>> wl_resource_post_error(toplevel->resource,
>> ZXDG_SURFACE_V6_ERROR_NOT_CONSTRUCTED,
>> @@ -757,11 +766,18 @@ weston_desktop_xdg_popup_protocol_grab(struct wl_client *wl_client,
>> struct weston_desktop_xdg_popup *popup =
>> weston_desktop_surface_get_implementation_data(dsurface);
>> struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
>> - struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
>> + struct weston_desktop_seat *seat;
>> struct weston_desktop_surface *topmost;
>> bool parent_is_toplevel =
>> popup->parent->role == WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL;
>>
>> + if (wseat == NULL) {
>> + weston_desktop_surface_popup_dismiss(popup->base.desktop_surface);
This is good.
You need to do the same for v5 and wl_shell, otherwise the client will
be out of sync. Specifically for v5 where it creates an xdg_popup object
while this patch would not.
With that fixed (or v5 just left as unmaintained/unused), the
libweston-desktop stuff is:
Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>
Thanks,
>> + return;
>> + }
>> +
>> + seat = weston_desktop_seat_from_seat(wseat);
>> +
>> if (popup->committed) {
>> wl_resource_post_error(popup->resource,
>> ZXDG_POPUP_V6_ERROR_INVALID_GRAB,
--
Quentin “Sardem FF7” Glidic
More information about the wayland-devel
mailing list