Ticket #2678: file-transfer-progress-v2.patch

css/10_header_bar.css

@@ +1 @@
+/*
+ * This file is part of Xpra.
+ * Copyright (C) 2020 Antoine Martin <antoine@xpra.org>
+ * Xpra is released under the terms of the GNU GPL v2, or, at your option, any
+ * later version. See the file COPYING for details.
+ *
+ * Try to make the header bar use less vertical space
+ * (why is it so hard?)
+ */
+
+headerbar entry,
+headerbar spinbutton,
+headerbar button,
+headerbar separator {
+    margin-top: 0px; /* same as headerbar side padding for nicer proportions */
+    margin-bottom: 0px;
+}
+
+headerbar {
+    min-height: 24px;
+    padding-left: 2px; /* same as childrens vertical margins for nicer proportions */
+    padding-right: 2px;
+    margin: 0px; /* same as headerbar side padding for nicer proportions */
+    padding: 0px;
+}

css/20_progress_bar.css

@@ +1 @@
+/*
+ * This file is part of Xpra.
+ * Copyright (C) 2020 Antoine Martin <antoine@xpra.org>
+ * Xpra is released under the terms of the GNU GPL v2, or, at your option, any
+ * later version. See the file COPYING for details.
+ *
+ * Make the progress bar more visible
+ * (but we can't set the min-width this way...)
+ */
+ 
+progress, trough {
+  min-height: 30px;
+}

setup.py

@@ -1682 +1682 @@
     add_data_files("%s/icons" % share_xpra,          glob.glob("icons/*png"))
     add_data_files("%s/content-type" % share_xpra,   glob.glob("content-type/*"))
     add_data_files("%s/content-categories" % share_xpra, glob.glob("content-categories/*"))
+    add_data_files("%s/css" % share_xpra,            glob.glob("./css/*"))
 
 
 if html5_ENABLED:

xpra/client/gtk_base/css_overrides.py

@@ +1 @@
+# This file is part of Xpra.
+# Copyright (C) 2020 Antoine Martin <antoine@xpra.org>
+# Xpra is released under the terms of the GNU GPL v2, or, at your option, any
+# later version. See the file COPYING for details.
+
+# load xpra's custom css overrides
+
+import os.path
+
+from xpra.util import envbool
+from xpra.platform.paths import get_resources_dir
+from xpra.log import Logger
+
+log = Logger("gtk", "util")
+
+CSS_OVERRIDES = envbool("XPRA_CSS_OVERRIDES", True)
+
+
+_done = False
+def inject_css_overrides():
+    global _done
+    if _done or not CSS_OVERRIDES:
+        return
+    _done = True
+
+    css_dir = os.path.join(get_resources_dir(), "css")
+    log("inject_css_overrides() css_dir=%s", css_dir)
+    from gi.repository import Gtk, Gdk
+    style_provider = Gtk.CssProvider()
+    filename = None
+    def parsing_error(_css_provider, _section, error):
+        log.error("Error: CSS parsing error on")
+        log.error(" '%s'", filename)
+        log.error(" %s", error)
+    style_provider.connect("parsing-error", parsing_error)
+    for f in sorted(os.listdir(css_dir)):
+        filename = os.path.join(css_dir, f)
+        try:
+            style_provider.load_from_path(filename)
+            log(" - loaded '%s'", filename)
+        except Exception as e:
+            log("load_from_path(%s)", filename, exc_info=True)
+            log.error("Error: CSS loading error on")
+            log.error(" '%s'", filename)
+            log.error(" %s", e)
+
+    Gtk.StyleContext.add_provider_for_screen(
+        Gdk.Screen.get_default(),
+        style_provider,
+        Gtk.STYLE_PROVIDER_PRIORITY_APPLICATION
+    )

xpra/client/gtk_base/gtk_client_base.py

@@ -35 +35 @@
 from xpra.client.ui_client_base import UIXpraClient
 from xpra.client.gobject_client_base import GObjectXpraClient
 from xpra.client.gtk_base.gtk_keyboard_helper import GTKKeyboardHelper
+from xpra.client.gtk_base.css_overrides import inject_css_overrides
 from xpra.client.mixins.window_manager import WindowClient
 from xpra.platform.paths import get_icon_filename
 from xpra.platform.gui import (
@@ -66 +67 @@
 NO_OPENGL_WINDOW_TYPES = os.environ.get("XPRA_NO_OPENGL_WINDOW_TYPES",
                                         "DOCK,TOOLBAR,MENU,UTILITY,SPLASH,DROPDOWN_MENU,POPUP_MENU,TOOLTIP,NOTIFICATION,COMBO,DND").split(",")
 
+inject_css_overrides()
 
+
 class GTKXpraClient(GObjectXpraClient, UIXpraClient):
     __gsignals__ = {}
     #add signals from super classes (all no-arg signals)
@@ -393 +396 @@
                 #record our response, so we will accept the file
                 self.data_send_requests[send_id] = (dtype, url, printit, newopenit)
             cb_answer(accept)
-        self.file_ask_dialog = getOpenRequestsWindow(self.show_file_upload)
+        self.file_ask_dialog = getOpenRequestsWindow(self.show_file_upload, self.cancel_download)
         self.file_ask_dialog.add_request(rec_answer, send_id, dtype, url, filesize, printit, openit, timeout)
         self.file_ask_dialog.show()
 
@@ -405 +408 @@
 
     def show_ask_data_dialog(self, *_args):
         from xpra.client.gtk_base.open_requests import getOpenRequestsWindow
-        self.file_ask_dialog = getOpenRequestsWindow(self.show_file_upload)
+        self.file_ask_dialog = getOpenRequestsWindow(self.show_file_upload, self.cancel_download)
         self.file_ask_dialog.show()
 
+    def transfer_progress_update(self, send=True, transfer_id=0, elapsed=0, position=0, total=0):
+        fad = self.file_ask_dialog
+        if fad:
+            self.idle_add(fad.transfer_progress_update, send, transfer_id, elapsed, position, total)
 
+
     def accept_data(self, send_id, dtype, url, printit, openit):
         #check if we have accepted this file via the GUI:
         r = self.data_send_requests.pop(send_id, None)

xpra/client/gtk_base/open_requests.py

@@ -10 +10 @@
 
 import gi
 gi.require_version("Gtk", "3.0")
+gi.require_version("Gdk", "3.0")
 gi.require_version("Pango", "1.0")
 from gi.repository import GLib, Gtk, GdkPixbuf, Pango
 
+from xpra.util import envint
+from xpra.os_util import monotonic_time, bytestostr, WIN32, OSX
 from xpra.gtk_common.gobject_compat import register_os_signals
-from xpra.util import envint
-from xpra.os_util import monotonic_time, bytestostr, get_util_logger, WIN32, OSX
+from xpra.client.gtk_base.css_overrides import inject_css_overrides
 from xpra.child_reaper import getChildReaper
 from xpra.net.file_transfer import ACCEPT, OPEN, DENY
-from xpra.simple_stats import std_unit_dec
+from xpra.simple_stats import std_unit, std_unit_dec
 from xpra.gtk_common.gtk_util import (
     add_close_accel, scaled_image,
     TableBuilder,
     )
 from xpra.platform.paths import get_icon_dir, get_download_dir
+from xpra.log import Logger
 
-log = get_util_logger()
+log = Logger("gtk", "file")
 
 URI_MAX_WIDTH = envint("XPRA_URI_MAX_WIDTH", 320)
 
+inject_css_overrides()
 
+
 _instance = None
-def getOpenRequestsWindow(show_file_upload_cb=None):
+def getOpenRequestsWindow(show_file_upload_cb=None, cancel_download=None):
     global _instance
     if _instance is None:
-        _instance = OpenRequestsWindow(show_file_upload_cb)
+        _instance = OpenRequestsWindow(show_file_upload_cb, cancel_download)
     return _instance
 
 
 class OpenRequestsWindow:
 
-    def __init__(self, show_file_upload_cb=None):
+    def __init__(self, show_file_upload_cb=None, cancel_download=None):
         self.show_file_upload_cb = show_file_upload_cb
+        self.cancel_download = cancel_download
         self.populate_timer = None
         self.table = None
         self.requests = []
         self.expire_labels = {}
+        self.progress_bars = {}
         self.window = Gtk.Window()
         self.window.set_border_width(20)
         self.window.connect("delete-event", self.close)
@@ -106 +113 @@
 
     def update_expires_label(self):
         expired = 0
-        for label, expiry in self.expire_labels.items():
+        for label, expiry in self.expire_labels.values():
             seconds = max(0, expiry-monotonic_time())
             label.set_text("%i" % seconds)
             if seconds==0:
@@ -141 +148 @@
                 if dtype==b"file" and filesize>0:
                     details = "%sB" % std_unit_dec(filesize)
                 expires_label = l()
-                self.expire_labels[expires_label] = expires
+                self.expire_labels[send_id] = (expires_label, expires)
                 buttons = self.action_buttons(cb_answer, send_id, dtype, printit, openit)
                 s = bytestostr(url)
                 main_label = l(s)
@@ -158 +165 @@
         self.alignment.add(self.table)
         self.table.show_all()
 
+    def remove_entry(self, send_id, can_close=True):
+        self.expire_labels.pop(send_id, None)
+        self.requests = [x for x in self.requests if x[1]!=send_id]
+        self.progress_bars.pop(send_id, None)
+        if not self.requests and can_close:
+            self.close()
+        else:
+            self.populate_table()
+            self.window.resize(1, 1)
+
     def action_buttons(self, cb_answer, send_id, dtype, printit, openit):
         hbox = Gtk.HBox()
         def remove_entry(can_close=False):
-            self.requests = [x for x in self.requests if x[1]!=send_id]
-            if not self.requests and can_close:
-                self.close()
-            else:
-                self.populate_table()
-                self.window.resize(1, 1)
+            self.remove_entry(send_id, can_close)
+        def show_progressbar():
+            expire = self.expire_labels.pop(send_id, None)
+            if expire:
+                expire_label = expire[0]
+                expire_label.set_text("")
+            for b in hbox.get_children():
+                hbox.remove(b)
+            def stop(*_args):
+                self.remove_entry(True)
+                self.cancel_download(send_id, "User cancelled")
+            stop_btn = self.btn("Stop", None, stop, "close.png")
+            hbox.pack_start(stop_btn)
+            pb = Gtk.ProgressBar()
+            hbox.set_spacing(20)
+            hbox.pack_start(pb)
+            hbox.show_all()
+            pb.set_size_request(420, 30)
+            self.progress_bars[send_id] = (pb, stop_btn)
+        def cancel(*_args):
+            remove_entry(True)
+            cb_answer(DENY)
         def ok(*_args):
             remove_entry(False)
             cb_answer(ACCEPT, False)
-        def okopen(*_args):
-            remove_entry(True)
-            cb_answer(ACCEPT, True)
         def remote(*_args):
             remove_entry(True)
             cb_answer(OPEN)
-        def cancel(*_args):
-            remove_entry(True)
-            cb_answer(DENY)
-        hbox.pack_start(self.btn("Cancel", None, cancel, "close.png"))
+        def progress(*_args):
+            cb_answer(ACCEPT)
+            show_progressbar()
+        def progressopen(*_args):
+            cb_answer(OPEN)
+            show_progressbar()
+        cancel_btn = self.btn("Cancel", None, cancel, "close.png")
+        hbox.pack_start(cancel_btn)
         if bytestostr(dtype)=="url":
             hbox.pack_start(self.btn("Open Locally", None, ok, "open.png"))
             hbox.pack_start(self.btn("Open on server", None, remote))
         elif printit:
-            hbox.pack_start(self.btn("Print", None, ok, "printer.png"))
+            hbox.pack_start(self.btn("Print", None, progress, "printer.png"))
         else:
-            hbox.pack_start(self.btn("Download", None, ok, "download.png"))
+            hbox.pack_start(self.btn("Download", None, progress, "download.png"))
             if openit:
-                hbox.pack_start(self.btn("Download and Open", None, okopen, "open.png"))
+                hbox.pack_start(self.btn("Download and Open", None, progressopen, "open.png"))
                 hbox.pack_start(self.btn("Open on server", None, remote))
         return hbox
 
@@ -203 +237 @@
             self.populate_timer = 0
 
 
+    def transfer_progress_update(self, send=True, transfer_id=0, elapsed=0, position=0, total=0):
+        buttons = self.progress_bars.get(transfer_id)
+        if not buttons:
+            #we're not tracking this transfer: no progress bar
+            return
+        pb, stop_btn = buttons
+        log("transfer_progress_update%s pb=%s", (send, transfer_id, elapsed, position, total), pb)
+        if position<0:
+            stop_btn.hide()
+            pb.set_text("Error: file transfer aborted")
+            GLib.timeout_add(5000, self.remove_entry, transfer_id)
+            return
+        if pb:
+            pb.set_fraction(position/total)
+            pb.set_text("%sB of %s" % (std_unit(position), std_unit(total)))
+            pb.set_show_text(True)
+        if position==total:
+            stop_btn.hide()
+            pb.set_text("Complete: %i bytes" % total)
+            pb.set_show_text(True)
+            GLib.timeout_add(5000, self.remove_entry, transfer_id)
+
+
     def show(self):
         log("show()")
         self.window.show_all()
@@ -235 +292 @@
             cmd = ["open", downloads]
         else:
             cmd = ["xdg-open", downloads]
-        proc = subprocess.Popen(cmd)
-        getChildReaper().add_process(proc, "show-downloads", cmd, ignore=True, forget=True)
+        try:
+            proc = subprocess.Popen(cmd)
+        except Exception as e:
+            log("show_downloads()", exc_info=True)
+            log.error("Error: failed to open 'Downloads' folder:")
+            log.error(" %s", e)
+        else:
+            getChildReaper().add_process(proc, "show-downloads", cmd, ignore=True, forget=True)
 
 
     def run(self):

xpra/net/file_transfer.py

@@ -37 +37 @@
 ACCEPT = 1
 OPEN = 2
 
+def osclose(fd):
+    try:
+        os.close(fd)
+    except OSError as e:
+        filelog("os.close(%s)", fd, exc_info=True)
+        filelog.error("Error closing file download:")
+        filelog.error(" %s", e)
+
 def basename(filename):
     #we can't use os.path.basename,
     #because the remote end may have sent us a filename
@@ -242 +250 @@
             }
         return info
 
-    def check_digest(self, filename, digest, expected_digest, algo="sha1"):
-        if digest!=expected_digest:
-            filelog.error("Error: data does not match, invalid %s file digest for '%s'", algo, filename)
-            filelog.error(" received %s, expected %s", digest, expected_digest)
-            raise Exception("failed %s digest verification" % algo)
-        else:
-            filelog("%s digest matches: %s", algo, digest)
 
+    def digest_mismatch(self, filename, digest, expected_digest, algo="sha1"):
+        filelog.error("Error: data does not match, invalid %s file digest for '%s'", algo, filename)
+        filelog.error(" received %s, expected %s", digest, expected_digest)
+        raise Exception("failed %s digest verification" % algo)
 
+
     def _check_chunk_receiving(self, chunk_id, chunk_no):
         chunk_state = self.receive_chunks_in_progress.get(chunk_id)
         filelog("_check_chunk_receiving(%s, %s) chunk_state=%s", chunk_id, chunk_no, chunk_state)
         if chunk_state:
+            if chunk_state[-4]:
+                #transfer has been cancelled
+                return
             chunk_state[-2] = 0     #this timer has been used
             if chunk_state[-1]==0:
                 filelog.error("Error: chunked file transfer timed out")
-                del self.receive_chunks_in_progress[chunk_id]
+                self.receive_chunks_in_progress.pop(chunk_id, None)
 
+    def cancel_download(self, send_id, message="Cancelled"):
+        filelog("cancel_download(%s, %s)", send_id, message)
+        for chunk_id, chunk_state in dict(self.receive_chunks_in_progress).items():
+            if chunk_state[-3]==send_id:
+                self.cancel_file(chunk_id, message)
+                return
+        filelog.error("Error: cannot cancel download %s, entry not found!", bytestostr(send_id))
+
+    def cancel_file(self, chunk_id, message, chunk=0):
+        filelog("cancel_file%s", (chunk_id, message, chunk))
+        chunk_state = self.receive_chunks_in_progress.get(chunk_id)
+        if chunk_state:
+            #mark it as cancelled:
+            chunk_state[-4] = True
+            #remove this transfer after a little while,
+            #so in-flight packets won't cause errors
+            def clean_receive_state():
+                self.receive_chunks_in_progress.pop(chunk_id, None)
+                return False
+            self.timeout_add(20000, clean_receive_state)
+        self.send("ack-file-chunk", chunk_id, False, message, chunk)
+
     def _process_send_file_chunk(self, packet):
         chunk_id, chunk, file_data, has_more = packet[1:5]
         filelog("_process_send_file_chunk%s", (chunk_id, chunk, "%i bytes" % len(file_data), has_more))
@@ -266 +297 @@
         chunk_state = self.receive_chunks_in_progress.get(chunk_id)
         if not chunk_state:
             filelog.error("Error: cannot find the file transfer id '%s'", nonl(bytestostr(chunk_id)))
-            self.send("ack-file-chunk", chunk_id, False, "file transfer id not found", chunk)
+            self.cancel_file(chunk_id, "file transfer id %s not found" % chunk_id, chunk)
             return
+        if chunk_state[-4]:
+            filelog("got chunk for a cancelled file transfer, ignoring it")
+            return
+        def progress(position):
+            start = chunk_state[0]
+            send_id = chunk_state[-3]
+            filesize = chunk_state[6]
+            self.transfer_progress_update(False, send_id, monotonic_time()-start, position, filesize)
         fd = chunk_state[1]
         if chunk_state[-1]+1!=chunk:
             filelog.error("Error: chunk number mismatch, expected %i but got %i", chunk_state[-1]+1, chunk)
-            self.send("ack-file-chunk", chunk_id, False, "chunk number mismatch", chunk)
-            del self.receive_chunks_in_progress[chunk_id]
-            os.close(fd)
+            self.cancel_file(chunk_id, "chunk number mismatch", chunk)
+            osclose(fd)
+            progress(-1)
             return
         #update chunk number:
         chunk_state[-1] = chunk
@@ -287 +326 @@
         except OSError as e:
             filelog.error("Error: cannot write file chunk")
             filelog.error(" %s", e)
-            self.send("ack-file-chunk", chunk_id, False, "write error: %s" % e, chunk)
-            del self.receive_chunks_in_progress[chunk_id]
-            try:
-                os.close(fd)
-            except OSError:
-                pass
+            self.cancel_file(chunk_id, "write error: %s" % e, chunk)
+            osclose(fd)
+            progress(-1)
             return
         self.send("ack-file-chunk", chunk_id, True, "", chunk)
         if has_more:
+            progress(written)
             timer = chunk_state[-2]
             if timer:
                 self.source_remove(timer)
@@ -303 +340 @@
             timer = self.timeout_add(CHUNK_TIMEOUT, self._check_chunk_receiving, chunk_id, chunk)
             chunk_state[-2] = timer
             return
-        del self.receive_chunks_in_progress[chunk_id]
-        os.close(fd)
+        self.receive_chunks_in_progress.pop(chunk_id, None)
+        osclose(fd)
         #check file size and digest then process it:
         filename, mimetype, printit, openit, filesize, options = chunk_state[2:8]
         if written!=filesize:
             filelog.error("Error: expected a file of %i bytes, got %i", filesize, written)
+            progress(-1)
             return
         expected_digest = options.get("sha1")
-        if expected_digest:
-            self.check_digest(filename, digest.hexdigest(), expected_digest)
+        if expected_digest and digest.hexdigest()!=expected_digest:
+            progress(-1)
+            self.digest_mismatch(filename, digest, expected_digest, "sha1")
+            return
+
+        progress(written)
         start_time = chunk_state[0]
         elapsed = monotonic_time()-start_time
         mimetype = bytestostr(mimetype)
@@ -397 +439 @@
                 monotonic_time(),
                 fd, filename, mimetype,
                 printit, openit, filesize,
-                options, digest, 0, timer, chunk,
+                options, digest, 0, False, send_id,
+                timer, chunk,
                 ]
             self.receive_chunks_in_progress[chunk_id] = chunk_state
             self.send("ack-file-chunk", chunk_id, True, "", chunk)
@@ -415 +458 @@
                 u = libfn()
                 u.update(file_data)
                 l("%s digest: %s - expected: %s", algo, u.hexdigest(), digest)
-                self.check_digest(basefilename, u.hexdigest(), digest, algo)
+                if digest!=u.hexdigest():
+                    self.digest_mismatch(filename, digest, u.hexdigest(), algo)
         check_digest("sha1", hashlib.sha1)
         check_digest("md5", hashlib.md5)
         try:
@@ -790 +834 @@
         chunk_state = self.send_chunks_in_progress.get(chunk_id)
         filelog("_check_chunk_sending(%s, %s) chunk_state found: %s", chunk_id, chunk_no, bool(chunk_state))
         if chunk_state:
-            chunk_state[-2] = 0         #timer has fired
+            chunk_state[3] = 0         #timer has fired
             if chunk_state[-1]==chunk_no:
                 filelog.error("Error: chunked file transfer timed out on chunk %i", chunk_no)
-                try:
-                    del self.send_chunks_in_progress[chunk_id]
-                except KeyError:
-                    pass
+                self.cancel_sending(chunk_id)
 
+    def cancel_sending(self, chunk_id):
+        chunk_state = self.send_chunks_in_progress.pop(chunk_id, None)
+        if chunk_state:
+            timer = chunk_state[3]
+            if timer:
+                self.source_remove(timer)
+                chunk_state[3] = 0
+
     def _process_ack_file_chunk(self, packet):
         #the other end received our send-file or send-file-chunk,
         #send some more file data
@@ -804 +853 @@
         filelog("ack-file-chunk: %s", packet[1:])
         chunk_id, state, error_message, chunk = packet[1:5]
         if not state:
-            filelog.error("Error: remote end is cancelling the file transfer:")
-            filelog.error(" %s", error_message)
-            del self.send_chunks_in_progress[chunk_id]
+            filelog.info("the remote end is cancelling the file transfer:")
+            filelog.info(" %s", bytestostr(error_message))
+            self.cancel_sending(chunk_id)
             return
         chunk_id = bytestostr(chunk_id)
         chunk_state = self.send_chunks_in_progress.get(chunk_id)
@@ -815 +864 @@
             return
         if chunk_state[-1]!=chunk:
             filelog.error("Error: chunk number mismatch (%i vs %i)", chunk_state, chunk)
-            del self.send_chunks_in_progress[chunk_id]
+            self.cancel_sending(chunk_id)
             return
         start_time, data, chunk_size, timer, chunk = chunk_state
         if not data:
@@ -823 +872 @@
             elapsed = monotonic_time()-start_time
             filelog("%i chunks of %i bytes sent in %ims (%sB/s)",
                     chunk, chunk_size, elapsed*1000, std_unit(chunk*chunk_size/elapsed))
-            del self.send_chunks_in_progress[chunk_id]
+            self.cancel_sending(chunk_id)
             return
         assert chunk_size>0
         #carve out another chunk:
@@ -841 +890 @@
 
     def compressed_wrapper(self, datatype, data, level=5):
         raise NotImplementedError()
+
+    def transfer_progress_update(self, send=True, transfer_id=0, elapsed=0, position=0, total=0):
+        #this method is overriden in the gtk client:
+        filelog("transfer_progress_update%s", (send, transfer_id, elapsed, position, total))