[Background Fetch] Cleanup/fix thread safety

content/browser/background_fetch/background_fetch_context.cc

Index: content/browser/background_fetch/background_fetch_context.cc
diff --git a/content/browser/background_fetch/background_fetch_context.cc b/content/browser/background_fetch/background_fetch_context.cc
index 4facffeac426a3d758822190ff7538302bea81c6..4a26d013e6d1c78924ba418793af0dede97745d6 100644
--- a/content/browser/background_fetch/background_fetch_context.cc
+++ b/content/browser/background_fetch/background_fetch_context.cc
@@ -10,7 +10,6 @@
 #include "content/browser/background_fetch/background_fetch_job_controller.h"
 #include "content/browser/background_fetch/background_fetch_registration_id.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
-#include "content/browser/storage_partition_impl.h"
 #include "content/public/browser/blob_handle.h"
 #include "content/public/browser/browser_context.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
@@ -37,36 +36,26 @@ void RecordRegistrationDeletedError(blink::mojom::BackgroundFetchError error) {
 
 BackgroundFetchContext::BackgroundFetchContext(
     BrowserContext* browser_context,
-    StoragePartitionImpl* storage_partition,
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context)
     : browser_context_(browser_context),
       data_manager_(
           base::MakeUnique<BackgroundFetchDataManager>(browser_context)),
       event_dispatcher_(base::MakeUnique<BackgroundFetchEventDispatcher>(
-          std::move(service_worker_context))) {
+          std::move(service_worker_context))),
+      weak_factory_(this) {
+  // Although this lives only on the IO thread, it is constructed on UI thread.
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
 BackgroundFetchContext::~BackgroundFetchContext() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
 }
 
 void BackgroundFetchContext::InitializeOnIOThread(
     scoped_refptr<net::URLRequestContextGetter> request_context_getter) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  request_context_getter_ = request_context_getter;
-}
 
-void BackgroundFetchContext::Shutdown() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&BackgroundFetchContext::ShutdownOnIO, this));
-}
-
-void BackgroundFetchContext::ShutdownOnIO() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  active_fetches_.clear();
+  request_context_getter_ = request_context_getter;
 }
 
 void BackgroundFetchContext::StartFetch(
@@ -75,10 +64,12 @@ void BackgroundFetchContext::StartFetch(
     const BackgroundFetchOptions& options,
     blink::mojom::BackgroundFetchService::FetchCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   data_manager_->CreateRegistration(
       registration_id, requests, options,
-      base::BindOnce(&BackgroundFetchContext::DidCreateRegistration, this,
-                     registration_id, options, std::move(callback)));
+      base::BindOnce(&BackgroundFetchContext::DidCreateRegistration,
+                     weak_factory_.GetWeakPtr(), registration_id, options,
+                     std::move(callback)));
 }
 
 void BackgroundFetchContext::DidCreateRegistration(
@@ -87,6 +78,8 @@ void BackgroundFetchContext::DidCreateRegistration(
     blink::mojom::BackgroundFetchService::FetchCallback callback,
     blink::mojom::BackgroundFetchError error,
     std::vector<scoped_refptr<BackgroundFetchRequestInfo>> initial_requests) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   RecordRegistrationCreatedError(error);
   if (error != blink::mojom::BackgroundFetchError::NONE) {
     std::move(callback).Run(error, base::nullopt /* registration */);
@@ -112,6 +105,8 @@ std::vector<std::string>
 BackgroundFetchContext::GetActiveTagsForServiceWorkerRegistration(
     int64_t service_worker_registration_id,
     const url::Origin& origin) const {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   std::vector<std::string> tags;
   for (const auto& pair : active_fetches_) {
     const BackgroundFetchRegistrationId& registration_id =
@@ -149,6 +144,8 @@ void BackgroundFetchContext::CreateController(
     const BackgroundFetchRegistrationId& registration_id,
     const BackgroundFetchOptions& options,
     std::vector<scoped_refptr<BackgroundFetchRequestInfo>> initial_requests) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   net::NetworkTrafficAnnotationTag traffic_annotation =
       net::DefineNetworkTrafficAnnotation("background_fetch_context", R"(
         semantics {
@@ -179,7 +176,8 @@ void BackgroundFetchContext::CreateController(
       base::MakeUnique<BackgroundFetchJobController>(
           registration_id, options, data_manager_.get(), browser_context_,
           request_context_getter_,
-          base::BindOnce(&BackgroundFetchContext::DidCompleteJob, this),
+          base::BindOnce(&BackgroundFetchContext::DidCompleteJob,
+                         weak_factory_.GetWeakPtr()),
           traffic_annotation);
 
   // TODO(peter): We should actually be able to use Background Fetch in layout
@@ -196,6 +194,8 @@ void BackgroundFetchContext::CreateController(
 
 void BackgroundFetchContext::DidCompleteJob(
     BackgroundFetchJobController* controller) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   const BackgroundFetchRegistrationId& registration_id =
       controller->registration_id();
 
@@ -204,15 +204,15 @@ void BackgroundFetchContext::DidCompleteJob(
     case BackgroundFetchJobController::State::ABORTED:
       event_dispatcher_->DispatchBackgroundFetchAbortEvent(
           registration_id,
-          base::Bind(&BackgroundFetchContext::DeleteRegistration, this,
-                     registration_id,
+          base::Bind(&BackgroundFetchContext::DeleteRegistration,
+                     weak_factory_.GetWeakPtr(), registration_id,
                      std::vector<std::unique_ptr<BlobHandle>>()));
       return;
     case BackgroundFetchJobController::State::COMPLETED:
       data_manager_->GetSettledFetchesForRegistration(
           registration_id,
-          base::BindOnce(&BackgroundFetchContext::DidGetSettledFetches, this,
-                         registration_id));
+          base::BindOnce(&BackgroundFetchContext::DidGetSettledFetches,
+                         weak_factory_.GetWeakPtr(), registration_id));
       return;
     case BackgroundFetchJobController::State::INITIALIZED:
     case BackgroundFetchJobController::State::FETCHING:
@@ -229,6 +229,8 @@ void BackgroundFetchContext::DidGetSettledFetches(
     bool background_fetch_succeeded,
     std::vector<BackgroundFetchSettledFetch> settled_fetches,
     std::vector<std::unique_ptr<BlobHandle>> blob_handles) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   if (error != blink::mojom::BackgroundFetchError::NONE) {
     DeleteRegistration(registration_id, std::move(blob_handles));
     return;
@@ -240,19 +242,22 @@ void BackgroundFetchContext::DidGetSettledFetches(
   if (background_fetch_succeeded) {
     event_dispatcher_->DispatchBackgroundFetchedEvent(
         registration_id, std::move(settled_fetches),
-        base::Bind(&BackgroundFetchContext::DeleteRegistration, this,
-                   registration_id, std::move(blob_handles)));
+        base::Bind(&BackgroundFetchContext::DeleteRegistration,
+                   weak_factory_.GetWeakPtr(), registration_id,
+                   std::move(blob_handles)));
   } else {
     event_dispatcher_->DispatchBackgroundFetchFailEvent(
         registration_id, std::move(settled_fetches),
-        base::Bind(&BackgroundFetchContext::DeleteRegistration, this,
-                   registration_id, std::move(blob_handles)));
+        base::Bind(&BackgroundFetchContext::DeleteRegistration,
+                   weak_factory_.GetWeakPtr(), registration_id,
+                   std::move(blob_handles)));
   }
 }
 
 void BackgroundFetchContext::DeleteRegistration(
     const BackgroundFetchRegistrationId& registration_id,
     const std::vector<std::unique_ptr<BlobHandle>>& blob_handles) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK_GT(active_fetches_.count(registration_id), 0u);
 
   // Delete all persistent information associated with the |registration_id|.