[Spice-devel] [spice-gtk] gtk-session: do not request guest's clipboard data unnecessarily

[Victor Toso]

Hi,

Thanks for taking a look!

On Sun, Dec 30, 2018 at 10:23:02PM +0100, Jakub Janku wrote:
> Hi,
> 
> On Wed, Dec 19, 2018 at 3:30 PM Victor Toso <victortoso at redhat.com> wrote:
> >
> > From: Victor Toso <me at victortoso.com>
> >
> > If SpiceGtkSession is holding the keyboard, that's huge indication
> > that we should not be requesting clipboard data yet. The proper time
> > to request it is when another application in the client machine is
> > asking for it, which means the user would switch to another
> > application to paste the guest's clipboard data.
> >

> I'm having a rather hard time understanding this commit message.
> I read it the following way:
> "spice-gtk should not request clipboard data from vdagent, while
> spice-gtk holds the keyboard focus"
> However, what the patch actually does is that it makes sure that
> spice-gtk doesn't send clipboard grab to vdagent, while spice-gtk
> holds the keyboard focus.
> 
> These 2 things seem rather unrelated. What am I missing?

Client request for grab will make agent to look into clipboard
data in the guest which would lead to VD_AGENT_CLIPBOARD_REQUEST
being sent from agent to client.

I tried to avoid the grab first; I can also delay the clipboard
data request to a later time (like when the keyboard is out of
focus, as I suggested in the patch and explained below)

> > This is default behavior over wayland.
> 
> I can confirm that on Wayland, this patch breaks copy&paste
> from client to guest.
> According to the Wayland docs, the windows should receive the
> owner-change event before the focus-in event (
> https://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-data-sharing
> have a look at Data devices.Selection: "This event is also generated
> on a client immediately before it receives keyboard focus.").
> But as it turns out, gtk+ delays the event so that the focus-in event
> comes first ( https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gdk/wayland/gdkdevice-wayland.c
> check out the keyboard_handle_enter() ).
> So this patch will unfortunately need some changes in order to
> work on Wayland.

Yep!

> > Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> 
> Could you please elaborate a bit more on why this patch solves these issues?

I can reproduce the client requesting guest's clipboard data on
X11/KDE multiple times while the user is just copy-paste between
applications in the guest. The idea of this patch was to avoid
those requests till the keyboard focus is lost, which means that
user is trying to paste guest's clipboard data into another
application in the client's OS.

Let me know if it isn't clear!

> Cheers,
> Jakub

Thanks again,
Victor


> >
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > Tested-by: James Harvey @jamespharvey20
> > ---
> >  src/spice-gtk-session.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 1ccae07..0d3438c 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -645,9 +645,11 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> >          if (gtk_clipboard_get_owner(clipboard) == G_OBJECT(self))
> >              break;
> >
> > +
> >          s->clipboard_by_guest[selection] = FALSE;
> >          s->clip_hasdata[selection] = TRUE;
> > -        if (s->auto_clipboard_enable && !read_only(self))
> > +        if (s->auto_clipboard_enable && !read_only(self) &&
> > +            !spice_gtk_session_get_keyboard_has_focus(self))
> >              gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
> >                                            get_weak_ref(self));
> >          break;
> > --
> > 2.19.2
> >
> > _______________________________________________
> > 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: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190102/22469398/attachment-0001.sig>