[Background Fetch] Lazily initialize download service used by the delegate.

If the storage partitions are not fully initialized, the delegate factory
will enter an infinite loop when trying to create an instance of
BackgroundFetchDelegateImpl.

This changes moves the service initialization logic from the constructor
to a lazy accessor function, which is guaranteed to be called after the IO
thread is up and running. This means all Storage Partitions will have
been loaded.

This behavior can be triggered when creating a
BackgroundFetchDelegateImpl in incognito mode.

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

chrome/browser/background_fetch/background_fetch_delegate_impl.cc

@@ -27,15 +27,24 @@
 #include "ui/gfx/image/image_skia.h"
 
 BackgroundFetchDelegateImpl::BackgroundFetchDelegateImpl(Profile* profile)
-    : download_service_(
-          DownloadServiceFactory::GetInstance()->GetForBrowserContext(profile)),
+    : profile_(profile),
       offline_content_aggregator_(
           OfflineContentAggregatorFactory::GetForBrowserContext(profile)),
       weak_ptr_factory_(this) {
+  DCHECK(profile_);
   offline_content_aggregator_->RegisterProvider("background_fetch", this);
 }
 
-BackgroundFetchDelegateImpl::~BackgroundFetchDelegateImpl() {}
+BackgroundFetchDelegateImpl::~BackgroundFetchDelegateImpl() = default;
+
+download::DownloadService* BackgroundFetchDelegateImpl::GetDownloadService() {
+  if (download_service_)
+    return download_service_;
+
+  download_service_ =
+      DownloadServiceFactory::GetInstance()->GetForBrowserContext(profile_);
+  return download_service_;
+}
 
 void BackgroundFetchDelegateImpl::Shutdown() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -150,9 +159,9 @@ 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);
-    if (download_service_->GetStatus() ==
+    if (GetDownloadService()->GetStatus() ==
         download::DownloadService::ServiceStatus::READY) {
-      download_service_->ResumeDownload(download_guid);
+      GetDownloadService()->ResumeDownload(download_guid);
     }
   }
 
@@ -189,7 +198,7 @@ void BackgroundFetchDelegateImpl::DownloadUrl(
   params.traffic_annotation =
       net::MutableNetworkTrafficAnnotationTag(traffic_annotation);
 
-  download_service_->StartDownload(params);
+  GetDownloadService()->StartDownload(params);
 }
 
 void BackgroundFetchDelegateImpl::Abort(const std::string& job_unique_id) {
@@ -203,7 +212,7 @@ void BackgroundFetchDelegateImpl::Abort(const std::string& job_unique_id) {
   job_details.cancelled = true;
 
   for (const auto& download_guid : job_details.current_download_guids) {
-    download_service_->CancelDownload(download_guid);
+    GetDownloadService()->CancelDownload(download_guid);
     download_job_unique_id_map_.erase(download_guid);
   }
   UpdateOfflineItemAndUpdateObservers(&job_details);
@@ -481,18 +490,18 @@ void BackgroundFetchDelegateImpl::ResumeDownload(
 
   JobDetails& job_details = job_details_iter->second;
   for (auto& download_guid : job_details.current_download_guids)
-    download_service_->ResumeDownload(download_guid);
+    GetDownloadService()->ResumeDownload(download_guid);
 
   // TODO(delphick): Start new downloads that weren't started because of pause.
 }
 
 void BackgroundFetchDelegateImpl::ResumeActiveJobs() {
-  DCHECK_EQ(download_service_->GetStatus(),
+  DCHECK_EQ(GetDownloadService()->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);
+      GetDownloadService()->ResumeDownload(download_guid);
   }
 }
 

chrome/browser/background_fetch/background_fetch_delegate_impl.h

@@ -43,6 +43,9 @@ class BackgroundFetchDelegateImpl
 
   ~BackgroundFetchDelegateImpl() override;
 
+  // Lazily initializes and returns the DownloadService.
+  download::DownloadService* GetDownloadService();
+
   // KeyedService implementation:
   void Shutdown() override;
 
@@ -139,9 +142,12 @@ class BackgroundFetchDelegateImpl
   void OnDownloadReceived(const std::string& guid,
                           download::DownloadParams::StartResult result);
 
+  // The profile this service is being created for.
+  Profile* profile_;
+
   // The BackgroundFetchDelegateImplFactory depends on the
   // DownloadServiceFactory, so |download_service_| should outlive |this|.
-  download::DownloadService* download_service_;
+  download::DownloadService* download_service_ = nullptr;
 
   // Map from individual download GUIDs to job unique ids.
   std::map<std::string, std::string> download_job_unique_id_map_;