Re: [spice-gtk v3 1/3] gtk-session: clipboard: document owner-changed event

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

 



Hi,

I see that I'm being late to the party as this has already been pushed
upstream, but I'd like to comment anyway:

On Thu, Jan 10, 2019 at 1:47 PM Victor Toso <victortoso@xxxxxxxxxx> wrote:
>
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> This patch improves the code style by adding braces, removing the
> single case switch and add documentation to the handler of
> GtkClipboard::owner-changed.
>
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> ---
>  src/spice-gtk-session.c | 54 +++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 1ccae07..adc72a2 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -618,6 +618,23 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
>      s->nclip_targets[selection] = 0;
>  }
>
> +/* Callback for every owner-change event for given @clipboard.
> + * This event is triggered in different ways depending on the environment of
> + * the Client, some examples:
> + *
> + * Situation 1: When another application on the client machine is holding and
> + * changing the clipboard. If client is on Wayland, spice-gtk only receives the
> + * related GtkClipboard::owner-changed event after focus-in event on Spice
> + * widget; On X11, we will receive it at the moment the clipboard data has been
> + * changed in by other application.
> + *
> + * Situation 2: When spice-gtk holds the focus and is changing the clipboard by
> + * either setting new content information with gtk_clipboard_set_with_owner() or
> + * clearing up old content with gtk_clipboard_clear(). The main difference between
> + * Wayland and X11 is that on X11, gtk_clipboard_clear() set the owner to none, which

"set" -> "sets" ?

> + * emits owner-change event; On Wayland that does not happen as spice-gtk still is
> + * the owner of the clipboard.

You must have come across this bug, since you commented on it:
https://gitlab.gnome.org/GNOME/gtk/issues/715

I've created a simple test case which is attached.
It seems like no "owner-change" event is emitted after
gtk_clipboard_clear() is called,
but after gtk_clipboard_set_with_owner() the event is emitted twice.

So the Wayland behavior you described here might just be caused by the bug...
> + */
>  static void clipboard_owner_change(GtkClipboard        *clipboard,
>                                     GdkEventOwnerChange *event,
>                                     gpointer            user_data)
> @@ -631,30 +648,37 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
>      selection = get_selection_from_clipboard(s, clipboard);
>      g_return_if_fail(selection != -1);
>
> -    if (s->main == NULL)
> +    if (s->main == NULL) {
>          return;
> +    }
>
> +    /* 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]) {
>          s->clip_grabbed[selection] = FALSE;
> -        if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
> +        if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) {
>              spice_main_channel_clipboard_selection_release(s->main, selection);
> +        }
>      }
>
> -    switch (event->reason) {
> -    case GDK_OWNER_CHANGE_NEW_OWNER:
> -        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))
> -            gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
> -                                          get_weak_ref(self));
> -        break;
> -    default:
> +    /* We are mostly interested when owner has changed in which case
> +     * we would like to let agent know about new clipboard data. */
> +    if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) {
>          s->clip_hasdata[selection] = FALSE;
> -        break;
> +        return;
>      }
> +
> +    /* This situation happens when clipboard is being cleared by us, when agent
> +     * sends a release-grab for instance */

This is not true imho. It should only happen after a grab (after
gtk_clipboard_set_with_owner() is called).
After a release (gtk_clipboard_clear() call),
gtk_clipboard_get_owner(clipboard) should already return NULL.

> +    if (gtk_clipboard_get_owner(clipboard) == G_OBJECT(self)) {
> +        return;
> +    }
> +
> +    s->clipboard_by_guest[selection] = FALSE;
> +    s->clip_hasdata[selection] = TRUE;
> +    if (s->auto_clipboard_enable && !read_only(self))
> +        gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
> +                                      get_weak_ref(self));
>  }
>
>  typedef struct
> --
> 2.20.1
>

Cheers,
Jakub
#include <gtk/gtk.h>

GtkClipboard *clipboard;

static void clipboard_owner_change_cb(GtkClipboard        *clipboard,
                                      GdkEventOwnerChange *event,
                                      gpointer             user_data)
{
    g_message("owner_change_cb: owner=%p", gtk_clipboard_get_owner(clipboard));
}

static void clipboard_get_cb(GtkClipboard     *clipboard,
                             GtkSelectionData *selection_data,
                             guint             info,
                             gpointer          user_data)
{
    gtk_selection_data_set_text(selection_data, "abcd", -1);
}

static void clipboard_clear_cb(GtkClipboard *clipboard,
                               gpointer      user_data)
{
    g_message("clear_cb");

    GObject *owner = user_data;
    g_object_unref(owner);
}

static void btn_grab_click(GtkButton *button,
                           gpointer   user_data)
{
    GObject *owner;
    GtkTargetList *list;
    GtkTargetEntry *table;
    gint n_targets;

    g_message("grabbing clipboard");

    list = gtk_target_list_new(NULL, 0);
    gtk_target_list_add_text_targets(list, 0);
    table = gtk_target_table_new_from_list(list, &n_targets);

    owner = g_object_new(G_TYPE_OBJECT, NULL);

    gtk_clipboard_set_with_owner(clipboard, table, n_targets,
                                clipboard_get_cb, clipboard_clear_cb, owner);

    gtk_target_table_free(table, n_targets);
    gtk_target_list_unref(list);
}

static void btn_release_click(GtkButton *button,
                              gpointer   user_data)
{
    g_message("clearing clipboard");

    gtk_clipboard_clear(clipboard);
}

static void create_gui()
{
    GtkWidget *window, *box, *btn_grab, *btn_release;

    window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
    box = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 10);

    btn_grab = gtk_button_new_with_label("Grab");
    btn_release = gtk_button_new_with_label("Release");
    g_signal_connect(btn_grab, "clicked", G_CALLBACK(btn_grab_click), NULL);
    g_signal_connect(btn_release, "clicked", G_CALLBACK(btn_release_click), NULL);

    gtk_box_pack_start((GtkBox *)box, btn_grab, TRUE, TRUE, 0);
    gtk_box_pack_start((GtkBox *)box, btn_release, TRUE, TRUE, 0);

    gtk_container_add(GTK_CONTAINER(window), box);
    gtk_widget_show_all(window);
}

int main(int argc, char *argv[])
{
    gtk_init(&argc, &argv);

    clipboard = gtk_clipboard_get(GDK_SELECTION_CLIPBOARD);
    g_signal_connect(clipboard,
                     "owner-change",
                     G_CALLBACK(clipboard_owner_change_cb),
                     NULL);

    create_gui();

    gtk_main ();

    return 0;
}
_______________________________________________
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]