[virt-tools-list] [PATCH virt-viewer v3] RFC: Simplify file transfer dialog UI

Jonathon Jongsma jjongsma at redhat.com
Wed Aug 17 18:21:16 UTC 2016


When transferring a large number of files, the file transfer dialog was
unusable because the window size would be larger than the client
desktop. To solve this, remove the list of individual files (and the
ability to cancel each file transfer independantly) and only display
a single overall progress bar that shows the status of all ongoing
transfers.

This also allows us to remove the delayed unref of the task since we
don't need to show the task information about each individual transfer
task until the window is closed. Removes TaskFinishedData type.

This patch requires new API from spice-gtk to calculate the overall
progress:
 spice_file_transfer_task_get_total_bytes()
 spice_file_transfer_task_get_transferred_bytes()
---
Changes in v3:
 - Remove the TaskFinishedData struct as mentioned in the commit log above.

 src/Makefile.am                                    |   1 +
 .../ui/virt-viewer-file-transfer-dialog.ui         |  87 ++++++++++
 src/resources/virt-viewer.gresource.xml            |   1 +
 src/virt-viewer-file-transfer-dialog.c             | 177 +++++++--------------
 4 files changed, 148 insertions(+), 118 deletions(-)
 create mode 100644 src/resources/ui/virt-viewer-file-transfer-dialog.ui

diff --git a/src/Makefile.am b/src/Makefile.am
index 0c48e40..272c4ff 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -13,6 +13,7 @@ noinst_DATA = \
 	resources/ui/virt-viewer-vm-connection.ui \
 	resources/ui/virt-viewer-preferences.ui \
 	resources/ui/remote-viewer-connect.ui \
+	resources/ui/virt-viewer-file-transfer-dialog.ui \
 	$(NULL)
 
 EXTRA_DIST =					\
diff --git a/src/resources/ui/virt-viewer-file-transfer-dialog.ui b/src/resources/ui/virt-viewer-file-transfer-dialog.ui
new file mode 100644
index 0000000..e3514d6
--- /dev/null
+++ b/src/resources/ui/virt-viewer-file-transfer-dialog.ui
@@ -0,0 +1,87 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<interface>
+  <requires lib="gtk+" version="2.24"/>
+  <!-- interface-naming-policy project-wide -->
+  <template class="VirtViewerFileTransferDialog" parent="GtkDialog">
+    <property name="default_width">400</property>
+    <property name="can_focus">False</property>
+    <property name="border_width">5</property>
+    <property name="type_hint">dialog</property>
+    <child internal-child="vbox">
+      <object class="GtkVBox" id="dialog-vbox1">
+        <property name="visible">True</property>
+        <property name="can_focus">False</property>
+        <property name="spacing">12</property>
+        <property name="border-width">12</property>
+        <child internal-child="action_area">
+          <object class="GtkHButtonBox" id="dialog-action_area1">
+            <property name="visible">True</property>
+            <property name="can_focus">False</property>
+            <property name="layout_style">end</property>
+            <child>
+              <placeholder/>
+            </child>
+            <child>
+              <object class="GtkButton" id="button1">
+                <property name="label">gtk-cancel</property>
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="receives_default">True</property>
+                <property name="use_underline">True</property>
+                <property name="use_stock">True</property>
+              </object>
+              <packing>
+                <property name="expand">False</property>
+                <property name="fill">False</property>
+                <property name="position">1</property>
+              </packing>
+            </child>
+          </object>
+          <packing>
+            <property name="expand">True</property>
+            <property name="fill">True</property>
+            <property name="position">0</property>
+          </packing>
+        </child>
+        <child>
+          <object class="GtkVBox" id="vbox1">
+            <property name="visible">True</property>
+            <property name="can_focus">False</property>
+            <property name="spacing">12</property>
+            <child>
+              <object class="GtkLabel" id="transfer_summary">
+                <property name="visible">True</property>
+                <property name="can_focus">False</property>
+                <property name="label" translatable="yes">label</property>
+              </object>
+              <packing>
+                <property name="expand">True</property>
+                <property name="fill">True</property>
+                <property name="position">0</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkProgressBar" id="progressbar">
+                <property name="visible">True</property>
+                <property name="can_focus">False</property>
+              </object>
+              <packing>
+                <property name="expand">True</property>
+                <property name="fill">True</property>
+                <property name="position">1</property>
+              </packing>
+            </child>
+          </object>
+          <packing>
+            <property name="expand">True</property>
+            <property name="fill">True</property>
+            <property name="position">1</property>
+          </packing>
+        </child>
+      </object>
+    </child>
+    <action-widgets>
+      <action-widget response="-6">button1</action-widget>
+    </action-widgets>
+  </template>
+</interface>
diff --git a/src/resources/virt-viewer.gresource.xml b/src/resources/virt-viewer.gresource.xml
index b8ced29..f9b4a9f 100644
--- a/src/resources/virt-viewer.gresource.xml
+++ b/src/resources/virt-viewer.gresource.xml
@@ -8,6 +8,7 @@
     <file>ui/virt-viewer-preferences.ui</file>
     <file>ui/virt-viewer-vm-connection.ui</file>
     <file>ui/virt-viewer.ui</file>
+    <file>ui/virt-viewer-file-transfer-dialog.ui</file>
     <file alias="icons/16x16/virt-viewer.png">../../icons/16x16/virt-viewer.png</file>
     <file alias="icons/22x22/virt-viewer.png">../../icons/22x22/virt-viewer.png</file>
     <file alias="icons/24x24/virt-viewer.png">../../icons/24x24/virt-viewer.png</file>
diff --git a/src/virt-viewer-file-transfer-dialog.c b/src/virt-viewer-file-transfer-dialog.c
index c8b50f0..3cddbd3 100644
--- a/src/virt-viewer-file-transfer-dialog.c
+++ b/src/virt-viewer-file-transfer-dialog.c
@@ -23,26 +23,31 @@
 #include "virt-viewer-file-transfer-dialog.h"
 #include <glib/gi18n.h>
 
-G_DEFINE_TYPE(VirtViewerFileTransferDialog, virt_viewer_file_transfer_dialog, GTK_TYPE_DIALOG)
-
-#define FILE_TRANSFER_DIALOG_PRIVATE(o) \
-        (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG, VirtViewerFileTransferDialogPrivate))
-
 struct _VirtViewerFileTransferDialogPrivate
 {
     /* GHashTable<SpiceFileTransferTask, widgets> */
-    GHashTable *file_transfers;
+    GSList *file_transfers;
     guint timer_show_src;
     guint timer_hide_src;
+    GtkWidget *transfer_summary;
+    GtkWidget *progressbar;
 };
 
+G_DEFINE_TYPE_WITH_PRIVATE(VirtViewerFileTransferDialog, virt_viewer_file_transfer_dialog, GTK_TYPE_DIALOG)
+
+#define FILE_TRANSFER_DIALOG_PRIVATE(o) \
+        (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG, VirtViewerFileTransferDialogPrivate))
+
 
 static void
 virt_viewer_file_transfer_dialog_dispose(GObject *object)
 {
     VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(object);
 
-    g_clear_pointer(&self->priv->file_transfers, g_hash_table_unref);
+    if (self->priv->file_transfers) {
+        g_slist_free_full(self->priv->file_transfers, g_object_unref);
+        self->priv->file_transfers = NULL;
+    }
 
     G_OBJECT_CLASS(virt_viewer_file_transfer_dialog_parent_class)->dispose(object);
 }
@@ -51,8 +56,16 @@ static void
 virt_viewer_file_transfer_dialog_class_init(VirtViewerFileTransferDialogClass *klass)
 {
     GObjectClass *object_class = G_OBJECT_CLASS(klass);
+    GtkWidgetClass *widget_class = GTK_WIDGET_CLASS(klass);
 
-    g_type_class_add_private(klass, sizeof(VirtViewerFileTransferDialogPrivate));
+    gtk_widget_class_set_template_from_resource(widget_class,
+                                                "/org/virt-manager/virt-viewer/ui/virt-viewer-file-transfer-dialog.ui");
+    gtk_widget_class_bind_template_child_private(widget_class,
+                                                 VirtViewerFileTransferDialog,
+                                                 transfer_summary);
+    gtk_widget_class_bind_template_child_private(widget_class,
+                                                 VirtViewerFileTransferDialog,
+                                                 progressbar);
 
     object_class->dispose = virt_viewer_file_transfer_dialog_dispose;
 }
@@ -63,16 +76,13 @@ dialog_response(GtkDialog *dialog,
                 gpointer user_data G_GNUC_UNUSED)
 {
     VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(dialog);
-    GHashTableIter iter;
-    gpointer key, value;
+    GSList *slist = self->priv->file_transfers;
 
     switch (response_id) {
         case GTK_RESPONSE_CANCEL:
             /* cancel all current tasks */
-            g_hash_table_iter_init(&iter, self->priv->file_transfers);
-
-            while (g_hash_table_iter_next(&iter, &key, &value)) {
-                spice_file_transfer_task_cancel(SPICE_FILE_TRANSFER_TASK(key));
+            for (slist = self->priv->file_transfers; slist != NULL; slist = g_slist_next(slist)) {
+                spice_file_transfer_task_cancel(SPICE_FILE_TRANSFER_TASK(slist->data));
             }
             break;
         case GTK_RESPONSE_DELETE_EVENT:
@@ -83,53 +93,6 @@ dialog_response(GtkDialog *dialog,
     }
 }
 
-static void task_cancel_clicked(GtkButton *button G_GNUC_UNUSED,
-                                gpointer user_data)
-{
-    SpiceFileTransferTask *task = user_data;
-    spice_file_transfer_task_cancel(task);
-}
-
-typedef struct {
-    GtkWidget *vbox;
-    GtkWidget *hbox;
-    GtkWidget *progress;
-    GtkWidget *label;
-    GtkWidget *cancel;
-} TaskWidgets;
-
-static TaskWidgets *task_widgets_new(SpiceFileTransferTask *task)
-{
-    TaskWidgets *w = g_new0(TaskWidgets, 1);
-    gchar *filename;
-
-    w->vbox = gtk_box_new(GTK_ORIENTATION_VERTICAL, 6);
-    w->hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 12);
-    w->progress = gtk_progress_bar_new();
-    filename = spice_file_transfer_task_get_filename(task);
-    w->label = gtk_label_new(filename);
-    g_free(filename);
-    w->cancel = gtk_button_new_from_icon_name("gtk-cancel", GTK_ICON_SIZE_SMALL_TOOLBAR);
-    gtk_widget_set_hexpand(w->progress, TRUE);
-    gtk_widget_set_valign(w->progress, GTK_ALIGN_CENTER);
-    gtk_widget_set_hexpand(w->label, TRUE);
-    gtk_widget_set_valign(w->label, GTK_ALIGN_END);
-    gtk_widget_set_halign(w->label, GTK_ALIGN_START);
-    gtk_widget_set_hexpand(w->cancel, FALSE);
-    gtk_widget_set_valign(w->cancel, GTK_ALIGN_CENTER);
-
-    g_signal_connect(w->cancel, "clicked",
-                     G_CALLBACK(task_cancel_clicked), task);
-
-    gtk_box_pack_start(GTK_BOX(w->hbox), w->progress, TRUE, TRUE, 0);
-    gtk_box_pack_start(GTK_BOX(w->hbox), w->cancel, FALSE, TRUE, 0);
-    gtk_box_pack_start(GTK_BOX(w->vbox), w->label, TRUE, TRUE, 0);
-    gtk_box_pack_start(GTK_BOX(w->vbox), w->hbox, TRUE, TRUE, 0);
-
-    gtk_widget_show_all(w->vbox);
-    return w;
-}
-
 static gboolean delete_event(GtkWidget *widget,
                              GdkEvent *event G_GNUC_UNUSED,
                              gpointer user_data G_GNUC_UNUSED)
@@ -143,18 +106,10 @@ static gboolean delete_event(GtkWidget *widget,
 static void
 virt_viewer_file_transfer_dialog_init(VirtViewerFileTransferDialog *self)
 {
-    GtkBox *content = GTK_BOX(gtk_dialog_get_content_area(GTK_DIALOG(self)));
+    gtk_widget_init_template(GTK_WIDGET(self));
 
     self->priv = FILE_TRANSFER_DIALOG_PRIVATE(self);
 
-    gtk_widget_set_size_request(GTK_WIDGET(content), 400, -1);
-    gtk_container_set_border_width(GTK_CONTAINER(content), 12);
-    self->priv->file_transfers = g_hash_table_new_full(g_direct_hash, g_direct_equal,
-                                                       g_object_unref,
-                                                       (GDestroyNotify)g_free);
-    gtk_dialog_add_button(GTK_DIALOG(self), _("Cancel"), GTK_RESPONSE_CANCEL);
-    gtk_dialog_set_default_response(GTK_DIALOG(self),
-                                    GTK_RESPONSE_CANCEL);
     g_signal_connect(self, "response", G_CALLBACK(dialog_response), NULL);
     g_signal_connect(self, "delete-event", G_CALLBACK(delete_event), NULL);
 }
@@ -169,17 +124,38 @@ virt_viewer_file_transfer_dialog_new(GtkWindow *parent)
                         NULL);
 }
 
-static void task_progress_notify(GObject *object,
+static void update_global_progress(VirtViewerFileTransferDialog *self)
+{
+    GSList *slist;
+    guint64 total = 0, transferred = 0;
+    gchar *message = NULL;
+    guint n_files = 0;
+    gdouble fraction = 1.0;
+
+    for (slist = self->priv->file_transfers; slist != NULL; slist = g_slist_next(slist)) {
+        SpiceFileTransferTask *task = slist->data;
+        total += spice_file_transfer_task_get_total_bytes(task);
+        transferred += spice_file_transfer_task_get_transferred_bytes(task);
+        n_files++;
+    }
+
+    if (n_files > 0)
+        fraction = (gdouble)transferred / total;
+    message = g_strdup_printf(ngettext("Transferring %d file...",
+                                       "Transferring %d files...", n_files),
+                              n_files);
+    gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(self->priv->progressbar), fraction);
+    gtk_label_set_text(GTK_LABEL(self->priv->transfer_summary), message);
+    g_free(message);
+}
+
+static void task_progress_notify(GObject *object G_GNUC_UNUSED,
                                  GParamSpec *pspec G_GNUC_UNUSED,
                                  gpointer user_data)
 {
     VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(user_data);
-    SpiceFileTransferTask *task = SPICE_FILE_TRANSFER_TASK(object);
-    TaskWidgets *w = g_hash_table_lookup(self->priv->file_transfers, task);
-    g_return_if_fail(w);
 
-    double pct = spice_file_transfer_task_get_progress(task);
-    gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(w->progress), pct);
+    update_global_progress(self);
 }
 
 static gboolean hide_transfer_dialog(gpointer data)
@@ -193,51 +169,21 @@ static gboolean hide_transfer_dialog(gpointer data)
     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);
 
     if (error && !g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
         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;
-
-    g_timeout_add(500, task_finished_remove, d);
-
-    g_hash_table_steal(self->priv->file_transfers, task);
+    self->priv->file_transfers = g_slist_remove(self->priv->file_transfers, task);
+    g_object_unref(task);
+    update_global_progress(self);
 
     /* if this is the last transfer, close the dialog */
-    if (!g_hash_table_size(d->self->priv->file_transfers)) {
+    if (!g_slist_length(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) {
@@ -245,7 +191,7 @@ static void task_finished(SpiceFileTransferTask *task,
             self->priv->timer_show_src = 0;
         }
         self->priv->timer_hide_src = g_timeout_add(500, hide_transfer_dialog,
-                                                   d->self);
+                                                   self);
     }
 }
 
@@ -254,6 +200,7 @@ static gboolean show_transfer_dialog_delayed(gpointer user_data)
     VirtViewerFileTransferDialog *self = user_data;
 
     self->priv->timer_show_src = 0;
+    update_global_progress(self);
     gtk_widget_show(GTK_WIDGET(self));
 
     return G_SOURCE_REMOVE;
@@ -282,13 +229,7 @@ static void show_transfer_dialog(VirtViewerFileTransferDialog *self)
 void virt_viewer_file_transfer_dialog_add_task(VirtViewerFileTransferDialog *self,
                                                SpiceFileTransferTask *task)
 {
-    GtkBox *content = GTK_BOX(gtk_dialog_get_content_area(GTK_DIALOG(self)));
-    TaskWidgets *w = task_widgets_new(task);
-
-    gtk_box_pack_start(content,
-                       w->vbox,
-                       TRUE, TRUE, 12);
-    g_hash_table_insert(self->priv->file_transfers, g_object_ref(task), w);
+    self->priv->file_transfers = g_slist_prepend(self->priv->file_transfers, g_object_ref(task));
     g_signal_connect(task, "notify::progress", G_CALLBACK(task_progress_notify), self);
     g_signal_connect(task, "finished", G_CALLBACK(task_finished), self);
 
-- 
2.7.4




More information about the virt-tools-list mailing list