[Spice-devel] [PATCH spice-gtk v2 2/2] New file transfer API

[Pavel Grunt]

On Tue, 2015-10-06 at 11:55 -0500, Jonathon Jongsma wrote:
> On Tue, 2015-10-06 at 15:33 +0200, Pavel Grunt wrote:
> > Hi Jonathon,
> > 
> > thanks for the work, the user should have a way to cancel & monitor the file
> > transfer progress.
> > 
> > Ack from me, just a few comments below.
> > 
> > On Mon, 2015-10-05 at 13:39 -0500, Jonathon Jongsma wrote:
> > > There were several shortcomings to the existing file transfer API,
> > > particularly in terms of monitoring ongoing file transfers. The major
> > > issue is that spice_main_file_copy_async() allows you to pass an array
> > > of files, but the progress callback does not provide a way to
> > > identify which file the callback is associated with. This makes it
> > > nearly impossible for an application to monitor file transfers.
> > > 
> > > In addition, the SpiceDisplay widget automatically handles drag-and-drop
> > > actions on the widget, and initiates file transfers without allowing the
> > > application to specify a progress callback. So there's no way for an app
> > > to monitor file transfers that are initiated via drag and drop.
> > > 
> > > http://lists.freedesktop.org/archives/spice-devel/2015-September/021931.ht
> > > ml
> > > has a more detailed explanation of the issues.
> > > 
> > > This change doesn't break the existing API, but adds some new API that
> > > will allow an application to monitor file transfer progress, even for
> > > transfers that are initiated within spice-gtk itself.
> > > 
> > > - A new public SpiceFileTransferTask object is added.
> > > - The SpiceMainChannel object gains a "new-file-transfer" signal that is
> > >   emitted whenever a new file transfer is initiated. The
> > >   SpiceFileTransferTask object is passed to the signal handler.
> > > - The application can retain this object and monitor its 'progress'
> > >   property to be notified when the progress of the file transfer
> > >   changes. The SpiceFileTransferTask::finished signal indicates when the
> > >   given file transfer has completed. The application can also cancel the
> > >   file transfer by calling the _cancel() method.
> > > 
> > > The 'spicy' test application has been updated to use this new API and
> > > display a simple dialog showing the progress of individual files.
> > 
> > In general I don't like pop up dialogs, but they are ok for spicy.
> 
> I basically agree. I was thinking about doing the same for virt-viewer
> as a temporary solution until we can come up with a better design
> though. Or would you rather wait to implement it in virt-viewer until we
> have a better design?

I think the information is useful, so if you want to implement it :)

For instance I like (because it is not intrusive) what nautilus 3.18 is doing
for monitoring copying of files [1] - it shows a button with an icon acting like
a progress bar, if the user clicks on it, then it pops up additional
information. (But it is more a question about gtk2 or gtk3).

Pavel

[1] https://csorianognome.files.wordpress.com/2015/09/operations.png

> 
> 
> > > diff --git a/src/channel-main.h b/src/channel-main.h
> > > index f4fc005..6fb8395 100644
> > > --- a/src/channel-main.h
> > > +++ b/src/channel-main.h
> > > @@ -23,6 +23,7 @@
> > >  #endif
> > >  
> > >  #include "spice-channel.h"
> > > +#include "spice-file-transfer-task.h"
> > >  
> > this should be in spice-client.h, I also could not apply the patch because
> > of
> > it.
> 
> Oops, I had my header include changes on this branch. Sorry about that.
> 
> 
> > > +
> > > +#ifndef __SPICE_FILE_TRANSFER_TASK_H__
> > > +#define __SPICE_FILE_TRANSFER_TASK_H__
> > > +
> > > +#include <gio/gio.h>
> > > +#include <spice/vd_agent.h>
> > Why these includes? Just <glib-object.h> should be enough, no?
> 
> Not really sure where those came from... Must have been a previous
> iteration or something. Yes, glib-object.h should be enough.
> 
> 
> > 
> > There are some symbols undocumented. Please, take a look at
> > /doc/reference/spice-gtk-undocumented.txt to make gtk-doc happy.
> > 
> > Thanks,
> > Pavel
> > 
> > 
> 
> Ah, thanks. Will send a new version.
>