[Spice-devel] [spice-gtk v1] gtk-session: reply agent's clipboard request on failure

[Victor Toso]

Hi,

On Fri, Feb 24, 2017 at 10:25:43AM +0100, Pavel Grunt wrote:
> Hi,
>
> On Fri, 2017-02-24 at 10:07 +0100, Victor Toso wrote:
> > Hi,
> >
> > On Wed, Feb 22, 2017 at 01:29:20PM +0100, Victor Toso wrote:
> Maybe "gtk-session: Always reply to agent's clipboard request" fits
> better

Ok, fixed locally

>
> > > From: Victor Toso <me at victortoso.com>
> > > 
> > > We need to reply back to the agent all clipboard requests even in
> > > case
> > > of failure otherwise it will have a pending request. The following
> > > error message can be seen in afterwards, when client sends down
> > > some
> > > clipboard data:
> >
> > It is worth to mention in the commit log that this is a regression
> > from
> > 7b0de6217670e0f668aff2949f, as we were notifying the agent even on
> > failure, see:
> thanks
> >
> > https://gitlab.com/spice/spice-gtk/commit/7b0de6217670e0f668aff2949f
> > ba174ed3cc0b50#7d95131068c82b7224989c1c61cc3c85a9743df3_956_990
> > 
> > I've included in the commit log:
> > 
> > "This fixes a regression from 7b0de6217670e0f668aff2949f"
> > 
> > Cheers,
> > 
> > > 
> > >  > clipboard: selection requests pending on clipboard ownership
> > >  > change, clearing
> > > 
> > > An easy way to reproduce this is:
> > > 1-) In client, copy image from lo-draw (selection or ctrl+c)
> > > 2-) In guest, paste it to GEdit (mouse3 our ctrl+v)
> > > 3-) Move to the client
> > > 4-) Move back to the guest
> > > 5-) see error on vdagent logs
> > >
>
> This seems to be specific to the linux agent

Yes, we could improve the linux agent too if possible, in the future but
it makes sense to me that we reply an agent's request for clipboard
data even if it is empty.

>
> > > The reason for failure is that client's clipboard contains
> > > different
> > > data type (image) then what was requested from guest's editor
> > > (text)
> > >
> > > While at it, include extra debug message as it might be hard to
> > > identify why clipboard did not work.
> > >
> > > Resolves: rhbz#1409854
> > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > ---
> > >  src/spice-gtk-session.c | 19 ++++++++++++-------
> > >  1 file changed, 12 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > index 0426d8f..315bc26 100644
> > > --- a/src/spice-gtk-session.c
> > > +++ b/src/spice-gtk-session.c
> > > @@ -940,28 +940,33 @@ static void
> > > clipboard_received_text_cb(GtkClipboard *clipboard,
> > >      if (self == NULL)
> > >          return;
> > >  
> > > +    selection = get_selection_from_clipboard(self->priv,
> > > clipboard);
> > > +    g_return_if_fail(selection != -1);
>
> so is it not needed to notify here as well ?

We can't without knowing the selection.

>
> > > +
> > >      if (text == NULL) {
> > >          SPICE_DEBUG("Failed to retrieve clipboard text");
> > > -        return;
> > > +        goto notify_agent;
> > >      }
> > >  
> > >      g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> > >  
> > > -    selection = get_selection_from_clipboard(self->priv,
> > > clipboard);
> > > -    g_return_if_fail(selection != -1);
> > > -
> > >      len = strlen(text);
> > >      if (!check_clipboard_size_limits(self, len)) {
> > > -        return;
> > > +        SPICE_DEBUG("Failed sized limits of clipboard text (%d
> > > bytes)", len);
> > > +        len = 0;
> > > +        goto notify_agent;
> > >      }
> > >  
> > >      /* gtk+ internal utf8 newline is always LF, even on windows
> > > */
> > >      conv = fixup_clipboard_text(self, text, &len);
> > >      if (!check_clipboard_size_limits(self, len)) {
> > > -        g_free(conv);
> > > -        return;
> > > +        SPICE_DEBUG("Failed sized limits of clipboard text (%d
> > > bytes)", len);
> > > +        g_clear_pointer(&conv, g_free);
>
> thanks to goto there is no need to clear it

We do. Conv might not have failed, this check checks the
clipboard_size_limits so, if limit exceed the protocol limit, we need to
free conv here to send the right value to agent (nothing).

>
> > > +        len = 0;
> > > +        goto notify_agent;
> > >      }
> > >  
> > > +notify_agent:
> > >      spice_main_clipboard_selection_notify(self->priv->main,
> > > selection,
> > >                                            VD_AGENT_CLIPBOARD_UTF8
> > > _TEXT,
> > >                                            (guchar *)(conv ?:
> > > text), len);
> > > -- 
> > > 2.9.3
> > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170224/34671486/attachment.sig>