gtk: only release clipboard when neither guest nor client own it

08:55 < hansg> elmarco, this is from vdagent.log with debugging enabled. What is happening is that the last thing done was a primary
               selection in the client, so vdagent owns the clipboard in the guest (on behalf of spice-gtk), then something gets selected
               inside the guest, the agent sends a grab to spice-gtk, which then does a gtk_clipboard_set_with_data, this triggers an
               clipboard_owner_change which sends a release message to the agent
08:56 < hansg> To which the agent then responds by dropping it, and logging:
08:56 < hansg> primary: received release while not owning client clipboard
09:11 < elmarco> hansg: but this bug we are talking about is not related to multi-clipboard right?
09:11 < elmarco> and it's only a warning in vdagent, things works as expected otherwise, right?
09:11 < elmarco> the bug was thee before I suppose
09:12 < hansg> right, they work because vdagent is diligent and sees the client sends a release while it is not owning the clipboard. The
               diligence is mainly there in case things race though (release on client racing with a grab on guest), not to make things
               work with buggy clients :)
09:13 < hansg> wrt: <elmarco> hansg: d->clip_grabbed is only for client-side grab, iirc
09:13 < elmarco> ok, I think it's just an obscure area of the spec, where basically, we don't define exactly the "state machine"
09:13 < elmarco> so the client is releasing his last client-side grab, because he had one before, but now, it is a guest grab
09:13 < hansg> True (not exactly definging the state machine)
09:14 < hansg> So to try again wrt the  d->clip_grabbed, what happens there (which has the same cause) is:
09:14 < elmarco> so, what it should do is just don't release the clipboard if it is switching from client-side to guest
09:15 < hansg> gtk/spice-widget.c: clipboard_grab gets called, and does:
09:15 < hansg> Hmm, hold on, I see what you mean wrt d->clip_grabbed now
09:16 < elmarco> to me, it looks like the client made a grab and to complete it's cycle, it should release his grab
09:16 < elmarco> but the order of things confuse vdagent and we should agree on something and document it
09:18 < hansg> elmarco, I need some time to take a somewhat closer look at the spice-gtk code in this area, give me 1/2 an hour and I'l
               get back to you
09:20 < elmarco> from client 1. grab -> 2. grab <- 3. release -> or 1. grab -> 2. grab <- 3. no release
09:21 < elmarco> I think state should not be mixed between client grab / release -> and guest grab/release <-
09:22 < elmarco> so, overriding client grab by guest grab should release client grab
09:23 < hansg> spicec and the linux agent both assume that after sending a grab they won't get back a release (for that selection). The
               purpose of the release is to tell the OS that the agent resp,  client no longer own the clipboard (by setting the owner to
               None under X11), so that other apps can disable their paste menu item, etc. There is no need to do that (and actually
               doing so would be a bug) if an other app now owns the clipboard. So if the other side claim
09:23 < hansg> s ownership of the clipboard there is no need to tell it you're releasing your side, since it already assume you have
09:24 < hansg> Scenarios to keep in mind are:
09:24 < hansg> Seen from the client side:
09:28 < hansg> client grabs clipboard
09:28 < hansg> some app on guest becomes owner, guest sends grab, assume client release
09:29 < hansg> some app on guest asks agent for clipboard data -> tells it to go away since the client no longer the owner
09:29 < hansg> If it would not assume the release, there would be a window where it would think the client still owns the clipboard and
               forward potential request to the client, even though the client no longer owns the clipboard
09:30 < hansg> The thing to keep in mind is that the delivery of messages is not instant, so there is some window where the 2 sides are
               out of sync.
09:30 < hansg> I can see the logic in how you're advocating to do things, but that is not how they are currently done and I'm reluctant
               to change this
09:33 < elmarco> hansg: yeah, I don't think one solution or the other affect user experience, for me there is no gap if the client and
                 agent agrees, it's only protocol/implementation details
09:33 < elmarco> since there was prior implementation, we can decide to follow it

1 files changed, 3 insertions, 2 deletions

diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index dc652d9..2903649 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -1127,8 +1127,9 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
 
     if (d->clip_grabbed[selection]) {
         d->clip_grabbed[selection] = false;
-        spice_main_clipboard_selection_release(d->main,
-            get_selection_from_clipboard(d, clipboard));
+        if (!d->clipboard_by_guest[selection])
+            spice_main_clipboard_selection_release(d->main,
+                get_selection_from_clipboard(d, clipboard));
     }
 
     switch (event->reason) {