2016-12-08 16:46 GMT+01:00 Quentin Glidic <[email protected]>: > On 08/12/2016 16:20, Giulio Camuffo wrote: >> >> When sending a ping event to a surface using the wl_shell interface, >> if that surface is destroyed before we receive the pong we will never >> receive it, even if the client is actually responsive, since the >> interface does not exist anymore. So when the surface if destroyed >> pretend it's a pong and reset the ping state. >> >> Signed-off-by: Giulio Camuffo <[email protected]> > > > I made a few renames to match the current code. > I was going to push it, but I found a last issue, see below. > > >> --- >> >> v3: store the ping serial in the surface instead of the client wrapper >> v4: removed leftover change >> >> libweston-desktop/wl-shell.c | 22 +++++++++++++++++++++- >> 1 file changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c >> index 399139c..f75b022 100644 >> --- a/libweston-desktop/wl-shell.c >> +++ b/libweston-desktop/wl-shell.c >> @@ -57,6 +57,7 @@ struct weston_desktop_wl_shell_surface { >> struct weston_desktop_seat *popup_seat; >> enum weston_desktop_wl_shell_surface_state state; >> struct wl_listener wl_surface_resource_destroy_listener; >> + uint32_t ping_serial; >> }; >> >> static void >> @@ -112,6 +113,7 @@ weston_desktop_wl_shell_surface_ping(struct >> weston_desktop_surface *dsurface, >> { >> struct weston_desktop_wl_shell_surface *surface = user_data; >> >> + surface->ping_serial = serial; >> wl_shell_surface_send_ping(surface->resource, serial); >> } >> >> @@ -181,11 +183,27 @@ weston_desktop_wl_shell_change_state(struct >> weston_desktop_wl_shell_surface *sur >> } >> >> static void >> +pong_client(struct weston_desktop_wl_shell_surface *surface) > > > weston_desktop_wl_shell_surface_pong_client > >> +{ >> + struct weston_desktop_client *client = >> + weston_desktop_surface_get_client(surface->surface); >> + >> + weston_desktop_client_pong(client, surface->ping_serial); >> + surface->ping_serial = 0; >> +} >> + >> +static void >> weston_desktop_wl_shell_surface_destroy(struct weston_desktop_surface >> *dsurface, >> void *user_data) >> { >> struct weston_desktop_wl_shell_surface *surface = user_data; >> >> + /* If the surface being destroyed was the one that was pinged >> before >> + * we need to fake a pong here, because it cannot answer the ping >> anymore, >> + * even if the client is responsive. */ >> + if (surface->ping_serial != 0) >> + pong_client(surface); >> + >> >> wl_list_remove(&surface->wl_surface_resource_destroy_listener.link); >> >> weston_desktop_wl_shell_surface_maybe_ungrab(surface); >> @@ -203,8 +221,10 @@ weston_desktop_wl_shell_surface_protocol_pong(struct >> wl_client *wl_client, >> uint32_t serial) >> { >> struct weston_desktop_surface *surface = >> wl_resource_get_user_data(resource); > > > dsurface > >> + struct weston_desktop_wl_shell_surface *wls = >> + weston_desktop_surface_get_implementation_data(surface); > > > surface > >> - >> weston_desktop_client_pong(weston_desktop_surface_get_client(surface), >> serial); >> + pong_client(wls); > > > We should check that surface->ping_serial == serial somehow. > Maybe it is safe to ignore serial for now, as it fixes something, then add a > check in a follow-up commit.
Well, but weston_desktop_client_pong() already checks that, so i don't think we need to check it here too. > > Thoughts? Pekka (aka assertman)? > > Cheers, > > >> } >> >> static void >> > > > -- > > Quentin “Sardem FF7” Glidic _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
