Adding status info to DownloadUIItem and piping it through.

components/offline_pages/core/downloads/download_ui_item.h

Index: components/offline_pages/core/downloads/download_ui_item.h
diff --git a/components/offline_pages/core/downloads/download_ui_item.h b/components/offline_pages/core/downloads/download_ui_item.h
index aea7cf5ccebb9b8e8dc8d698d049b4c4051117f6..18648cfa3b75dc423f06d5230079931accb484c7 100644
--- a/components/offline_pages/core/downloads/download_ui_item.h
+++ b/components/offline_pages/core/downloads/download_ui_item.h
@@ -18,20 +18,54 @@ namespace offline_pages {
 struct OfflinePageItem;
 class SavePageRequest;
 
+// The abstract "download item" that may be a media file, a web page (together
+// with all the resources) or a PWA web package. This is a data bag that exposes
+// only bits potentially visible by the user, not the internal data.
 struct DownloadUIItem {
  public:
+  // All Items start from PENDING and terminate at COMPLETED or are removed.
+  // In sync with state values in Java OfflinePageDownloadItem class.
+  enum DownloadState {

[fgorski, 2017/01/20 17:03:02]

Please, put this line above:
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.offlinepages.downloads

And add entry in omponents/offline_pages/core/BUILD.gn in
offline_page_model_enums_java section.

Which will generate an enum and things will be kept in sync for you.

[Dmitry Titov, 2017/01/27 04:26:24]

Nice advice, thanks! 

+    PENDING = 0,
+    DOWNLOADING = 1,
+    PAUSED = 2,
+    COMPLETED = 3,
+  };
+
+  // The abstract notion of download progress. The |current| is always less

[fgorski, 2017/01/20 17:03:02]

less or "less or equal" ? 

[Dmitry Titov, 2017/01/27 04:26:25]

Removed the whole concept of current/max progress, leaved the bytes_downloaded
since this is the most likely candidate for implementation now, and also the
kind supported by DownlaodHome and mocks...

+  // than |max|, the download 'percentage' at any moment is current/max.
+  // Note the |max| can change over time, as more resources are discovered from
+  // the bytes already loaded (which is often happening when a web page is
+  // loaded and more resources are found in the markup).
+  // Typical implementation would use the byte count loaded (current) and total
+  // of Content-Size: headers of resources that are already known to be part of
+  // the page (or approximation if there is no exact data yet).
+  struct DownloadProgress {
+    DownloadProgress(int64_t current, int64_t max)
+      : current(current),
+        max(max) {}
+
+    int64_t current;

[dewittj, 2017/01/18 19:24:07]

s/current/current_bytes/ (or whatever unit it represents)

[Dmitry Titov, 2017/01/27 04:26:24]

Done. Replaced with just byte counter.

+    int64_t max;
+  };
+
   DownloadUIItem();
   explicit DownloadUIItem(const OfflinePageItem& page);
   explicit DownloadUIItem(const SavePageRequest& request);
   DownloadUIItem(const DownloadUIItem& other);
   ~DownloadUIItem();
 
-  // Unique id.
+  // Unique id. Not necesarily a GUID, may be any string unique in the Chrome
+  // instance across all DownloadUIItem providers.

[dewittj, 2017/01/18 19:24:07]

I'm not sure across all providers is a requirement.

[Dmitry Titov, 2017/01/27 04:26:25]

Right, it is not. I believe we always keep a bit "is this item an offline page"
separately for now so we never send the ids of pages to Download Backend and
vice versa.

   std::string guid;

[dewittj, 2017/01/18 19:24:07]

nit: can we rename guid?

[Dmitry Titov, 2017/01/27 04:26:25]

Looks like a separate CL... That CL can also start putting an offline_id there
instead of guid and we also can stop generating guids for client_id for
Downloads and async_load namespaces... This also will make this adapter work for
Recent tabs.

 
   // The URL of the captured page.
   GURL url;
 
+  DownloadState download_state;
+
+  DownloadProgress download_progress;
+
   // The Title of the captured page, if any. It can be empty string either
   // because the page is not yet fully loaded, or because it doesn't have any.
   base::string16 title;