[Spice-devel] [spice-gtk v3 2/3] gtk-session: clipboard: x11: owner-change improvement

[Marc-André Lureau]

On Thu, Jan 10, 2019 at 4:47 PM Victor Toso <victortoso at redhat.com> wrote:
>
> From: Victor Toso <me at victortoso.com>
>
> While interacting within the guest the VD_AGENT_CLIPBOARD_GRAB message
> will be sent by the guest agent to notify that agent is holding some
> clipboard data. When this clipboard data changes, the agent will send
> VD_AGENT_CLIPBOARD_RELEASE to simply notify that previous clipboard
> data is not available anymore and a new VD_AGENT_CLIPBOARD_GRAB
> follows up with the current clipboard data being hold by agent.
>
> This patch helps in fixing a state race, in X11, between who is
> 'holding' the clipboard grab.
>
> The bug happens because gtk_clipboard_clear() will set owner to none,
> making the rest of the code path consider that clipboard data has
> changed in the *client* and ends up requesting the metadata with
> gtk_clipboard_request_targets(), handled by clipboard_get_targets().
>
> It is possible to have clipboard_get_targets() being called between
> one VD_AGENT_CLIPBOARD_RELEASE and other VD_AGENT_CLIPBOARD_GRAB.
>
> Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
>
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> Tested-by: James Harvey @jamespharvey20
> ---
>  src/spice-gtk-session.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index adc72a2..85d5880 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -27,6 +27,9 @@
>  #include <X11/Xlib.h>
>  #include <gdk/gdkx.h>
>  #endif
> +#ifdef GDK_WINDOWING_WAYLAND
> +#include <gdk/gdkwayland.h>
> +#endif
>  #ifdef G_OS_WIN32
>  #include <windows.h>
>  #include <gdk/gdkwin32.h>
> @@ -674,6 +677,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
>          return;
>      }
>
> +#ifdef GDK_WINDOWING_X11
> +    /* In X11, while holding the keyboard-grab we are not interested in this
> +     * event as it either came by grab or release messages from agent.  */
> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> +        spice_gtk_session_get_keyboard_has_focus(self)) {
> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> +                    "while holding focus");

Couldn't this race happen without keyboard focus?

> +        return;
> +    }
> +#endif
> +    SPICE_DEBUG("clipboard: owner-changed event: has-foucus=%d",
> +                spice_gtk_session_get_keyboard_has_focus(self));

"foucus" spelling

> +
>      s->clipboard_by_guest[selection] = FALSE;
>      s->clip_hasdata[selection] = TRUE;
>      if (s->auto_clipboard_enable && !read_only(self))
> --
> 2.20.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau