> Hi,
>
> On Thu, Dec 06, 2018 at 03:54:41AM -0500, Frediano Ziglio wrote:
> > >
> > > From: Victor Toso <me@xxxxxxxxxxxxxx>
> > >
> > > Commit 284c1f2d switched to
> > > spice_main_channel_clipboard_selection_release() which does check if
> > > agent is connected and does the right thing (expected) in regards to
> > > releasing the clipboard by calling agent_clipboard_release() which
> > > does check VD_AGENT_CAP_CLIPBOARD_SELECTION (like current removed
> > > code).
> > >
> > > So this patch removes redundant check.
> > >
> > > Same goes for spice_main_channel_clipboard_selection_grab() function.
> > >
> > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> >
> > This is not the same. The test in agent_clipboard_grab is done
> > with g_return_if_fail which emits a warning while current code
> > does not.
>
> Which probably should
>
IMHO is the opposite, as we supports agents that does not support
VD_AGENT_CAP_CLIPBOARD_BY_DEMAND why giving warning every time?
If we would like to warn the user that guest is using a very old
agent would not be better to warn once when the capabilities are
received? Maybe if it's really important this could be also done
with a GUI message (maybe with a persistent tick "don't warn
about it next time" like for the exit message).
> > Also spice_main_channel_clipboard_selection_grab always calls
> > spice_channel_wakeup which current code does not.
>
> Which is probably a bug there (should call spice_channel_wakeup
> if message was queued.
>
> What do you think?
>
Not sure if this function is also expected to handle pending
events and what would happen if this is removed.
If it's safe to do so then I would move spice_channel_wakeup
call inside spice_main_channel_clipboard_selection_grab at
the end of agent_clipboard_grab (which is only called by
spice_main_channel_clipboard_selection_grab).
> > > ---
> > > src/spice-gtk-session.c | 7 ++-----
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > index 20a4060..8d31045 100644
> > > --- a/src/spice-gtk-session.c
> > > +++ b/src/spice-gtk-session.c
> > > @@ -610,9 +610,7 @@ static void clipboard_get_targets(GtkClipboard
> > > *clipboard,
> > > }
> > >
> > > s->clip_grabbed[selection] = TRUE;
> > > -
> > > - if (spice_main_channel_agent_test_capability(s->main,
> > > VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
> > > - spice_main_channel_clipboard_selection_grab(s->main, selection,
> > > types, num_types);
> > > + spice_main_channel_clipboard_selection_grab(s->main, selection,
> > > types,
> > > num_types);
> > >
> > > /* Sending a grab causes the agent to do an implicit release */
> > > s->nclip_targets[selection] = 0;
> > > @@ -636,8 +634,7 @@ static void clipboard_owner_change(GtkClipboard
> > > *clipboard,
> > >
> > > if (s->clip_grabbed[selection]) {
> > > s->clip_grabbed[selection] = FALSE;
> > > - if (spice_main_channel_agent_test_capability(s->main,
> > > VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
> > > - spice_main_channel_clipboard_selection_release(s->main,
> > > selection);
> > > + spice_main_channel_clipboard_selection_release(s->main,
> > > selection);
> > > }
> > >
> > > switch (event->reason) {
> >
> > Frediano
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel