[Spice-devel] [PATCH spice-gtk 1/2] clipboard: do not release between client grabs

[marcandre.lureau at redhat.com]

From: Marc-André Lureau <marcandre.lureau at redhat.com>

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 at redhat.com>
---
 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);
-        }
-    }
-
-    /* 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