[Spice-devel] [RFC spice-gtk 2/8] main: support VD_AGENT_SELECTION_* messages
Victor Toso
victortoso at redhat.com
Thu Aug 9 14:53:05 UTC 2018
Hi,
Note that this and next one got merge conflict with
0117daddc4e98f2 and I think next one with 2212f05145c5f1d
Not your fault :) Simple to fix, just please rebase when sending
the next (non RFC one)
On Thu, May 31, 2018 at 10:52:19PM +0200, Jakub Janků wrote:
> These enable transferring arbitrary type of data
> unlike the VDAgentClipboard* messages that are
> restricted to types defined in spice protocol
> (and atom2agent[] in spice-gtk-session.c).
>
> This will later be used for clipboard and DND data.
You could reference to rhbz#1381906, not sure if there is
upstream bug around.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1381906
> ---
> doc/reference/spice-gtk-sections.txt | 4 +
> src/channel-main.c | 275 +++++++++++++++++++++++++++
> src/channel-main.h | 9 +
> src/map-file | 4 +
> src/spice-glib-sym-file | 4 +
> src/spice-marshal.txt | 3 +
> 6 files changed, 299 insertions(+)
>
> diff --git a/doc/reference/spice-gtk-sections.txt b/doc/reference/spice-gtk-sections.txt
> index a85df7a..3f64a81 100644
> --- a/doc/reference/spice-gtk-sections.txt
> +++ b/doc/reference/spice-gtk-sections.txt
> @@ -94,6 +94,10 @@ spice_main_file_copy_async
> spice_main_channel_file_copy_async
> spice_main_file_copy_finish
> spice_main_channel_file_copy_finish
> +spice_main_channel_selection_grab
> +spice_main_channel_selection_send_data
> +spice_main_channel_selection_release
> +spice_main_channel_selection_request
> <SUBSECTION Standard>
> SPICE_MAIN_CHANNEL
> SPICE_IS_MAIN_CHANNEL
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 3d682d6..047b74e 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -167,6 +167,10 @@ enum {
> SPICE_MAIN_CLIPBOARD_SELECTION_RELEASE,
> SPICE_MIGRATION_STARTED,
> SPICE_MAIN_NEW_FILE_TRANSFER,
> + SPICE_MAIN_SELECTION_GRAB,
> + SPICE_MAIN_SELECTION_DATA,
> + SPICE_MAIN_SELECTION_RELEASE,
> + SPICE_MAIN_SELECTION_REQUEST,
> SPICE_MAIN_LAST_SIGNAL,
> };
>
> @@ -851,6 +855,86 @@ static void spice_main_channel_class_init(SpiceMainChannelClass *klass)
> 1,
> G_TYPE_OBJECT);
>
> + /**
> + * SpiceMainChannel::main-selection-grab:
> + * @main: the #SpiceMainChannel that emitted the signal
> + * @selection: VD_AGENT_CLIPBOARD_SELECTION_*
> + * @targets: advertised MIME types
> + *
> + * Inform that selection data is available from the guest
> + * and in which formats it can be provided.
> + **/
Very clear doc, many thanks! Just lacking `Since: 0.36`
> + signals[SPICE_MAIN_SELECTION_GRAB] =
> + g_signal_new("main-selection-grab",
> + G_OBJECT_CLASS_TYPE(gobject_class),
> + G_SIGNAL_RUN_LAST,
> + 0,
> + NULL, NULL,
> + g_cclosure_user_marshal_VOID__UINT_BOXED,
> + G_TYPE_NONE,
> + 2,
> + G_TYPE_UINT, G_TYPE_STRV);
> +
> + /**
> + * SpiceMainChannel::main-selection-data:
> + * @main: the #SpiceMainChannel that emitted the signal
> + * @selection: VD_AGENT_CLIPBOARD_SELECTION_*
> + * @format: number of bits per unit of @data
> + * @type: MIME type of @data
> + * @data: selection data
> + * @size: size of @data in bytes
> + *
> + * Inform that selection data has been retrieved.
> + **/
> + signals[SPICE_MAIN_SELECTION_DATA] =
> + g_signal_new("main-selection-data",
> + G_OBJECT_CLASS_TYPE(gobject_class),
> + G_SIGNAL_RUN_LAST,
> + 0,
> + NULL, NULL,
> + g_cclosure_user_marshal_VOID__UINT_INT_STRING_POINTER_UINT,
> + G_TYPE_NONE,
> + 5,
> + G_TYPE_UINT, G_TYPE_INT, G_TYPE_STRING, G_TYPE_POINTER, G_TYPE_UINT);
> +
> + /**
> + * SpiceMainChannel::main-selection-release:
> + * @main: the #SpiceMainChannel that emitted the signal
> + * @selection: VD_AGENT_CLIPBOARD_SELECTION_*
> + *
> + * Inform that the selection in the guest has been released
> + * and selection data is not available anymore.
> + **/
> + signals[SPICE_MAIN_SELECTION_RELEASE] =
> + g_signal_new("main-selection-release",
> + G_OBJECT_CLASS_TYPE(gobject_class),
> + G_SIGNAL_RUN_LAST,
> + 0,
> + NULL, NULL,
> + g_cclosure_marshal_VOID__UINT,
> + G_TYPE_NONE,
> + 1,
> + G_TYPE_UINT);
> +
> + /**
> + * SpiceMainChannel::main-selection-request:
> + * @main: the #SpiceMainChannel that emitted the signal
> + * @selection: VD_AGENT_CLIPBOARD_SELECTION_*
> + * @target: requested MIME type of selection data
> + *
> + * Request selection data from the client in @target format.
> + **/
> + signals[SPICE_MAIN_SELECTION_REQUEST] =
> + g_signal_new("main-selection-request",
> + G_OBJECT_CLASS_TYPE(gobject_class),
> + G_SIGNAL_RUN_LAST,
> + 0,
> + NULL, NULL,
> + g_cclosure_user_marshal_VOID__UINT_STRING,
> + G_TYPE_NONE,
> + 2,
> + G_TYPE_UINT, G_TYPE_STRING);
> +
> g_type_class_add_private(klass, sizeof(SpiceMainChannelPrivate));
> channel_set_handlers(SPICE_CHANNEL_CLASS(klass));
> }
> @@ -1339,6 +1423,7 @@ static void agent_announce_caps(SpiceMainChannel *channel)
> VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_CLIPBOARD_SELECTION);
> VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MONITORS_CONFIG_POSITION);
> VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS);
> + VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_SELECTION_DATA);
>
> agent_msg_queue(channel, VD_AGENT_ANNOUNCE_CAPABILITIES, size, caps);
> g_free(caps);
> @@ -2085,6 +2170,76 @@ static void main_agent_handle_msg(SpiceChannel *channel,
> case VD_AGENT_FILE_XFER_STATUS:
> main_agent_handle_xfer_status(self, payload);
> break;
> + case VD_AGENT_SELECTION_GRAB:
> + {
> + VDAgentSelectionGrab *s = payload;
> + GStrv targets;
> + guint i, n, len;
> +
> + len = msg->size - sizeof(VDAgentSelectionGrab);
> + /* make sure data is properly formatted */
> + g_return_if_fail(len >= 2);
> + g_return_if_fail(s->targets[0] != 0);
> + g_return_if_fail(s->targets[len-1] == 0);
> +
> + for (i = 1, n = 0; i < len; i++)
> + if (s->targets[i] == 0) {
> + g_return_if_fail(s->targets[i-1] != 0);
> + n++;
> + }
> +
> + targets = g_new0(gchar *, n + 1);
> + for (i = 0, n = 0; i < len; i++)
> + if (s->targets[i]) {
> + if (targets[n] == NULL)
> + targets[n] = (gchar *)(s->targets + i);
> + } else
> + n++;
Marc-André suggestion seems good for those. Maybe cherry-pick
that from his git tree, add it in the series and use it here?
> + g_coroutine_signal_emit(channel, signals[SPICE_MAIN_SELECTION_GRAB], 0,
> + s->selection, targets);
> + g_free(targets);
> + break;
> + }
> + case VD_AGENT_SELECTION_DATA:
> + {
> + VDAgentSelectionData *s = payload;
> + guint offset, len;
> +
> + len = msg->size - sizeof(VDAgentSelectionData);
> + for (offset = 0; offset < len; offset++)
> + if (s->data[offset] == 0)
> + break;
> + offset++;
> + g_return_if_fail(offset >= 2 && offset <= len);
> +
> + g_coroutine_signal_emit(self, signals[SPICE_MAIN_SELECTION_DATA], 0,
> + s->selection, s->format, s->data,
> + s->data + offset, len - offset);
> + break;
> + }
> + case VD_AGENT_SELECTION_RELEASE:
> + {
> + VDAgentSelectionRelease *s = payload;
> + g_coroutine_signal_emit(self, signals[SPICE_MAIN_SELECTION_RELEASE], 0,
> + s->selection);
> + break;
> + }
> + case VD_AGENT_SELECTION_REQUEST:
> + {
> + VDAgentSelectionRequest *s = payload;
> + guint i, len;
> +
> + len = msg->size - sizeof(VDAgentSelectionRequest);
> + for (i = 0; i < len; i++)
> + if (s->target[i] == 0)
> + break;
> + g_return_if_fail(i > 0 && i == len - 1);
> +
> + g_coroutine_signal_emit(self, signals[SPICE_MAIN_SELECTION_REQUEST], 0,
> + s->selection, s->target);
> + break;
> + }
> default:
> g_warning("unhandled agent message type: %u (%s), size %u",
> msg->type, NAME(agent_msg_types, msg->type), msg->size);
> @@ -3408,3 +3563,123 @@ gboolean spice_main_channel_file_copy_finish(SpiceMainChannel *channel,
>
> return g_task_propagate_boolean(task, error);
> }
> +
> +static gboolean main_selection_params_valid(SpiceMainChannel *channel,
> + guint selection)
> +{
> + g_return_val_if_fail(channel != NULL, FALSE);
> + g_return_val_if_fail(SPICE_IS_MAIN_CHANNEL(channel), FALSE);
> + g_return_val_if_fail(test_agent_cap(channel, VD_AGENT_CAP_SELECTION_DATA), FALSE);
> + g_return_val_if_fail(selection <= VD_AGENT_CLIPBOARD_SELECTION_SECONDARY, FALSE);
> +
> + return channel->priv->agent_connected;
Probably worth to put channel->priv->agent_connected in a guard
as well otherwise, if agent is disconnected, those selection api
would fail without any information of why. It is a critical
warning but those API shouldn't be called if "agent-connected" is
false.
+ g_return_val_if_fail(selection <= VD_AGENT_CLIPBOARD_SELECTION_SECONDARY, FALSE);
+ g_return_val_if_fail(return channel->priv->agent_connected, FALSE);
+
- return channel->priv->agent_connected;
+ return TRUE;
> +}
> +
> +/**
> + * spice_main_channel_selection_grab:
> + * @channel: a #SpiceMainChannel
> + * @selection: #VD_AGENT_CLIPBOARD_SELECTION_*
> + * @targets: NULL-terminated array of MIME types to advertise
> + *
> + * Grab the guest selection.
> + **/
> +void spice_main_channel_selection_grab(SpiceMainChannel *channel,
> + guint selection,
> + const gchar **targets)
> +{
> + if (!main_selection_params_valid(channel, selection))
> + return;
> +
> + VDAgentSelectionGrab *msg;
> + guint i, size;
> + gchar *ptr;
> +
> + size = sizeof(VDAgentSelectionGrab);
> + for (i = 0; targets[i]; i++)
> + size += strlen(targets[i]) + 1;
> +
> + msg = g_malloc(size);
> + msg->selection = selection;
> + for (i = 0, ptr = (gchar *)msg->targets; targets[i]; i++)
> + ptr = g_stpcpy(ptr, targets[i]) + 1;
g_stpcpy is new to me, very useful.
> +
> + agent_msg_queue(channel, VD_AGENT_SELECTION_GRAB, size, msg);
> + g_free(msg);
> + spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
> +}
> +
> +/**
> + * spice_main_channel_selection_send_data:
> + * @channel: a #SpiceMainChannel
> + * @selection: #VD_AGENT_CLIPBOARD_SELECTION_*
> + * @format: number of bits per unit of @data
> + * @type: MIME type of @data
> + * @data: selection data
> + * @size: length of @data in bytes
> + *
> + * Send the selection data to the guest.
`Since: 0.36`
> + **/
> +void spice_main_channel_selection_send_data(SpiceMainChannel *channel,
> + guint selection,
> + gint format,
> + const gchar *type,
> + const guchar *data,
> + guint size)
> +{
> + if (!main_selection_params_valid(channel, selection))
> + return;
> +
> + VDAgentSelectionData msg;
I would prefer to declare the variables before the check above
> + msg.selection = selection;
> + msg.format = format;
> + agent_msg_queue_many(channel, VD_AGENT_SELECTION_DATA,
> + &msg, sizeof(msg),
> + type, strlen(type) + 1,
> + data, size, NULL);
> + spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
> +}
> +
> +/**
> + * spice_main_channel_selection_release:
> + * @channel: a #SpiceMainChannel
> + * @selection: #VD_AGENT_CLIPBOARD_SELECTION_*
> + *
> + * Release grab of the selection,
> + * inform the guest data is not available anymore.
> + **/
> +void spice_main_channel_selection_release(SpiceMainChannel *channel,
> + guint selection)
> +{
> + if (!main_selection_params_valid(channel, selection))
> + return;
> +
> + VDAgentSelectionRelease msg;
ditto
> + msg.selection = selection;
> + agent_msg_queue(channel, VD_AGENT_SELECTION_RELEASE, sizeof(msg), &msg);
> + spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
> +}
> +
> +/**
> + * spice_main_channel_selection_request:
> + * @channel: a #SpiceMainChannel
> + * @selection: #VD_AGENT_CLIPBOARD_SELECTION_*
> + * @target: MIME type of selection data to retrieve
> + *
> + * Request selection data from the guest in @target type.
> + * The reply is sent through the #SpiceMainChannel::main-selection-data signal.
> + **/
> +gboolean spice_main_channel_selection_request(SpiceMainChannel *channel,
> + guint selection,
> + const gchar *target)
> +{
> + if (!main_selection_params_valid(channel, selection))
> + return FALSE;
> +
> + VDAgentSelectionRequest msg;
ditto
> + msg.selection = selection;
> + agent_msg_queue_many(channel, VD_AGENT_SELECTION_REQUEST,
> + &msg, sizeof(msg),
> + target, strlen(target) + 1, NULL);
Maybe check target != NULL before strlen?
> + spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
> + return TRUE;
> +}
> diff --git a/src/channel-main.h b/src/channel-main.h
> index 530378d..1c915ad 100644
> --- a/src/channel-main.h
> +++ b/src/channel-main.h
> @@ -85,6 +85,15 @@ void spice_main_channel_clipboard_selection_notify(SpiceMainChannel *channel, gu
> void spice_main_channel_clipboard_selection_request(SpiceMainChannel *channel, guint selection,
> guint32 type);
>
> +void spice_main_channel_selection_grab(SpiceMainChannel *channel, guint selection,
> + const gchar **targets);
> +void spice_main_channel_selection_send_data(SpiceMainChannel *channel, guint selection,
> + gint format, const gchar *type,
> + const guchar *data, guint size);
> +void spice_main_channel_selection_release(SpiceMainChannel *channel, guint selection);
> +gboolean spice_main_channel_selection_request(SpiceMainChannel *channel, guint selection,
> + const gchar *target);
> +
> gboolean spice_main_channel_agent_test_capability(SpiceMainChannel *channel, guint32 cap);
> void spice_main_channel_file_copy_async(SpiceMainChannel *channel,
> GFile **sources,
> diff --git a/src/map-file b/src/map-file
> index cdb81c3..4b20454 100644
> --- a/src/map-file
> +++ b/src/map-file
> @@ -108,6 +108,10 @@ spice_main_set_display;
> spice_main_set_display_enabled;
> spice_main_update_display;
> spice_main_update_display_enabled;
> +spice_main_channel_selection_grab;
> +spice_main_channel_selection_send_data;
> +spice_main_channel_selection_release;
> +spice_main_channel_selection_request;
> spice_playback_channel_get_type;
> spice_playback_channel_set_delay;
> spice_port_channel_event;
> diff --git a/src/spice-glib-sym-file b/src/spice-glib-sym-file
> index b19844c..2cb7738 100644
> --- a/src/spice-glib-sym-file
> +++ b/src/spice-glib-sym-file
> @@ -87,6 +87,10 @@ spice_main_set_display
> spice_main_set_display_enabled
> spice_main_update_display
> spice_main_update_display_enabled
> +spice_main_channel_selection_grab
> +spice_main_channel_selection_send_data
> +spice_main_channel_selection_release
> +spice_main_channel_selection_request
> spice_playback_channel_get_type
> spice_playback_channel_set_delay
> spice_port_channel_event
> diff --git a/src/spice-marshal.txt b/src/spice-marshal.txt
> index b3a8e69..18b8d4c 100644
> --- a/src/spice-marshal.txt
> +++ b/src/spice-marshal.txt
> @@ -14,3 +14,6 @@ BOOLEAN:UINT,UINT
> VOID:OBJECT,OBJECT
> VOID:BOXED,BOXED
> POINTER:BOOLEAN
> +VOID:UINT,BOXED
> +VOID:UINT,STRING
> +VOID:UINT,INT,STRING,POINTER,UINT
Cheers,
Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180809/cd486296/attachment-0001.sig>
More information about the Spice-devel
mailing list