[Background Fetch] Use downloadTotal in progress UI.

downloadTotal is a field that can be set by the developer from the fetch() API, through
BackgroundFetchOptions (https://wicg.github.io/background-fetch/#dictdef-backgroundfetchoptions).

Before this change, the progress bar in the UI showed a percentage based on number of files
downloaded. After this change, the progress bar will use bytes downloaded out of total bytes to be
downloaded to calculate this percentage.

If downloadTotal is set incorrectly (larger than total size of downloads), or not set at all, we
fall back to previous behavior. Additionally, we update the progress bar more often with this
change -- whenever DownloadManager notifies us of more bytes downloaded (~500 ms) instead of only
updating when a download is completed or aborted.

Bug: 813565
Change-Id: Ia704ea99b9ee3966aabb3f64a4ccd2112e36ab80
Reviewed-on: https://chromium-review.googlesource.com/998105
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557243}

chrome/browser/background_fetch/background_fetch_browsertest.cc

@@ -50,6 +50,13 @@ const char kResultAccepted[] = "ACCEPTED";
 // Title of a Background Fetch started by StartSingleFileDownload().
 const char kSingleFileDownloadTitle[] = "Single-file Background Fetch";
 
+// Size of the downloaded resource, used in BackgroundFetch tests.
+const int kDownloadedResourceSizeInBytes = 82;
+
+// Incorrect downloadTotal, set in the JavaScript file loaded by this test,
+// which is chrome/test/data/background_fetch/background_fetch.js
+const int kIncorrectDownloadTotalBytes = 1000;
+
 // Implementation of a download system logger that provides the ability to wait
 // for certain events to happen, notably added and progressing downloads.
 class WaitableDownloadLoggerObserver : public download::Logger::Observer {
@@ -95,6 +102,7 @@ class OfflineContentProviderObserver : public OfflineContentProvider::Observer {
  public:
   using ItemsAddedCallback =
       base::OnceCallback<void(const std::vector<OfflineItem>&)>;
+  using ItemDownloadedCallback = base::OnceCallback<void(const OfflineItem&)>;
 
   OfflineContentProviderObserver() = default;
   ~OfflineContentProviderObserver() final = default;
@@ -103,6 +111,10 @@ class OfflineContentProviderObserver : public OfflineContentProvider::Observer {
     items_added_callback_ = std::move(callback);
   }
 
+  void set_item_downloaded_callback(ItemDownloadedCallback callback) {
+    item_downloaded_callback_ = std::move(callback);
+  }
+
   // OfflineContentProvider::Observer implementation:
   void OnItemsAdded(
       const OfflineContentProvider::OfflineItemList& items) override {
@@ -111,10 +123,15 @@ class OfflineContentProviderObserver : public OfflineContentProvider::Observer {
   }
 
   void OnItemRemoved(const ContentId& id) override {}
-  void OnItemUpdated(const OfflineItem& item) override {}
+  void OnItemUpdated(const OfflineItem& item) override {
+    if (item.state == offline_items_collection::OfflineItemState::COMPLETE &&
+        item_downloaded_callback_)
+      std::move(item_downloaded_callback_).Run(item);
+  }
 
  private:
   ItemsAddedCallback items_added_callback_;
+  ItemDownloadedCallback item_downloaded_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(OfflineContentProviderObserver);
 };
@@ -244,6 +261,15 @@ class BackgroundFetchBrowserTest : public InProcessBrowserTest {
     std::move(quit_closure).Run();
   }
 
+  // Called when the an offline item has been downloaded.
+  void DidDownloadItem(base::OnceClosure quit_closure,
+                       OfflineItem* out_item,
+                       const OfflineItem& downloaded_item) {
+    DCHECK(out_item);
+    *out_item = downloaded_item;
+    std::move(quit_closure).Run();
+  }
+
  protected:
   download::DownloadService* download_service_{nullptr};
 
@@ -312,8 +338,9 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
   EXPECT_FALSE(offline_item.is_suggested);
   EXPECT_FALSE(offline_item.is_off_the_record);
 
+  // When downloadTotal isn't specified, we report progress by parts.
   EXPECT_EQ(offline_item.progress.value, 0);
-  EXPECT_EQ(offline_item.progress.max, 1);
+  EXPECT_EQ(offline_item.progress.max.value(), 1);
   EXPECT_EQ(offline_item.progress.unit, OfflineItemProgressUnit::PERCENTAGE);
 
   // Change-detector tests for values we might want to provide or change.
@@ -337,21 +364,10 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
   const OfflineItem& offline_item = items[0];
 
   // Verify that the appropriate data is being set.
-  EXPECT_EQ(offline_item.title, GetExpectedTitle(kSingleFileDownloadTitle));
-  EXPECT_EQ(offline_item.filter, OfflineItemFilter::FILTER_OTHER);
-  EXPECT_TRUE(offline_item.is_transient);
-  EXPECT_FALSE(offline_item.is_suggested);
-  EXPECT_FALSE(offline_item.is_off_the_record);
-
   EXPECT_EQ(offline_item.progress.value, 0);
-  EXPECT_EQ(offline_item.progress.max, 1);
+  EXPECT_EQ(offline_item.progress.max.value(), 1);
   EXPECT_EQ(offline_item.progress.unit, OfflineItemProgressUnit::PERCENTAGE);
 
-  // Change-detector tests for values we might want to provide or change.
-  EXPECT_TRUE(offline_item.description.empty());
-  EXPECT_TRUE(offline_item.page_url.is_empty());
-  EXPECT_FALSE(offline_item.is_resumable);
-
   // Get visuals associated with the newly added offline item.
   std::unique_ptr<OfflineItemVisuals> out_visuals;
   GetVisualsForOfflineItemSync(offline_item.id, &out_visuals);
@@ -364,4 +380,57 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
 #endif
 }
 
+IN_PROC_BROWSER_TEST_F(
+    BackgroundFetchBrowserTest,
+    OfflineItemCollection_VerifyResourceDownloadedWhenDownloadTotalLargerThanActualSize) {
+  // Starts a Background Fetch for a single to-be-downloaded file and waits for
+  // the fetch to be registered with the offline items collection.
+  std::vector<OfflineItem> items;
+  ASSERT_NO_FATAL_FAILURE(RunScriptAndWaitForOfflineItems(
+      "StartSingleFileDownloadWithBiggerThanActualDownloadTotal()", &items));
+  ASSERT_EQ(items.size(), 1u);
+
+  OfflineItem offline_item = items[0];
+
+  // Verify that the appropriate data is being set when we start downloading.
+  EXPECT_EQ(offline_item.progress.value, 0);
+  EXPECT_EQ(offline_item.progress.max.value(), kIncorrectDownloadTotalBytes);
+  EXPECT_EQ(offline_item.progress.unit, OfflineItemProgressUnit::PERCENTAGE);
+
+  // Wait for the download to be completed.
+  {
+    base::RunLoop run_loop;
+    offline_content_provider_observer_->set_item_downloaded_callback(
+        base::BindOnce(&BackgroundFetchBrowserTest::DidDownloadItem,
+                       base::Unretained(this), run_loop.QuitClosure(),
+                       &offline_item));
+    run_loop.Run();
+  }
+
+  // Download total is incorrect; check that we're still reporting by size,
+  // but have set the max value of the progress bar to the actual download size.
+  EXPECT_EQ(offline_item.progress.max.value(), offline_item.progress.value);
+  EXPECT_EQ(offline_item.progress.max.value(), kDownloadedResourceSizeInBytes);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    BackgroundFetchBrowserTest,
+    OfflineItemCollection_VerifyResourceDownloadedWhenCorrectDownloadTotalSpecified) {
+  // Starts a Background Fetch for a single to-be-downloaded file and waits for
+  // the fetch to be registered with the offline items collection.
+
+  std::vector<OfflineItem> items;
+  ASSERT_NO_FATAL_FAILURE(RunScriptAndWaitForOfflineItems(
+      "StartSingleFileDownloadWithCorrectDownloadTotal()", &items));
+  ASSERT_EQ(items.size(), 1u);
+
+  const OfflineItem& offline_item = items[0];
+
+  // Verify that the appropriate data is being set when downloadTotal is
+  // correctly set.
+  EXPECT_EQ(offline_item.progress.value, 0);
+  EXPECT_EQ(offline_item.progress.max.value(), kDownloadedResourceSizeInBytes);
+  EXPECT_EQ(offline_item.progress.unit, OfflineItemProgressUnit::PERCENTAGE);
+}
+
 }  // namespace

chrome/browser/background_fetch/background_fetch_delegate_impl.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/guid.h"
+#include "base/logging.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
@@ -18,6 +19,7 @@
 #include "components/download/public/background_service/download_service.h"
 #include "components/offline_items_collection/core/offline_content_aggregator.h"
 #include "components/offline_items_collection/core/offline_item.h"
+#include "content/public/browser/background_fetch_description.h"
 #include "content/public/browser/background_fetch_response.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -46,39 +48,45 @@ void BackgroundFetchDelegateImpl::Shutdown() {
 BackgroundFetchDelegateImpl::JobDetails::JobDetails(JobDetails&&) = default;
 
 BackgroundFetchDelegateImpl::JobDetails::JobDetails(
-    const std::string& job_unique_id,
-    const std::string& title,
-    const url::Origin& origin,
-    const SkBitmap& icon,
-    int completed_parts,
-    int total_parts)
-    : title(title),
-      origin(origin),
-      icon(gfx::ImageSkia::CreateFrom1xBitmap(icon)),
-      completed_parts(completed_parts),
-      total_parts(total_parts),
-      cancelled(false),
-      offline_item(offline_items_collection::ContentId("background_fetch",
-                                                       job_unique_id)) {
+    std::unique_ptr<content::BackgroundFetchDescription> fetch_description)
+    : cancelled(false),
+      offline_item(offline_items_collection::ContentId(
+          "background_fetch",
+          fetch_description->job_unique_id)),
+      fetch_description(std::move(fetch_description)) {
   UpdateOfflineItem();
 }
 
 BackgroundFetchDelegateImpl::JobDetails::~JobDetails() = default;
 
 void BackgroundFetchDelegateImpl::JobDetails::UpdateOfflineItem() {
-  if (total_parts > 0) {
-    offline_item.progress.value = completed_parts;
-    offline_item.progress.max = total_parts;
+  DCHECK_GT(fetch_description->total_parts, 0);
+
+  if (ShouldReportProgressBySize()) {
+    offline_item.progress.value = fetch_description->completed_parts_size;
+    // If we have completed all downloads, update progress max to
+    // completed_parts_size in case total_parts_size was set too high. This
+    // avoid unnecessary jumping in the progress bar.
+    offline_item.progress.max =
+        (fetch_description->completed_parts == fetch_description->total_parts)
+            ? fetch_description->completed_parts_size
+            : fetch_description->total_parts_size;
+  } else {
+    offline_item.progress.value = fetch_description->completed_parts;
+    offline_item.progress.max = fetch_description->total_parts;
+  }
+
   offline_item.progress.unit =
       offline_items_collection::OfflineItemProgressUnit::PERCENTAGE;
-  }
-  if (title.empty()) {
-    offline_item.title = origin.Serialize();
+
+  if (fetch_description->title.empty()) {
+    offline_item.title = fetch_description->origin.Serialize();
   } else {
     // TODO(crbug.com/774612): Make sure that the origin is displayed completely
     // in all cases so that long titles cannot obscure it.
-    offline_item.title = base::StringPrintf("%s (%s)", title.c_str(),
-                                            origin.Serialize().c_str());
+    offline_item.title =
+        base::StringPrintf("%s (%s)", fetch_description->title.c_str(),
+                           fetch_description->origin.Serialize().c_str());
   }
   // TODO(delphick): Figure out what to put in offline_item.description.
   offline_item.is_transient = true;
@@ -86,12 +94,28 @@ void BackgroundFetchDelegateImpl::JobDetails::UpdateOfflineItem() {
   using OfflineItemState = offline_items_collection::OfflineItemState;
   if (cancelled)
     offline_item.state = OfflineItemState::CANCELLED;
-  else if (completed_parts == total_parts)
+  else if (fetch_description->completed_parts == fetch_description->total_parts)
     offline_item.state = OfflineItemState::COMPLETE;
   else
     offline_item.state = OfflineItemState::IN_PROGRESS;
 }
 
+bool BackgroundFetchDelegateImpl::JobDetails::ShouldReportProgressBySize() {
+  if (!fetch_description->total_parts_size) {
+    // total_parts_size was not set. Cannot report by size.
+    return false;
+  }
+
+  if (fetch_description->completed_parts < fetch_description->total_parts &&
+      fetch_description->completed_parts_size >
+          fetch_description->total_parts_size) {
+    // total_parts_size was set too low.
+    return false;
+  }
+
+  return true;
+}
+
 void BackgroundFetchDelegateImpl::GetIconDisplaySize(
     BackgroundFetchDelegate::GetIconDisplaySizeCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -109,24 +133,16 @@ void BackgroundFetchDelegateImpl::GetIconDisplaySize(
 }
 
 void BackgroundFetchDelegateImpl::CreateDownloadJob(
-    const std::string& job_unique_id,
-    const std::string& title,
-    const url::Origin& origin,
-    const SkBitmap& icon,
-    int completed_parts,
-    int total_parts,
-    const std::vector<std::string>& current_guids) {
+    std::unique_ptr<content::BackgroundFetchDescription> fetch_description) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
+  std::string job_unique_id = fetch_description->job_unique_id;
   DCHECK(!job_details_map_.count(job_unique_id));
-
   auto emplace_result = job_details_map_.emplace(
-      job_unique_id, JobDetails(job_unique_id, title, origin, icon,
-                                completed_parts, total_parts));
+      job_unique_id, JobDetails(std::move(fetch_description)));
 
   const JobDetails& details = emplace_result.first->second;
-
-  for (const auto& download_guid : current_guids) {
+  for (const auto& download_guid : details.fetch_description->current_guids) {
     DCHECK(!download_job_unique_id_map_.count(download_guid));
     download_job_unique_id_map_.emplace(download_guid, job_unique_id);
   }
@@ -220,6 +236,12 @@ void BackgroundFetchDelegateImpl::OnDownloadUpdated(
 
   const std::string& job_unique_id = download_job_unique_id_iter->second;
 
+  // This will update the progress bar.
+  DCHECK(job_details_map_.count(job_unique_id));
+  JobDetails& job_details = job_details_map_.find(job_unique_id)->second;
+  job_details.fetch_description->completed_parts_size = bytes_downloaded;
+  UpdateOfflineItemAndUpdateObservers(&job_details);
+
   if (client())
     client()->OnDownloadUpdated(job_unique_id, download_guid, bytes_downloaded);
 }
@@ -242,7 +264,7 @@ void BackgroundFetchDelegateImpl::OnDownloadFailed(
 
   const std::string& job_unique_id = download_job_unique_id_iter->second;
   JobDetails& job_details = job_details_map_.find(job_unique_id)->second;
-  ++job_details.completed_parts;
+  ++job_details.fetch_description->completed_parts;
   UpdateOfflineItemAndUpdateObservers(&job_details);
 
   switch (reason) {
@@ -295,7 +317,8 @@ void BackgroundFetchDelegateImpl::OnDownloadSucceeded(
 
   const std::string& job_unique_id = download_job_unique_id_iter->second;
   JobDetails& job_details = job_details_map_.find(job_unique_id)->second;
-  ++job_details.completed_parts;
+  ++job_details.fetch_description->completed_parts;
+  job_details.fetch_description->completed_parts_size = size;
   UpdateOfflineItemAndUpdateObservers(&job_details);
 
   if (client()) {
@@ -446,7 +469,8 @@ void BackgroundFetchDelegateImpl::GetVisualsForItem(
       std::make_unique<offline_items_collection::OfflineItemVisuals>();
   auto it = job_details_map_.find(id.id);
   if (it != job_details_map_.end()) {
-    visuals->icon = it->second.icon;
+    visuals->icon =
+        gfx::Image::CreateFrom1xBitmap(it->second.fetch_description->icon);
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(

chrome/browser/background_fetch/background_fetch_delegate_impl.h

@@ -22,7 +22,6 @@
 #include "url/origin.h"
 
 class Profile;
-class SkBitmap;
 
 namespace download {
 class DownloadService;
@@ -49,14 +48,8 @@ class BackgroundFetchDelegateImpl
 
   // BackgroundFetchDelegate implementation:
   void GetIconDisplaySize(GetIconDisplaySizeCallback callback) override;
-  void CreateDownloadJob(
-      const std::string& job_unique_id,
-      const std::string& title,
-      const url::Origin& origin,
-      const SkBitmap& icon,
-      int completed_parts,
-      int total_parts,
-      const std::vector<std::string>& current_guids) override;
+  void CreateDownloadJob(std::unique_ptr<content::BackgroundFetchDescription>
+                             fetch_description) override;
   void DownloadUrl(const std::string& job_unique_id,
                    const std::string& guid,
                    const std::string& method,
@@ -99,21 +92,12 @@ class BackgroundFetchDelegateImpl
  private:
   struct JobDetails {
     JobDetails(JobDetails&&);
-    JobDetails(const std::string& job_unique_id,
-               const std::string& title,
-               const url::Origin& origin,
-               const SkBitmap& icon,
-               int completed_parts,
-               int total_parts);
+    explicit JobDetails(
+        std::unique_ptr<content::BackgroundFetchDescription> fetch_description);
     ~JobDetails();
 
     void UpdateOfflineItem();
 
-    std::string title;
-    const url::Origin origin;
-    gfx::Image icon;
-    int completed_parts;
-    const int total_parts;
     bool cancelled;
 
     // Set of DownloadService GUIDs that are currently downloading. They are
@@ -122,8 +106,13 @@ class BackgroundFetchDelegateImpl
     base::flat_set<std::string> current_download_guids;
 
     offline_items_collection::OfflineItem offline_item;
+    std::unique_ptr<content::BackgroundFetchDescription> fetch_description;
 
    private:
+    // Whether we should report progress of the job in terms of size of
+    // downloads or in terms of the number of files being downloaded.
+    bool ShouldReportProgressBySize();
+
     DISALLOW_COPY_AND_ASSIGN(JobDetails);
   };
 

chrome/test/data/background_fetch/background_fetch.js

@@ -4,6 +4,14 @@
 
 // Background Fetch Id to use when its value is not significant.
 const kBackgroundFetchId = 'bg-fetch-id';
+const kBackgroundFetchResource = [ '/background_fetch/types_of_cheese.txt' ];
+const kIcon = [
+  {
+    src: '/notifications/icon.png',
+    sizes: '100x100',
+    type: 'image/png'
+  }
+];
 
 function RegisterServiceWorker() {
   navigator.serviceWorker.register('sw.js').then(() => {
@@ -15,18 +23,46 @@ function RegisterServiceWorker() {
 function StartSingleFileDownload() {
   navigator.serviceWorker.ready.then(swRegistration => {
     const options = {
-      icons: [
-        {
-          src: '/notifications/icon.png',
-          sizes: '100x100',
-          type: 'image/png'
-        }
-      ],
+      icons: kIcon,
       title: 'Single-file Background Fetch'
     };
 
     return swRegistration.backgroundFetch.fetch(
-        kBackgroundFetchId, [ '/notifications/icon.png' ], options);
+        kBackgroundFetchId, kBackgroundFetchResource, options);
+  }).then(bgFetchRegistration => {
+    sendResultToTest('ok');
+  }).catch(sendErrorToTest);
+}
+
+// Starts a Background Fetch request for a single to-be-downloaded file, with
+// downloadTotal greater than the actual size.
+function StartSingleFileDownloadWithBiggerThanActualDownloadTotal() {
+  navigator.serviceWorker.ready.then(swRegistration => {
+    const options = {
+      icons: kIcon,
+      title: 'Single-file Background Fetch with incorrect downloadTotal',
+      downloadTotal: 1000
+    };
+
+    return swRegistration.backgroundFetch.fetch(
+        kBackgroundFetchId, kBackgroundFetchResource, options);
+  }).then(bgFetchRegistration => {
+    sendResultToTest('ok');
+  }).catch(sendErrorToTest);
+}
+
+// Starts a Background Fetch request for a single to-be-downloaded file, with
+// downloadTotal equal to the actual size (in bytes).
+function StartSingleFileDownloadWithCorrectDownloadTotal() {
+  navigator.serviceWorker.ready.then(swRegistration => {
+    const options = {
+      icons: kIcon,
+      title: 'Single-file Background Fetch with accurate downloadTotal',
+      downloadTotal: 82
+    };
+
+    return swRegistration.backgroundFetch.fetch(
+        kBackgroundFetchId, kBackgroundFetchResource, options);
   }).then(bgFetchRegistration => {
     sendResultToTest('ok');
   }).catch(sendErrorToTest);

chrome/test/data/background_fetch/types_of_cheese.txt

+mozzarella
+cheddar
+brie
+goat's cheese
+cream cheese
+blue cheese
+non-dairy "cheese"

content/browser/background_fetch/background_fetch_delegate_proxy.cc

@@ -9,6 +9,7 @@
 #include "components/download/public/common/download_item.h"
 #include "components/download/public/common/download_url_parameters.h"
 #include "content/browser/background_fetch/background_fetch_job_controller.h"
+#include "content/public/browser/background_fetch_description.h"
 #include "content/public/browser/background_fetch_response.h"
 #include "content/public/browser/download_manager.h"
 #include "ui/gfx/geometry/size.h"
@@ -63,19 +64,12 @@ class BackgroundFetchDelegateProxy::Core
     }
   }
 
-  void CreateDownloadJob(const std::string& job_unique_id,
-                         const std::string& title,
-                         const url::Origin& origin,
-                         const SkBitmap& icon,
-                         int completed_parts,
-                         int total_parts,
-                         const std::vector<std::string>& current_guids) {
+  void CreateDownloadJob(
+      std::unique_ptr<BackgroundFetchDescription> fetch_description) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-    if (delegate_) {
-      delegate_->CreateDownloadJob(job_unique_id, title, origin, icon,
-                                   completed_parts, total_parts, current_guids);
-    }
+    if (delegate_)
+      delegate_->CreateDownloadJob(std::move(fetch_description));
   }
 
   void StartRequest(const std::string& job_unique_id,
@@ -261,24 +255,17 @@ void BackgroundFetchDelegateProxy::GetIconDisplaySize(
 }
 
 void BackgroundFetchDelegateProxy::CreateDownloadJob(
-    const std::string& job_unique_id,
-    const std::string& title,
-    const url::Origin& origin,
-    const SkBitmap& icon,
     base::WeakPtr<Controller> controller,
-    int completed_parts,
-    int total_parts,
-    const std::vector<std::string>& current_guids) {
+    std::unique_ptr<BackgroundFetchDescription> fetch_description) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  DCHECK(!job_details_map_.count(job_unique_id));
-  job_details_map_.emplace(job_unique_id, JobDetails(controller));
+  DCHECK(!job_details_map_.count(fetch_description->job_unique_id));
+  job_details_map_.emplace(fetch_description->job_unique_id,
+                           JobDetails(controller));
 
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::BindOnce(&Core::CreateDownloadJob, ui_core_ptr_, job_unique_id,
-                     title, origin, icon, completed_parts, total_parts,
-                     current_guids));
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::BindOnce(&Core::CreateDownloadJob, ui_core_ptr_,
+                                         std::move(fetch_description)));
 }
 
 void BackgroundFetchDelegateProxy::StartRequest(

content/browser/background_fetch/background_fetch_delegate_proxy.h

@@ -16,11 +16,10 @@
 #include "base/memory/weak_ptr.h"
 #include "content/browser/background_fetch/background_fetch_request_info.h"
 #include "content/public/browser/background_fetch_delegate.h"
+#include "content/public/browser/background_fetch_description.h"
 #include "content/public/browser/background_fetch_response.h"
 #include "content/public/browser/browser_thread.h"
 
-class SkBitmap;
-
 namespace content {
 
 // Proxy class for passing messages between BackgroundFetchJobControllers on the
@@ -59,22 +58,19 @@ class CONTENT_EXPORT BackgroundFetchDelegateProxy {
   void GetIconDisplaySize(
       BackgroundFetchDelegate::GetIconDisplaySizeCallback callback);
 
-  // Creates a new download grouping identified by |job_unique_id|. Further
-  // downloads started by StartRequest will also use this |job_unique_id| so
-  // that a notification can be updated with the current status. If the download
-  // was already started in a previous browser session, then |current_guids|
-  // should contain the GUIDs of in progress downloads, while completed
-  // downloads are recorded in |completed_parts|.
-  // Should only be called from the Controller (on the IO
-  // thread).
-  void CreateDownloadJob(const std::string& job_unique_id,
-                         const std::string& title,
-                         const url::Origin& origin,
-                         const SkBitmap& icon,
+  // Creates a new download grouping described by |fetch_description|. Further
+  // downloads started by StartRequest will also use
+  // |fetch_description.job_unique_id| so that a notification can be updated
+  // with the current status. If the download was already started in a previous
+  // browser session, then |fetch_description.current_guids| should contain the
+  // GUIDs of in progress downloads, while completed downloads are recorded in
+  // |fetch_description.completed_parts|. The size of the completed parts is
+  // recorded in |fetch_description.completed_parts_size| and total download
+  // size is stored in |fetch_description.total_parts_size|. Should only be
+  // called from the Controller (on the IO thread).
+  void CreateDownloadJob(
       base::WeakPtr<Controller> controller,
-                         int completed_parts,
-                         int total_parts,
-                         const std::vector<std::string>& current_guids);
+      std::unique_ptr<BackgroundFetchDescription> fetch_description);
 
   // Requests that the download manager start fetching |request|.
   // Should only be called from the Controller (on the IO

content/browser/background_fetch/background_fetch_delegate_proxy_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/run_loop.h"
 #include "content/browser/background_fetch/background_fetch_test_base.h"
 #include "content/public/browser/background_fetch_delegate.h"
+#include "content/public/browser/background_fetch_description.h"
 #include "content/public/browser/background_fetch_response.h"
 #include "content/public/browser/browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -33,13 +34,7 @@ class FakeBackgroundFetchDelegate : public BackgroundFetchDelegate {
     std::move(callback).Run(gfx::Size(kIconDisplaySize, kIconDisplaySize));
   }
   void CreateDownloadJob(
-      const std::string& job_unique_id,
-      const std::string& title,
-      const url::Origin& origin,
-      const SkBitmap& icon,
-      int completed_parts,
-      int total_parts,
-      const std::vector<std::string>& current_guids) override {}
+      std::unique_ptr<BackgroundFetchDescription> fetch_description) override {}
   void DownloadUrl(const std::string& job_unique_id,
                    const std::string& guid,
                    const std::string& method,
@@ -154,9 +149,13 @@ TEST_F(BackgroundFetchDelegateProxyTest, StartRequest) {
   EXPECT_FALSE(controller.request_started_);
   EXPECT_FALSE(controller.request_completed_);
 
-  delegate_proxy_.CreateDownloadJob(
+  auto fetch_description = std::make_unique<BackgroundFetchDescription>(
       kExampleUniqueId, "Job 1", url::Origin(), SkBitmap(),
-      controller.weak_ptr_factory_.GetWeakPtr(), 0, 1, {});
+      0 /* completed_parts */, 1 /* total_parts */,
+      0 /* completed_parts_size */, 0 /* total_parts_size */,
+      std::vector<std::string>());
+  delegate_proxy_.CreateDownloadJob(controller.weak_ptr_factory_.GetWeakPtr(),
+                                    std::move(fetch_description));
 
   delegate_proxy_.StartRequest(kExampleUniqueId, url::Origin(), request);
   base::RunLoop().RunUntilIdle();
@@ -174,9 +173,13 @@ TEST_F(BackgroundFetchDelegateProxyTest, StartRequest_NotCompleted) {
   EXPECT_FALSE(controller.request_completed_);
 
   delegate_.set_complete_downloads(false);
-  delegate_proxy_.CreateDownloadJob(
+  auto fetch_description = std::make_unique<BackgroundFetchDescription>(
       kExampleUniqueId, "Job 1", url::Origin(), SkBitmap(),
-      controller.weak_ptr_factory_.GetWeakPtr(), 0, 1, {});
+      0 /* completed_parts */, 1 /* total_parts */,
+      0 /* completed_parts_size */, 0 /* total_parts_size */,
+      std::vector<std::string>());
+  delegate_proxy_.CreateDownloadJob(controller.weak_ptr_factory_.GetWeakPtr(),
+                                    std::move(fetch_description));
 
   delegate_proxy_.StartRequest(kExampleUniqueId, url::Origin(), request);
   base::RunLoop().RunUntilIdle();
@@ -198,13 +201,21 @@ TEST_F(BackgroundFetchDelegateProxyTest, Abort) {
   EXPECT_FALSE(controller2.request_started_);
   EXPECT_FALSE(controller2.request_completed_);
 
-  delegate_proxy_.CreateDownloadJob(
+  auto fetch_description1 = std::make_unique<BackgroundFetchDescription>(
       kExampleUniqueId, "Job 1", url::Origin(), SkBitmap(),
-      controller.weak_ptr_factory_.GetWeakPtr(), 0, 1, {});
+      0 /* completed_parts */, 1 /* total_parts */,
+      0 /* completed_parts_size */, 0 /* total_parts_size */,
+      std::vector<std::string>());
+  delegate_proxy_.CreateDownloadJob(controller.weak_ptr_factory_.GetWeakPtr(),
+                                    std::move(fetch_description1));
 
-  delegate_proxy_.CreateDownloadJob(
+  auto fetch_description2 = std::make_unique<BackgroundFetchDescription>(
       kExampleUniqueId2, "Job 2", url::Origin(), SkBitmap(),
-      controller2.weak_ptr_factory_.GetWeakPtr(), 0, 1, {});
+      0 /* completed_parts */, 1 /* total_parts */,
+      0 /* completed_parts_size */, 0 /* total_parts_size */,
+      std::vector<std::string>());
+  delegate_proxy_.CreateDownloadJob(controller2.weak_ptr_factory_.GetWeakPtr(),
+                                    std::move(fetch_description2));
 
   delegate_proxy_.StartRequest(kExampleUniqueId, url::Origin(), request);
   delegate_proxy_.StartRequest(kExampleUniqueId2, url::Origin(), request2);

content/browser/background_fetch/background_fetch_job_controller.cc

@@ -45,10 +45,17 @@ void BackgroundFetchJobController::InitializeRequestStatus(
   completed_downloads_ = completed_downloads;
   total_downloads_ = total_downloads;
 
-  delegate_proxy_->CreateDownloadJob(registration_id().unique_id(),
-                                     options_.title, registration_id().origin(),
-                                     icon_, GetWeakPtr(), completed_downloads,
-                                     total_downloads, outstanding_guids);
+  // TODO(nator): Update this when we support uploads.
+  int total_downloads_size = options_.download_total;
+
+  auto fetch_description = std::make_unique<BackgroundFetchDescription>(
+      registration_id().unique_id(), options_.title, registration_id().origin(),
+      icon_, completed_downloads, total_downloads,
+      complete_requests_downloaded_bytes_cache_, total_downloads_size,
+      outstanding_guids);
+
+  delegate_proxy_->CreateDownloadJob(GetWeakPtr(),
+                                     std::move(fetch_description));
 }
 
 BackgroundFetchJobController::~BackgroundFetchJobController() {

content/browser/background_fetch/mock_background_fetch_delegate.cc

@@ -8,6 +8,7 @@
 #include "base/files/file_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/background_fetch/mock_background_fetch_delegate.h"
+#include "content/public/browser/background_fetch_description.h"
 #include "content/public/browser/background_fetch_response.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/http/http_response_headers.h"
@@ -59,13 +60,7 @@ void MockBackgroundFetchDelegate::GetIconDisplaySize(
     GetIconDisplaySizeCallback callback) {}
 
 void MockBackgroundFetchDelegate::CreateDownloadJob(
-    const std::string& job_unique_id,
-    const std::string& title,
-    const url::Origin& origin,
-    const SkBitmap& icon,
-    int completed_parts,
-    int total_parts,
-    const std::vector<std::string>& current_guids) {}
+    std::unique_ptr<BackgroundFetchDescription> fetch_description) {}
 
 void MockBackgroundFetchDelegate::DownloadUrl(
     const std::string& job_unique_id,

content/browser/background_fetch/mock_background_fetch_delegate.h

@@ -66,13 +66,7 @@ class MockBackgroundFetchDelegate : public BackgroundFetchDelegate {
   void GetIconDisplaySize(
       BackgroundFetchDelegate::GetIconDisplaySizeCallback callback) override;
   void CreateDownloadJob(
-      const std::string& job_unique_id,
-      const std::string& title,
-      const url::Origin& origin,
-      const SkBitmap& icon,
-      int completed_parts,
-      int total_parts,
-      const std::vector<std::string>& current_guids) override;
+      std::unique_ptr<BackgroundFetchDescription> fetch_description) override;
   void DownloadUrl(const std::string& job_unique_id,
                    const std::string& guid,
                    const std::string& method,

content/public/browser/BUILD.gn

@@ -41,6 +41,8 @@ jumbo_source_set("browser_sources") {
     "ax_event_notification_details.h",
     "background_fetch_delegate.cc",
     "background_fetch_delegate.h",
+    "background_fetch_description.cc",
+    "background_fetch_description.h",
     "background_fetch_response.cc",
     "background_fetch_response.h",
     "background_sync_controller.h",

content/public/browser/background_fetch_delegate.h

@@ -16,7 +16,6 @@
 #include "content/common/content_export.h"
 
 class GURL;
-class SkBitmap;
 
 namespace gfx {
 class Size;
@@ -27,13 +26,10 @@ class HttpRequestHeaders;
 struct NetworkTrafficAnnotationTag;
 }  // namespace net
 
-namespace url {
-class Origin;
-}  // namespace url
-
 namespace content {
 struct BackgroundFetchResponse;
 struct BackgroundFetchResult;
+struct BackgroundFetchDescription;
 
 // Interface for launching background fetches. Implementing classes would
 // generally interface with the DownloadService or DownloadManager.
@@ -92,13 +88,7 @@ class CONTENT_EXPORT BackgroundFetchDelegate {
   // contain the GUIDs of in progress downloads, while completed downloads are
   // recorded in |completed_parts|.
   virtual void CreateDownloadJob(
-      const std::string& job_unique_id,
-      const std::string& title,
-      const url::Origin& origin,
-      const SkBitmap& icon,
-      int completed_parts,
-      int total_parts,
-      const std::vector<std::string>& current_guids) = 0;
+      std::unique_ptr<BackgroundFetchDescription> fetch_description) = 0;
 
   // Creates a new download identified by |download_guid| in the download job
   // identified by |job_unique_id|.

content/public/browser/background_fetch_description.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/public/browser/background_fetch_description.h"
+
+namespace content {
+
+BackgroundFetchDescription::BackgroundFetchDescription(
+    std::string job_unique_id,
+    std::string title,
+    url::Origin origin,
+    SkBitmap icon,
+    int completed_parts,
+    int total_parts,
+    int completed_parts_size,
+    int total_parts_size,
+    std::vector<std::string> current_guids)
+    : job_unique_id(job_unique_id),
+      title(title),
+      origin(origin),
+      icon(icon),
+      completed_parts(completed_parts),
+      total_parts(total_parts),
+      completed_parts_size(completed_parts_size),
+      total_parts_size(total_parts_size),
+      current_guids(std::move(current_guids)) {}
+
+BackgroundFetchDescription::~BackgroundFetchDescription() = default;
+
+}  // namespace content

content/public/browser/background_fetch_description.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+#ifndef CONTENT_PUBLIC_BROWSER_BACKGROUND_FETCH_DESCRIPTION_H_
+#define CONTENT_PUBLIC_BROWSER_BACKGROUND_FETCH_DESCRIPTION_H_
+
+#include <vector>
+#include "content/common/content_export.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "url/origin.h"
+
+namespace content {
+
+// Contains all information necessary to create
+// a BackgroundFetch download (and in the future, upload) job.
+struct CONTENT_EXPORT BackgroundFetchDescription {
+  BackgroundFetchDescription(std::string job_unique_id,
+                             std::string title,
+                             url::Origin origin,
+                             SkBitmap icon,
+                             int completed_parts,
+                             int total_parts,
+                             int completed_parts_size,
+                             int total_parts_size,
+                             std::vector<std::string> current_guids);
+  ~BackgroundFetchDescription();
+
+  const std::string job_unique_id;
+  std::string title;
+  const url::Origin origin;
+  SkBitmap icon;
+  int completed_parts;
+  int total_parts;
+  int completed_parts_size;
+  int total_parts_size;
+  std::vector<std::string> current_guids;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BackgroundFetchDescription);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_BACKGROUND_FETCH_DESCRIPTION_H