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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,


On Wed, Jan 2, 2019, 5:31 PM Victor Toso <victortoso@xxxxxxxxxx wrote:
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@xxxxxxxxxx> wrote:
> >
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > 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.

Exactly! So it's the agent requesting the CLIENT's data. But in the commit message, you state "do not request GUEST's clipboard data unnecessarily". Or isn't it?

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!

Makes sense to me to try to avoid such requests. But why are those requests causing the issues described?

> Cheers,
> Jakub

Thanks again,
Victor


> >
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]