On Mon, 2016-05-30 at 11:55 +0200, Victor Toso wrote:
> Application can start multiple file-transfers in one operation and
> cancel a few of them while the operation is ongoing. In that case, we
> should remove the file-size of the transfer operation otherwise we
> will send incorrect progress data.
>
> Taking in consideration the split of SpiceFileTransferTask, this patch
> includes spice_file_transfer_task_get_file_size() internal helper
> function.
> ---
> src/channel-main.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index e204a1e..890d939 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -61,6 +61,7 @@ typedef void
> (*SpiceFileTransferTaskFlushCb)(SpiceFileTransferTask *xfer_task,
> static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
> static SpiceMainChannel
> *spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
> static GCancellable
> *spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
> +static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask
> *self);
> static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self,
> GError *error);
> static GList *spice_file_transfer_task_create_tasks(SpiceMainChannel
> *channel,
> GFile **files,
> @@ -1927,6 +1928,7 @@ static void file_xfer_end_callback(GObject
> *source_object,
> {
> GTask *task;
> FileTransferOperation *xfer_op;
> + SpiceFileTransferTask *xfer_task;
>
> task = G_TASK(res);
> if (!g_task_had_error(task))
> @@ -1934,7 +1936,9 @@ static void file_xfer_end_callback(GObject
> *source_object,
> * file_transfer_operation_task_finished */
> return;
>
> + xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
> xfer_op = user_data;
> + xfer_op->transfer_size -=
> spice_file_transfer_task_get_file_size(xfer_task);
I don't think this is sufficient. The file might get cancelled after it has
transferred some of its data already. In this case, that already-transferred
data will have been added to total_sent, but we will subtract the file size from
transfer_size. Consider the situation where you're transferring two 1MB files.
If you cancel one of these files after sending 75% of both files, the next
progress update will indicate that you've sent 1.5MB of 1MB (or 150%) even
though you still have 25% of the other file to send yet.
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>
> if (xfer_op->error != NULL)
> return;
> @@ -1948,9 +1952,6 @@ static void file_xfer_end_callback(GObject
> *source_object,
> * without GCancellabe */
> if (g_error_matches(xfer_op->error, G_IO_ERROR, G_IO_ERROR_CANCELLED) &&
> xfer_op->cancellable == NULL) {
> - SpiceFileTransferTask *xfer_task;
> -
> - xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
> spice_debug("file-transfer %u was cancelled",
> spice_file_transfer_task_get_id(xfer_task));
> g_clear_error(&xfer_op->error);
> @@ -3382,6 +3383,12 @@ static GCancellable
> *spice_file_transfer_task_get_cancellable(SpiceFileTransferT
> return self->cancellable;
> }
>
> +static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask
> *self)
> +{
> + g_return_val_if_fail(self != NULL, 0);
> + return self->file_size;
> +}
> +
> static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self,
> GError *error)
> {
> g_return_if_fail(self != NULL);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel