[Background Fetch] Cleanup/fix thread safety

content/browser/background_fetch/background_fetch_data_manager.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/background_fetch/background_fetch_data_manager.h"
 
 #include <algorithm>
 #include <queue>
 
 #include "base/memory/ptr_util.h"
@@ skipping 104 matching lines @@
                 "RegistrationData::completed_requests_ assumes no parallelism");
 
   std::vector<scoped_refptr<BackgroundFetchRequestInfo>> completed_requests_;
 
   DISALLOW_COPY_AND_ASSIGN(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|.
   blob_storage_context_ =
       make_scoped_refptr(ChromeBlobStorageContext::GetFor(browser_context));
   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,
         std::vector<scoped_refptr<BackgroundFetchRequestInfo>>());
     return;
   }
 
   std::unique_ptr<RegistrationData> registration_data =
       base::MakeUnique<RegistrationData>(requests, options);
 
@@ skipping 12 matching lines @@
 
   // Inform the |callback| of the newly created registration.
   std::move(callback).Run(blink::mojom::BackgroundFetchError::NONE,
                           std::move(initial_requests));
 }
 
 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());
 
   RegistrationData* registration_data = iter->second.get();
   registration_data->MarkRequestAsStarted(request, download_guid);
 }
 
 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());
 
   RegistrationData* registration_data = iter->second.get();
   registration_data->MarkRequestAsComplete(request);
 
   scoped_refptr<BackgroundFetchRequestInfo> next_request;
   if (registration_data->HasPendingRequests())
     next_request = registration_data->GetPendingRequest();
 
   std::move(callback).Run(std::move(next_request));
 }
 
 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());
 
   RegistrationData* registration_data = iter->second.get();
   DCHECK(!registration_data->HasPendingRequests());
 
   const std::vector<scoped_refptr<BackgroundFetchRequestInfo>>& requests =
       registration_data->GetCompletedRequests();
 
   bool background_fetch_succeeded = true;
@@ skipping 54 matching lines @@
   }
 
   std::move(callback).Run(blink::mojom::BackgroundFetchError::NONE,
                           background_fetch_succeeded,
                           std::move(settled_fetches), std::move(blob_handles));
 }
 
 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);
     return;
   }
 
   registrations_.erase(iter);
 
   std::move(callback).Run(blink::mojom::BackgroundFetchError::NONE);
 }
 
 }  // namespace content