[Background Fetch] Check DownloadService is ready then resume fetches

On browser startup we attempt to resume all previously active fetches.
The DownloadService might not be ready though, so this change only resumes
the fetches if the DownloadService is ready, and also goes through all the
jobs and resumes the active fetches once the OnServiceInitialized event
is triggered.

Note that ResumeDownload might be called twice on the same guid in some cases,
but that will not cause any problems since the second call will be ignored.

Change-Id: I1d06c944496cd0f1b711ae91d15c003abc70f8c3
Reviewed-on: https://chromium-review.googlesource.com/1152820
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578664}

chrome/browser/background_fetch/background_fetch_delegate_impl.cc

@@ -152,7 +152,10 @@ void BackgroundFetchDelegateImpl::CreateDownloadJob(
   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);
-    download_service_->ResumeDownload(download_guid);
+    if (download_service_->GetStatus() ==
+        download::DownloadService::ServiceStatus::READY) {
+      download_service_->ResumeDownload(download_guid);
+    }
   }
 
   for (auto* observer : observers_) {
@@ -482,6 +485,16 @@ void BackgroundFetchDelegateImpl::ResumeDownload(
   // TODO(delphick): Start new downloads that weren't started because of pause.
 }
 
+void BackgroundFetchDelegateImpl::ResumeActiveJobs() {
+  DCHECK_EQ(download_service_->GetStatus(),
+            download::DownloadService::ServiceStatus::READY);
+
+  for (const auto& job_details : job_details_map_) {
+    for (const auto& download_guid : job_details.second.current_download_guids)
+      download_service_->ResumeDownload(download_guid);
+  }
+}
+
 void BackgroundFetchDelegateImpl::GetItemById(
     const offline_items_collection::ContentId& id,
     SingleItemCallback callback) {

chrome/browser/background_fetch/background_fetch_delegate_impl.h

@@ -92,6 +92,10 @@ class BackgroundFetchDelegateImpl
   void AddObserver(Observer* observer) override;
   void RemoveObserver(Observer* observer) override;
 
+  // Called once the Download Service is initialized. Resumes all previously
+  // active Jobs.
+  void ResumeActiveJobs();
+
   base::WeakPtr<BackgroundFetchDelegateImpl> GetWeakPtr() {
     return weak_ptr_factory_.GetWeakPtr();
   }

chrome/browser/background_fetch/background_fetch_download_client.cc

@@ -39,16 +39,7 @@ void BackgroundFetchDownloadClient::OnServiceInitialized(
                   browser_context_->GetBackgroundFetchDelegate())
                   ->GetWeakPtr();
   DCHECK(delegate_);
-
-  // TODO(delphick): Reconnect the outstanding downloads with the content layer
-  // part of background_fetch. For now we just cancel all the downloads.
-  if (downloads.size() > 0) {
-    download::DownloadService* download_service =
-        DownloadServiceFactory::GetInstance()->GetForBrowserContext(
-            browser_context_);
-    for (const auto& download : downloads)
-      download_service->CancelDownload(download.guid);
-  }
+  delegate_->ResumeActiveJobs();
 }
 
 void BackgroundFetchDownloadClient::OnServiceUnavailable() {}