[Spice-devel] [PATCH linux/vd-agent 10/11] clipboard: only send release when no immediate grab

[Frediano Ziglio]

> 
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> Do not send a release event between two grabs, this helps with window
> manager interaction issues on peer side.
> 

I would explain which kind of issue this is supposed to fix.

> Advertise this behaviour via a capability introduced in spice-protocol
> 0.12.16, so the client doesn't need to do some time-based filtering.
> 
> (the capability shouldn't need to be negotiated, a client shouldn't
> expect a release between two grabs)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>

I suppose Jakub/Victor could do a much better review/test than me.

> ---
>  src/vdagent/clipboard.c | 12 ++++++------
>  src/vdagent/x11.c       |  7 +++----
>  src/vdagentd/vdagentd.c |  1 +
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/vdagent/clipboard.c b/src/vdagent/clipboard.c
> index 9fb87e3..097c6ee 100644
> --- a/src/vdagent/clipboard.c
> +++ b/src/vdagent/clipboard.c
> @@ -242,13 +242,13 @@ static void clipboard_owner_change_cb(GtkClipboard
> *clipboard,
>          return;
>      }
>  
> -    if (sel->owner == OWNER_GUEST) {
> -        clipboard_new_owner(c, sel_id, OWNER_NONE);
> -        udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel_id, 0, NULL,
> 0);
> -    }
> -
> -    if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER)
> +    if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) {
> +        if (sel->owner == OWNER_GUEST) {
> +            clipboard_new_owner(c, sel_id, OWNER_NONE);
> +            udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel_id, 0,
> NULL, 0);
> +        }
>          return;
> +    }
>  
>      /* if there's a pending request for clipboard targets, cancel it */
>      if (sel->last_targets_req)
> diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> index c2515a8..9409b53 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -530,11 +530,10 @@ static void vdagent_x11_handle_event(struct vdagent_x11
> *x11, XEvent event)
>          if (ev.xfev.owner == x11->selection_window)
>              return;
>  
> -        /* If the clipboard owner is changed we no longer own it */

Why not keeping this comment?

> -        vdagent_x11_set_clipboard_owner(x11, selection, owner_none);
> -
> -        if (ev.xfev.owner == None)
> +        if (ev.xfev.owner == None) {
> +            vdagent_x11_set_clipboard_owner(x11, selection, owner_none);
>              return;
> +        }
>  
>          /* Request the supported targets from the new owner */
>          XConvertSelection(x11->display, ev.xfev.selection,
>          x11->targets_atom,
> diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> index 72a3e13..683e5d3 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -133,6 +133,7 @@ static void send_capabilities(struct vdagent_virtio_port
> *vport,
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MAX_CLIPBOARD);
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_AUDIO_VOLUME_SYNC);
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_GRAPHICS_DEVICE_INFO);
> +    VD_AGENT_SET_CAPABILITY(caps->caps,
> VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB);
>      virtio_msg_uint32_to_le((uint8_t *)caps, size, 0);
>  
>      vdagent_virtio_port_write(vport, VDP_CLIENT_PORT,

Frediano