[Background Fetch] Cleanup/fix thread safety

content/browser/background_fetch/background_fetch_data_manager.cc

Index: content/browser/background_fetch/background_fetch_data_manager.cc
diff --git a/content/browser/background_fetch/background_fetch_data_manager.cc b/content/browser/background_fetch/background_fetch_data_manager.cc
index 2fcb85e21e19ebd5937f96b55a6da7ec01e73cbb..046641132e9fd264ac67d57dbfc4223f163c21cc 100644
--- a/content/browser/background_fetch/background_fetch_data_manager.cc
+++ b/content/browser/background_fetch/background_fetch_data_manager.cc
@@ -122,7 +122,10 @@ class BackgroundFetchDataManager::RegistrationData {
 BackgroundFetchDataManager::BackgroundFetchDataManager(
     BrowserContext* browser_context)
     : weak_ptr_factory_(this) {
+  // Constructed on the UI thread, then used on a different thread.
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+
   DCHECK(browser_context);
 
   // Store the blob storage context for the given |browser_context|.
@@ -131,13 +134,17 @@ BackgroundFetchDataManager::BackgroundFetchDataManager(
   DCHECK(blob_storage_context_);
 }
 
-BackgroundFetchDataManager::~BackgroundFetchDataManager() = default;
+BackgroundFetchDataManager::~BackgroundFetchDataManager() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
 
 void BackgroundFetchDataManager::CreateRegistration(
     const BackgroundFetchRegistrationId& registration_id,
     const std::vector<ServiceWorkerFetchRequest>& requests,
     const BackgroundFetchOptions& options,
     CreateRegistrationCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   if (registrations_.find(registration_id) != registrations_.end()) {
     std::move(callback).Run(
         blink::mojom::BackgroundFetchError::DUPLICATED_TAG,
@@ -170,6 +177,8 @@ void BackgroundFetchDataManager::MarkRequestAsStarted(
     const BackgroundFetchRegistrationId& registration_id,
     BackgroundFetchRequestInfo* request,
     const std::string& download_guid) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   auto iter = registrations_.find(registration_id);
   DCHECK(iter != registrations_.end());
 
@@ -181,6 +190,8 @@ void BackgroundFetchDataManager::MarkRequestAsCompleteAndGetNextRequest(
     const BackgroundFetchRegistrationId& registration_id,
     BackgroundFetchRequestInfo* request,
     NextRequestCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   auto iter = registrations_.find(registration_id);
   DCHECK(iter != registrations_.end());
 
@@ -197,6 +208,10 @@ void BackgroundFetchDataManager::MarkRequestAsCompleteAndGetNextRequest(
 void BackgroundFetchDataManager::GetSettledFetchesForRegistration(
     const BackgroundFetchRegistrationId& registration_id,
     SettledFetchesCallback callback) {
+  // This method calls ChromeBlobStorageContext, which must happen on IO thread.
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);

[Peter Beverloo, 2017/07/10 13:13:21]

ChromeBlobStorageContext::CreateFileBackedBlob() will DCHECK, so what about
adding a comment on line 254 to indicate this? This is the sort of line that
we'd forget about when refactoring this method (which is bound to happen).

[johnme, 2017/07/10 13:41:07]

Done.

+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   auto iter = registrations_.find(registration_id);
   DCHECK(iter != registrations_.end());
 
@@ -271,6 +286,8 @@ void BackgroundFetchDataManager::GetSettledFetchesForRegistration(
 void BackgroundFetchDataManager::DeleteRegistration(
     const BackgroundFetchRegistrationId& registration_id,
     DeleteRegistrationCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   auto iter = registrations_.find(registration_id);
   if (iter == registrations_.end()) {
     std::move(callback).Run(blink::mojom::BackgroundFetchError::INVALID_TAG);