[Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL

[Marc-André Lureau]

Hi

On Thu, Mar 28, 2019 at 6:05 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> >
> > ..Hi
> >
> > On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> > > > The role of the grab message is to take ownership of the clipboard (to
> > > > advertize clipboard data available). It may come at any time from both
> > > > side, and override the current grab owner. It may come from the guest
> > > > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > > > Or it may come from the client (C-c in client side app), and the agent
> > > > will grab the guest clipboard.
> > > >
> > >
> > > Yes, I realized this morning thinking about how clipboards works
> > > (very rusty in my mind).
> > > Instead of setting it you get the ownership that is you are willing
> > > to set a value on the clipboard but you defer setting it to avoid
> > > the expense of data copy.
> > > So, the old (original?) protocol was simply to sending entire data
> > > on every change, this avoided to loose the clipboard data entirely.
> >
> > I don't even remember that version. It looks like the original version
> > was already "on-demand"
> >
> >
> > commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
> > Author: Arnon Gilboa <agilboa at redhat.com>
> > Date:   Mon Oct 4 16:45:15 2010 +0200
> >
> >     vd_agent: add protocol messages for clipboard/selection-owner model
> >
> >     -VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
> > application in our side ("we") got ownership of the clipboard.
> >     -VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
> > owns the clipboard (GRAB received), we notify the os we are the owner.
> > when we are asked by the os/app for the clipboard data, we send this
> > RE
> > QUEST msg to the other side.
> >     -VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
> > the clipboard, is now sent only in response to REQUEST.
> >     -VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
> > longer the owner of the clipboard (e.g. the owner app was closed).
> >
> >     this patch will be followed by agent & client patches handling the
> > above messages.
> >
> >
> >
> > So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to me.
> >
>
> I suppose the "now" in "the existing message for sending the clipboard, is
> now sent only in response to REQUEST" means that was changed.
>
> I personally think the code should be readable from a tarball/snapshot not
> pretending people to dig into 10 years of history. I'll try to find some time
> to put these lines into vd_agent.h.
>
> > > The current is: if we get new local clipboard data send the "grab"
> > > so remote knows that will have to read our data.
> >
> > yes
> >
> > > So either we have ownership/grab, meaning the data are remote
> > > waiting to get fetched or we don't have ownership and the remote
> > > should be grabbing as we have data to send (at least in a stable
> > > state).
> >
> >
> > That last sentence is confusing. There are 2 states the client can
> > "have the grab".
> >
> > 1. the client took the grab for the remote data: we are talking about
> > the client app in the client desktop environment
> > 2. the client took the grab, at the protocol level: it sent a grab
> > over the protocol to announce it can provide clipboard data to the
> > remote. In this case, the client app doesn't have the grab in the
> > client desktop environment, but another client application.
> >
> > In general, when talking about the protocol, "client has the grab"
> > means 2) to me, iow it can provide data to the remote.
> >
>
> I think I mean the opposite. The reason is that some application
> have the ownership on either side, when you send a VD_AGENT_CLIPBOARD_GRAB
> the current local application (so spice-gtk/vd_agent) does NOT have the
> ownership and it's asking the remote to take to the application (so will
> remove the ownership from another remote application).
> From how I read the message (VD_AGENT_CLIPBOARD_GRAB) is telling the
> other side to grab (take ownership) of the clipboard so will be the
> remote to have the "grab", not the local.


I find it hard to understand you. The sequence of events is as such.

A & B are client or remote exchangeably:
- an application on A takes the clipboard grab (== announce clipboard
data is available)
- A get notified by the desktop and sends a GRAB message to B side
- B receives GRAB, and takes the clipboard grab on B desktop side

With this sequence, "A" has the clipboard grab at the Spice protocol
level, so to say. It means there is clipboard data on A side.

> >
> > > Not sure what is the initial state, when you connect.
> >
> > Initial state is undefined, and currently whatever the guest or client
> > side state is. Iow, Spice client/agent doen't do anything afaik. They
> > will only react to further grab events.
> >
>
> I suppose the agent will keep its state while the client if was not
> connected get some initial state (boolean variables have to be true or
> false at the end).
>
> > We could change the client or the guest to take the grab on connect,
> > if clipboard data is available. That doesn't require protocol change.
> > Although in case of conflict, we would probably end in the same
> > situation that this protocol change is aiming to solve.
> >
>
> When there's a disconnection we should drop the ownership as we
> cannot anymore get the data from the remote in case other application
> are asking so the "disconnected" state should be no ownership of
> the clipboard on both ends (although I suppose the client will be
> closed at that time).

That's an implementation issue, not related to the protocol and the
problem discussed here.

>
> Once a connection happens we could however have both ends with
> data on the clipboard (the total states are 3 (a) client/agent have
> ownership (b) other application have ownership (c) no ownership/data
> on clipboard, here (a) should be impossible).
> Which one "win" I would say hard to tell.
>

If both sides would try to take the grab simultaneously at init time,
they would race, and we get back to the problem we are solving here.