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

[Marc-André Lureau]

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?

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