[virt-tools-list] [PATCH virt-viewer v2] Add some timeouts to file transfer dialog

[Victor Toso]

Hey,

On Mon, Apr 25, 2016 at 05:34:42PM -0500, Jonathon Jongsma wrote:
> In order to avoid the situation where a dialog flashes onto the screen
> and then is immediately hidden, I've added a couple timeouts to the
> dialog.
>
> The first is a 250ms timeout before showing the dialog. This avoids
> showing the dialog at all for very small, quick transfers.
>
> There is also a 500ms timeout before hiding a finished task. This
> ensures that even transfers that only take e.g. 251ms to transfer will
> get shown to the user for at least 500ms rather than being hidden 1ms
> after showing the dialog.

If we had the network information (ping/bandwidth) we would know how long the
file-transfer would take...That would allows us to remove the first timeout. But
this comment is just some thought for the future, I think we are lacking the network
information in the client for quite some time now although we have the ping/pong
messages in spice-gtk.

Booth patches look good to me
Reviewed-by: Victor Toso <victortoso at redhat.com>

> ---
> Changes:
>  - Previous patch didn't actually work to avoid showing a dialog when
>    transferring very small files. This patch fixes the issues.
>
>  src/virt-viewer-file-transfer-dialog.c | 88 ++++++++++++++++++++++++++++++++--
>  1 file changed, 83 insertions(+), 5 deletions(-)
> 
> diff --git a/src/virt-viewer-file-transfer-dialog.c b/src/virt-viewer-file-transfer-dialog.c
> index bd85d82..d2250bd 100644
> --- a/src/virt-viewer-file-transfer-dialog.c
> +++ b/src/virt-viewer-file-transfer-dialog.c
> @@ -30,6 +30,8 @@ struct _VirtViewerFileTransferDialogPrivate
>  {
>      /* GHashTable<SpiceFileTransferTask, widgets> */
>      GHashTable *file_transfers;
> +    guint timer_show_src;
> +    guint timer_hide_src;
>  };
>  
>  
> @@ -175,10 +177,41 @@ static void task_progress_notify(GObject *object,
>      gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(w->progress), pct);
>  }
>  
> +static gboolean hide_transfer_dialog(gpointer data)
> +{
> +    VirtViewerFileTransferDialog *self = data;
> +    gtk_widget_hide(GTK_WIDGET(self));
> +    gtk_dialog_set_response_sensitive(GTK_DIALOG(self),
> +                                      GTK_RESPONSE_CANCEL, FALSE);
> +    self->priv->timer_hide_src = 0;
> +
> +    return G_SOURCE_REMOVE;
> +}
> +
> +typedef struct {
> +    VirtViewerFileTransferDialog *self;
> +    TaskWidgets *widgets;
> +    SpiceFileTransferTask *task;
> +} TaskFinishedData;
> +
> +static gboolean task_finished_remove(gpointer user_data)
> +{
> +    TaskFinishedData *d = user_data;
> +
> +    gtk_widget_destroy(d->widgets->vbox);
> +
> +    g_free(d->widgets);
> +    g_object_unref(d->task);
> +    g_free(d);
> +
> +    return G_SOURCE_REMOVE;
> +}
> +
>  static void task_finished(SpiceFileTransferTask *task,
>                            GError *error,
>                            gpointer user_data)
>  {
> +    TaskFinishedData *d;
>      VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(user_data);
>      TaskWidgets *w = g_hash_table_lookup(self->priv->file_transfers, task);
>  
> @@ -186,14 +219,59 @@ static void task_finished(SpiceFileTransferTask *task,
>          g_warning("File transfer task %p failed: %s", task, error->message);
>  
>      g_return_if_fail(w);
> +    gtk_widget_set_sensitive(w->cancel, FALSE);
> +
> +
> +    d = g_new0(TaskFinishedData, 1);
> +    d->self = self;
> +    d->widgets = w;
> +    d->task = task;
>  
> -    gtk_widget_destroy(w->vbox);
> +    g_timeout_add(500, task_finished_remove, d);
>  
> -    g_hash_table_remove(self->priv->file_transfers, task);
> +    g_hash_table_steal(self->priv->file_transfers, task);
>  
>      /* if this is the last transfer, close the dialog */
> -    if (!g_hash_table_size(self->priv->file_transfers))
> -        gtk_widget_hide(GTK_WIDGET(self));
> +    if (!g_hash_table_size(d->self->priv->file_transfers)) {
> +        /* cancel any pending 'show' operations if all tasks complete before
> +         * the dialog can be shown */
> +        if (self->priv->timer_show_src) {
> +            g_source_remove(self->priv->timer_show_src);
> +            self->priv->timer_show_src = 0;
> +        }
> +        self->priv->timer_hide_src = g_timeout_add(500, hide_transfer_dialog,
> +                                                   d->self);
> +    }
> +}
> +
> +static gboolean show_transfer_dialog_delayed(gpointer user_data)
> +{
> +    VirtViewerFileTransferDialog *self = user_data;
> +
> +    self->priv->timer_show_src = 0;
> +    gtk_widget_show(GTK_WIDGET(self));
> +
> +    return G_SOURCE_REMOVE;
> +}
> +
> +static void show_transfer_dialog(VirtViewerFileTransferDialog *self)
> +{
> +    /* if there's a pending 'hide', cancel it */
> +    if (self->priv->timer_hide_src) {
> +        g_source_remove(self->priv->timer_hide_src);
> +        self->priv->timer_hide_src = 0;
> +    }
> +
> +    /* don't show the dialog immediately. For very quick transfers, it doesn't
> +     * make sense to show a dialog and immediately hide it. But if there's
> +     * already a pending 'show' operation, don't trigger another one */
> +    if (self->priv->timer_show_src == 0)
> +        self->priv->timer_show_src = g_timeout_add(250,
> +                                                   show_transfer_dialog_delayed,
> +                                                   self);
> +
> +    gtk_dialog_set_response_sensitive(GTK_DIALOG(self),
> +                                      GTK_RESPONSE_CANCEL, TRUE);
>  }
>  
>  void virt_viewer_file_transfer_dialog_add_task(VirtViewerFileTransferDialog *self,
> @@ -209,5 +287,5 @@ void virt_viewer_file_transfer_dialog_add_task(VirtViewerFileTransferDialog *sel
>      g_signal_connect(task, "notify::progress", G_CALLBACK(task_progress_notify), self);
>      g_signal_connect(task, "finished", G_CALLBACK(task_finished), self);
>  
> -    gtk_widget_show(GTK_WIDGET(self));
> +    show_transfer_dialog(self);
>  }
> -- 
> 2.4.11
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list