[Spice-devel] [PATCH spice-gtk 3/3] clipboard: invalidate targets request when needed

[Jakub Janku]

Hi,

On Wed, Mar 6, 2019 at 7:11 PM Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
>
> Hi
>
> On Thu, Feb 28, 2019 at 8:12 PM Jakub Janků <jjanku at redhat.com> wrote:
> >
> > Targets request is no longer relevant when
> > clipboard owner changes since the retrieved targets
> > will be outdated.
> >
> > When the request is no longer relevant,
> > invalidate it by pointing its weak ref to NULL.
> > As a consequence, free_weak_ref() call returns NULL
> > and clipboard_get_targets() exits.
>
> Why not simply check if user_data == last_targets_request?

This surely looks simpler,
thanks :)

Jakub
>
> What does nullify_weak_ref() really helps with?
>
> >
> > Signed-off-by: Jakub Janků <jjanku at redhat.com>
> > ---
> >
> > This addresses the same issue that
> > was present in spice-vdagent and is already fixed.
> >
> > ---
> >  src/spice-gtk-session.c | 39 +++++++++++++++++++++++++++++----------
> >  1 file changed, 29 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 018cb4b..037547a 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -59,6 +59,7 @@ struct _SpiceGtkSessionPrivate {
> >      gboolean                clip_hasdata[CLIPBOARD_LAST];
> >      gboolean                clip_grabbed[CLIPBOARD_LAST];
> >      gboolean                clipboard_by_guest[CLIPBOARD_LAST];
> > +    GWeakRef                *last_targets_request[CLIPBOARD_LAST];
> >      /* auto-usbredir related */
> >      gboolean                auto_usbredir_enable;
> >      int                     auto_usbredir_reqs;
> > @@ -537,6 +538,16 @@ static gpointer free_weak_ref(gpointer data)
> >      return object;
> >  }
> >
> > +static void nullify_weak_ref(GWeakRef **weak_ref_p)
> > +{
> > +    gpointer null_object = NULL;
> > +
> > +    if (*weak_ref_p) {
> > +        g_weak_ref_set(*weak_ref_p, null_object);
> > +        *weak_ref_p = NULL;
> > +    }
> > +}
> > +
> >  static void clipboard_get_targets(GtkClipboard *clipboard,
> >                                    GdkAtom *atoms,
> >                                    gint n_atoms,
> > @@ -551,23 +562,25 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
> >
> >      g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> >
> > -    if (atoms == NULL) {
> > -        SPICE_DEBUG("Retrieving the clipboard data has failed");
> > -        return;
> > -    }
> > -
> >      SpiceGtkSessionPrivate *s = self->priv;
> >      guint32 types[SPICE_N_ELEMENTS(atom2agent)] = { 0 };
> >      gint num_types;
> >      int a;
> >      int selection;
> >
> > -    if (s->main == NULL)
> > -        return;
> > -
> >      selection = get_selection_from_clipboard(s, clipboard);
> >      g_return_if_fail(selection != -1);
> >
> > +    s->last_targets_request[selection] = NULL;
> > +
> > +    if (atoms == NULL) {
> > +        SPICE_DEBUG("Retrieving the clipboard data has failed");
> > +        return;
> > +    }
> > +
> > +    if (s->main == NULL)
> > +        return;
> > +
> >      if (s->clip_grabbed[selection]) {
> >          SPICE_DEBUG("Clipboard is already grabbed, ignoring %d atoms", n_atoms);
> >          return;
> > @@ -652,6 +665,8 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> >          return;
> >      }
> >
> > +    nullify_weak_ref(&s->last_targets_request[selection]);
> > +
> >      /* In case we sent a grab to the agent, we need to release it now as
> >       * previous clipboard data should not be reachable anymore */
> >      if (s->clip_grabbed[selection]) {
> > @@ -690,9 +705,11 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> >  #endif
> >
> >      s->clip_hasdata[selection] = TRUE;
> > -    if (s->auto_clipboard_enable && !read_only(self))
> > +    if (s->auto_clipboard_enable && !read_only(self)) {
> > +        s->last_targets_request[selection] = get_weak_ref(self);
> >          gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
> > -                                      get_weak_ref(self));
> > +                                      s->last_targets_request[selection]);
> > +    }
> >  }
> >
> >  typedef struct
> > @@ -866,6 +883,8 @@ static gboolean clipboard_grab(SpiceMainChannel *main, guint selection,
> >          return TRUE;
> >      }
> >
> > +    nullify_weak_ref(&s->last_targets_request[selection]);
> > +
> >      if (!gtk_clipboard_set_with_owner(cb,
> >                                        targets,
> >                                        num_targets,
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>
> --
> Marc-André Lureau