Add the JobComplete callback and error/interrupt information

content/browser/background_fetch/background_fetch_job_controller.cc

Index: content/browser/background_fetch/background_fetch_job_controller.cc
diff --git a/content/browser/background_fetch/background_fetch_job_controller.cc b/content/browser/background_fetch/background_fetch_job_controller.cc
index e6b96f72d26484649f7d52fc3534b570d5947c01..6f145f3af55bbe5e9b01c9fb02aa30a5d6bb0622 100644
--- a/content/browser/background_fetch/background_fetch_job_controller.cc
+++ b/content/browser/background_fetch/background_fetch_job_controller.cc
@@ -21,13 +21,18 @@ BackgroundFetchJobController::BackgroundFetchJobController(
     const std::string& job_guid,
     BrowserContext* browser_context,
     StoragePartition* storage_partition,
-    std::unique_ptr<BackgroundFetchJobData> job_data)
+    std::unique_ptr<BackgroundFetchJobData> job_data,
+    base::OnceClosure completed_closure)
     : browser_context_(browser_context),
       storage_partition_(storage_partition),
       job_data_(std::move(job_data)),
+      completed_closure_(std::move(completed_closure)),
       weak_ptr_factory_(this) {}
 
-BackgroundFetchJobController::~BackgroundFetchJobController() = default;
+BackgroundFetchJobController::~BackgroundFetchJobController() {
+  if (completed_closure_)
+    std::move(completed_closure_).Run();

[Peter Beverloo, 2017/03/16 12:37:09]

nit: it's safe to delete this for now.

What's happening now is:

  BackgroundFetchContext::ShutdownOnIO()
  --> BackgroundFetchJobController::Shutdown()
  --> BackgroundFetchJobController::~BackgroundFetchJobController()
      --> BackgroundFetchContext::DidCompleteJob()

The latter will try to remove the |job_guid| from the |job_map_| that *is being
deleted* by the call to |job_map_.clear()| in
BackgroundFetchContext::ShutdownOnIO().

I don't know enough about the internals of STL to assess what this means - are
we deleting the object twice? Do we risk hitting the DCHECK in
BackgroundFetchContext::DidCompleteJob()?

So deleting this seems like the wise thing to do.

[harkness, 2017/03/17 11:14:11]

Done.

+}
 
 void BackgroundFetchJobController::Shutdown() {
   DownloadManager* manager =
@@ -66,26 +71,30 @@ void BackgroundFetchJobController::DownloadStarted(
   item->AddObserver(this);
   download_guid_map_[item->GetGuid()] = request_guid;
   job_data_->SetRequestDownloadGuid(request_guid, item->GetGuid());
+
+  // TODO(harkness): If DownloadStarted is called with a real interrupt reason,
+  // record that and don't mark it as in-progress.
 }
 
 void BackgroundFetchJobController::OnDownloadUpdated(DownloadItem* item) {
   DCHECK(download_guid_map_.find(item->GetGuid()) != download_guid_map_.end());
 
-  bool requests_remaining = false;
+  // Update the state of the request on the JobData. If the state is a final
+  // state, this will also update the complete status of the JobData.
+  bool requests_remaining = job_data_->UpdateBackgroundFetchRequestState(
+      download_guid_map_[item->GetGuid()], item->GetState(),
+      item->GetLastReason());
+
   switch (item->GetState()) {
     case DownloadItem::DownloadState::CANCELLED:
-    // TODO(harkness): Expand the "complete" flag on the RequestInfo to
-    // include error states.
     case DownloadItem::DownloadState::COMPLETE:
-      requests_remaining = job_data_->BackgroundFetchRequestInfoComplete(
-          download_guid_map_[item->GetGuid()]);
       // TODO(harkness): Tell the notification service to update the download
       // notification.
 
       if (requests_remaining) {
         ProcessRequest(job_data_->GetNextBackgroundFetchRequestInfo());
       } else if (job_data_->IsComplete()) {
-        // TODO(harkness): Return the data to the context.
+        std::move(completed_closure_).Run();
       }
       break;
     case DownloadItem::DownloadState::INTERRUPTED: