Hi On Sun, Mar 24, 2019 at 6:50 PM Jakub Janku <jjanku@xxxxxxxxxx> wrote: > > Hi, > > On Thu, Mar 21, 2019 at 1:21 PM <marcandre.lureau@xxxxxxxxxx> wrote: > > > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > > > > On the client side, whenever the grab owner changes (and the clipboard > > was previously grabbed), spice-gtk sends a clipboard release followed > > immediately by a new grab. But some clipboard managers on the remote > > side react to clipboard release events by taking a clipboard grab, > > presumably to avoid empty clipboards. > > > > The two grabs, coming from the client and from the remote sides, will > > race in both directions, which may confuse the client & remote side, > > as both believe the other side is the current grab owner, and thus > > further clipboard data requests are likely to fail. > > > > Let's avoid sending a release event when re-grabing. > > > > The race described above may still happen in other rare circunstances, > > and will require a protocol change. To avoid the conflict, a discussed > > solution could use a clipboard serial number. > > > > Tested with current linux & windows vdagent. Looking at earlier > > version of the code, it doesn't seem like subsequent grabs will be > > treated as an error. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > > --- > > src/spice-gtk-session.c | 21 ++++++++------------- > > 1 file changed, 8 insertions(+), 13 deletions(-) > > > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c > > index b48f92a..0e748b6 100644 > > --- a/src/spice-gtk-session.c > > +++ b/src/spice-gtk-session.c > > @@ -569,8 +569,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard, > > g_return_if_fail(selection != -1); > > > > if (s->clip_grabbed[selection]) { > > - SPICE_DEBUG("Clipboard is already grabbed, ignoring %d atoms", n_atoms); > > - return; > > + SPICE_DEBUG("Clipboard is already grabbed, re-grab: %d atoms", n_atoms); > > } > > > > /* Set all Atoms that matches our current protocol implementation */ > > @@ -652,18 +651,14 @@ static void clipboard_owner_change(GtkClipboard *clipboard, > > return; > > } > > > > - /* In case we sent a grab to the agent, we need to release it now as > > - * previous clipboard data should not be reachable anymore */ > > - if (s->clip_grabbed[selection]) { > > - s->clip_grabbed[selection] = FALSE; > > - if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) { > > - spice_main_channel_clipboard_selection_release(s->main, selection); > > - } > > - } > > If event->owner is NULL, the clipboard is empty at the moment and this > function exits without requesting the new targets. So in this case, we > should still send release to the agent, otherwise the guest might > think that clipboard data can be provided while the clipboard in the > client is empty for a long time. Pasting data in the guest would > result into an invalid request being sent to spice-gtk. This is going into undocumented territories. But if what you say is true in commit 9af2c481b74077ab7d6cb9d4bf589f9855a302f5, then yes, client should probably release the clipboard. I don't quite understand why gtk+ would allow NEW_OWNER, with owner == NULL and empty clipboard. It sounds more like a bug to me. Would this be enough ? diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c index 5b2c27c..3dbcae6 100644 --- a/src/spice-gtk-session.c +++ b/src/spice-gtk-session.c @@ -671,7 +671,11 @@ static void clipboard_owner_change(GtkClipboard *clipboard, return; } - if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) { + if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER || +#ifdef GDK_WINDOWING_X11 + (!event->owner && GDK_IS_X11_DISPLAY(gdk_display_get_default())) +#endif + ) { if (s->clip_grabbed[selection]) { /* grab was sent to the agent, so release it */ s->clip_grabbed[selection] = FALSE; @@ -690,13 +694,6 @@ static void clipboard_owner_change(GtkClipboard *clipboard, s->clipboard_by_guest[selection] = FALSE; -#ifdef GDK_WINDOWING_X11 - if (!event->owner && GDK_IS_X11_DISPLAY(gdk_display_get_default())) { - s->clip_hasdata[selection] = FALSE; - return; - } -#endif - > > - > > - /* We are mostly interested when owner has changed in which case > > - * we would like to let agent know about new clipboard data. */ > > if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) { > > + if (s->clip_grabbed[selection]) { > > + /* grab was sent to the agent, so release it */ > > + s->clip_grabbed[selection] = FALSE; > > + if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) { > > + spice_main_channel_clipboard_selection_release(s->main, selection); > > + } > > + } > > s->clip_hasdata[selection] = FALSE; > > return; > > } > > -- > > 2.21.0.4.g36eb1cb9cf > > > > Regards, > Jakub > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel