service worker: Prepare context wrapper for creating UI thread core.

This makes the wrapper's internal methods also core thread-aware rather
than assuming IO thread, following up to the public method changes from
https://chromium-review.googlesource.com/c/chromium/src/+/1753843.

After this CL, the plan is to locally change the thread to UI and see
what breaks, and start landing CLs for the individual callsites to make
them core thread-aware.

Bug: 824858
Change-Id: I9c351fa77d83cee9ef60d6fa2afee2fc35570c17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1753847
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687272}

content/browser/service_worker/embedded_worker_test_helper.cc

@@ -109,7 +109,7 @@ EmbeddedWorkerTestHelper::EmbeddedWorkerTestHelper(
           base::MakeRefCounted<URLLoaderFactoryGetter>()) {
   scoped_refptr<base::SequencedTaskRunner> database_task_runner =
       base::ThreadTaskRunnerHandle::Get();
-  wrapper_->InitOnIO(
+  wrapper_->InitOnCoreThread(
       user_data_directory, std::move(database_task_runner), nullptr, nullptr,
       nullptr, url_loader_factory_getter_.get(),
       blink::ServiceWorkerUtils::IsImportedScriptUpdateCheckEnabled()

content/browser/service_worker/service_worker_context_watcher.cc

@@ -43,33 +43,33 @@ ServiceWorkerContextWatcher::ServiceWorkerContextWatcher(
 
 void ServiceWorkerContextWatcher::Start() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
+  ServiceWorkerContextWrapper::RunOrPostTaskOnCoreThread(
+      FROM_HERE,
       base::BindOnce(
-          &ServiceWorkerContextWatcher::GetStoredRegistrationsOnIOThread,
+          &ServiceWorkerContextWatcher::GetStoredRegistrationsOnCoreThread,
           this));
 }
 
 void ServiceWorkerContextWatcher::Stop() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   stop_called_ = true;
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&ServiceWorkerContextWatcher::StopOnIOThread, this));
+  ServiceWorkerContextWrapper::RunOrPostTaskOnCoreThread(
+      FROM_HERE,
+      base::BindOnce(&ServiceWorkerContextWatcher::StopOnCoreThread, this));
 }
 
-void ServiceWorkerContextWatcher::GetStoredRegistrationsOnIOThread() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+void ServiceWorkerContextWatcher::GetStoredRegistrationsOnCoreThread() {
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   if (is_stopped_)
     return;
   context_->GetAllRegistrations(base::BindOnce(
-      &ServiceWorkerContextWatcher::OnStoredRegistrationsOnIOThread, this));
+      &ServiceWorkerContextWatcher::OnStoredRegistrationsOnCoreThread, this));
 }
 
-void ServiceWorkerContextWatcher::OnStoredRegistrationsOnIOThread(
+void ServiceWorkerContextWatcher::OnStoredRegistrationsOnCoreThread(
     blink::ServiceWorkerStatusCode status,
     const std::vector<ServiceWorkerRegistrationInfo>& stored_registrations) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   if (is_stopped_)
     return;
   context_->AddObserver(this);
@@ -114,8 +114,8 @@ void ServiceWorkerContextWatcher::OnStoredRegistrationsOnIOThread(
           std::move(versions)));
 }
 
-void ServiceWorkerContextWatcher::StopOnIOThread() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+void ServiceWorkerContextWatcher::StopOnCoreThread() {
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   context_->RemoveObserver(this);
   is_stopped_ = true;
 }
@@ -127,7 +127,7 @@ void ServiceWorkerContextWatcher::StoreRegistrationInfo(
     const ServiceWorkerRegistrationInfo& registration_info,
     std::unordered_map<int64_t, std::unique_ptr<ServiceWorkerRegistrationInfo>>*
         info_map) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   if (registration_info.registration_id ==
       blink::mojom::kInvalidServiceWorkerRegistrationId)
     return;
@@ -140,7 +140,7 @@ void ServiceWorkerContextWatcher::StoreRegistrationInfo(
 
 void ServiceWorkerContextWatcher::StoreVersionInfo(
     const ServiceWorkerVersionInfo& version_info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   if (version_info.version_id == blink::mojom::kInvalidServiceWorkerVersionId)
     return;
   version_info_map_[version_info.version_id] =
@@ -151,7 +151,7 @@ void ServiceWorkerContextWatcher::SendRegistrationInfo(
     int64_t registration_id,
     const GURL& scope,
     ServiceWorkerRegistrationInfo::DeleteFlag delete_flag) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   std::unique_ptr<std::vector<ServiceWorkerRegistrationInfo>> registrations =
       std::make_unique<std::vector<ServiceWorkerRegistrationInfo>>();
   ServiceWorkerRegistration* registration =
@@ -171,7 +171,7 @@ void ServiceWorkerContextWatcher::SendRegistrationInfo(
 
 void ServiceWorkerContextWatcher::SendVersionInfo(
     const ServiceWorkerVersionInfo& version_info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   std::unique_ptr<std::vector<ServiceWorkerVersionInfo>> versions =
       std::make_unique<std::vector<ServiceWorkerVersionInfo>>();
   versions->push_back(version_info);
@@ -210,14 +210,14 @@ void ServiceWorkerContextWatcher::RunWorkerErrorReportedCallback(
 
 void ServiceWorkerContextWatcher::OnNewLiveRegistration(int64_t registration_id,
                                                         const GURL& scope) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   SendRegistrationInfo(registration_id, scope,
                        ServiceWorkerRegistrationInfo::IS_NOT_DELETED);
 }
 
 void ServiceWorkerContextWatcher::OnNewLiveVersion(
     const ServiceWorkerVersionInfo& version_info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   int64_t version_id = version_info.version_id;
   auto it = version_info_map_.find(version_id);
   if (it != version_info_map_.end()) {
@@ -236,7 +236,7 @@ void ServiceWorkerContextWatcher::OnNewLiveVersion(
 void ServiceWorkerContextWatcher::OnRunningStateChanged(
     int64_t version_id,
     content::EmbeddedWorkerStatus running_status) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   auto it = version_info_map_.find(version_id);
   if (it == version_info_map_.end())
     return;
@@ -253,7 +253,7 @@ void ServiceWorkerContextWatcher::OnVersionStateChanged(
     int64_t version_id,
     const GURL& scope,
     content::ServiceWorkerVersion::Status status) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   auto it = version_info_map_.find(version_id);
   if (it == version_info_map_.end())
     return;
@@ -270,7 +270,7 @@ void ServiceWorkerContextWatcher::OnVersionDevToolsRoutingIdChanged(
     int64_t version_id,
     int process_id,
     int devtools_agent_route_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   auto it = version_info_map_.find(version_id);
   if (it == version_info_map_.end())
     return;
@@ -290,7 +290,7 @@ void ServiceWorkerContextWatcher::OnMainScriptHttpResponseInfoSet(
     int64_t version_id,
     base::Time script_response_time,
     base::Time script_last_modified) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   auto it = version_info_map_.find(version_id);
   if (it == version_info_map_.end())
     return;
@@ -302,7 +302,7 @@ void ServiceWorkerContextWatcher::OnMainScriptHttpResponseInfoSet(
 
 void ServiceWorkerContextWatcher::OnErrorReported(int64_t version_id,
                                                   const ErrorInfo& info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   int64_t registration_id = blink::mojom::kInvalidServiceWorkerRegistrationId;
   auto it = version_info_map_.find(version_id);
   if (it != version_info_map_.end())
@@ -317,7 +317,7 @@ void ServiceWorkerContextWatcher::OnErrorReported(int64_t version_id,
 void ServiceWorkerContextWatcher::OnReportConsoleMessage(
     int64_t version_id,
     const ConsoleMessage& message) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   if (message.message_level != blink::mojom::ConsoleMessageLevel::kError)
     return;
   int64_t registration_id = blink::mojom::kInvalidServiceWorkerRegistrationId;
@@ -339,7 +339,7 @@ void ServiceWorkerContextWatcher::OnControlleeAdded(
     const GURL& scope,
     const std::string& uuid,
     const ServiceWorkerClientInfo& info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   auto it = version_info_map_.find(version_id);
   if (it == version_info_map_.end())
     return;
@@ -351,7 +351,7 @@ void ServiceWorkerContextWatcher::OnControlleeAdded(
 void ServiceWorkerContextWatcher::OnControlleeRemoved(int64_t version_id,
                                                       const GURL& scope,
                                                       const std::string& uuid) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   auto it = version_info_map_.find(version_id);
   if (it == version_info_map_.end())
     return;
@@ -363,14 +363,14 @@ void ServiceWorkerContextWatcher::OnControlleeRemoved(int64_t version_id,
 void ServiceWorkerContextWatcher::OnRegistrationCompleted(
     int64_t registration_id,
     const GURL& scope) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   SendRegistrationInfo(registration_id, scope,
                        ServiceWorkerRegistrationInfo::IS_NOT_DELETED);
 }
 
 void ServiceWorkerContextWatcher::OnRegistrationDeleted(int64_t registration_id,
                                                         const GURL& scope) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   SendRegistrationInfo(registration_id, scope,
                        ServiceWorkerRegistrationInfo::IS_DELETED);
 }

content/browser/service_worker/service_worker_context_watcher.h

@@ -52,11 +52,11 @@ class CONTENT_EXPORT ServiceWorkerContextWatcher
 
   ~ServiceWorkerContextWatcher() override;
 
-  void GetStoredRegistrationsOnIOThread();
-  void OnStoredRegistrationsOnIOThread(
+  void GetStoredRegistrationsOnCoreThread();
+  void OnStoredRegistrationsOnCoreThread(
       blink::ServiceWorkerStatusCode status,
       const std::vector<ServiceWorkerRegistrationInfo>& stored_registrations);
-  void StopOnIOThread();
+  void StopOnCoreThread();
 
   void StoreRegistrationInfo(
       const ServiceWorkerRegistrationInfo& registration,
@@ -84,13 +84,11 @@ class CONTENT_EXPORT ServiceWorkerContextWatcher
   void OnNewLiveRegistration(int64_t registration_id,
                              const GURL& scope) override;
   void OnNewLiveVersion(const ServiceWorkerVersionInfo& version_info) override;
-  void OnRunningStateChanged(
-      int64_t version_id,
-      content::EmbeddedWorkerStatus running_status) override;
-  void OnVersionStateChanged(
-      int64_t version_id,
-      const GURL& scope,
-      content::ServiceWorkerVersion::Status status) override;
+  void OnRunningStateChanged(int64_t version_id,
+                             EmbeddedWorkerStatus running_status) override;
+  void OnVersionStateChanged(int64_t version_id,
+                             const GURL& scope,
+                             ServiceWorkerVersion::Status status) override;
   void OnVersionDevToolsRoutingIdChanged(int64_t version_id,
                                          int process_id,
                                          int devtools_agent_route_id) override;

content/browser/service_worker/service_worker_context_wrapper.cc

@@ -59,12 +59,6 @@ void RunOrPostTask(const base::Location& location,
   base::PostTask(location, {thread_id}, std::move(task));
 }
 
-void RunOrPostTaskOnCoreThread(const base::Location& location,
-                               base::OnceClosure task) {
-  RunOrPostTask(location, ServiceWorkerContextWrapper::GetCoreThreadId(),
-                std::move(task));
-}
-
 // Value used to set the timeout when starting a long running ServiceWorker. See
 // ServiceWorkerContextWrapper::StartServiceWorkerAndDispatchLongRunningMessage.
 const int kActiveWorkerTimeoutDays = 999;
@@ -194,6 +188,13 @@ void RunOnceClosure(scoped_refptr<ServiceWorkerContextWrapper> ref_holder,
 
 }  // namespace
 
+// static
+void ServiceWorkerContextWrapper::RunOrPostTaskOnCoreThread(
+    const base::Location& location,
+    base::OnceClosure task) {
+  RunOrPostTask(location, GetCoreThreadId(), std::move(task));
+}
+
 // static
 bool ServiceWorkerContext::ScopeMatches(const GURL& scope, const GURL& url) {
   return ServiceWorkerUtils::ScopeMatches(scope, url);
@@ -259,19 +260,20 @@ void ServiceWorkerContextWrapper::Init(
         CreateNonNetworkURLLoaderFactoryBundleInfoForUpdateCheck(
             storage_partition_->browser_context());
   }
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
+
+  RunOrPostTaskOnCoreThread(
+      FROM_HERE,
       base::BindOnce(
-          &ServiceWorkerContextWrapper::InitOnIO, this, user_data_directory,
-          std::move(database_task_runner),
+          &ServiceWorkerContextWrapper::InitOnCoreThread, this,
+          user_data_directory, std::move(database_task_runner),
           base::RetainedRef(quota_manager_proxy),
           base::RetainedRef(special_storage_policy),
           base::RetainedRef(blob_context),
           base::RetainedRef(loader_factory_getter),
           std::move(non_network_loader_factory_bundle_info_for_update_check)));
 
-  // The watcher also posts a IO thread task which must run after InitOnIO(), so
-  // start it after posting that task above.
+  // The watcher also runs or posts a core thread task which must run after
+  // InitOnCoreThread(), so start it after posting that task above.
   if (watcher_)
     watcher_->Start();
 }
@@ -285,9 +287,14 @@ void ServiceWorkerContextWrapper::Shutdown() {
     watcher_->Stop();
     watcher_ = nullptr;
   }
+  // TODO(https://crbug.com/824858): Make this RunOrPostTask. Some unit tests
+  // depend on this being a PostTask even if we are already on the core thread
+  // (in many unit tests the IO thread and UI thread are the same), or else the
+  // InitializeResourceContext() call by StoragePartitionImplMap() will run
+  // after the shutdown task and set the resource context back to non-null.
   base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&ServiceWorkerContextWrapper::ShutdownOnIO, this));
+      FROM_HERE, {GetCoreThreadId()},
+      base::BindOnce(&ServiceWorkerContextWrapper::ShutdownOnCoreThread, this));
 }
 
 void ServiceWorkerContextWrapper::InitializeResourceContext(
@@ -1498,7 +1505,7 @@ ServiceWorkerContextWrapper::~ServiceWorkerContextWrapper() {
   DCHECK(!resource_context_);
 }
 
-void ServiceWorkerContextWrapper::InitOnIO(
+void ServiceWorkerContextWrapper::InitOnCoreThread(
     const base::FilePath& user_data_directory,
     scoped_refptr<base::SequencedTaskRunner> database_task_runner,
     storage::QuotaManagerProxy* quota_manager_proxy,
@@ -1507,7 +1514,7 @@ void ServiceWorkerContextWrapper::InitOnIO(
     URLLoaderFactoryGetter* loader_factory_getter,
     std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
         non_network_loader_factory_bundle_info_for_update_check) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(GetCoreThreadId());
   DCHECK(!context_core_);
 
   if (quota_manager_proxy) {
@@ -1542,9 +1549,12 @@ void ServiceWorkerContextWrapper::FindRegistrationForScopeOnCoreThread(
           std::move(callback_runner)));
 }
 
-void ServiceWorkerContextWrapper::ShutdownOnIO() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  resource_context_ = nullptr;
+void ServiceWorkerContextWrapper::ShutdownOnCoreThread() {
+  DCHECK_CURRENTLY_ON(GetCoreThreadId());
+  RunOrPostTask(
+      FROM_HERE, BrowserThread::IO,
+      base::BindOnce(&ServiceWorkerContextWrapper::InitializeResourceContext,
+                     this, nullptr));
   context_core_.reset();
 }
 

content/browser/service_worker/service_worker_context_wrapper.h

@@ -74,6 +74,9 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
 
   explicit ServiceWorkerContextWrapper(BrowserContext* browser_context);
 
+  static void RunOrPostTaskOnCoreThread(const base::Location& location,
+                                        base::OnceClosure task);
+
   // Init and Shutdown are for use on the UI thread when the profile,
   // storagepartition is being setup and torn down.
   void Init(const base::FilePath& user_data_directory,
@@ -334,15 +337,16 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
 
   ~ServiceWorkerContextWrapper() override;
 
-  void InitOnIO(const base::FilePath& user_data_directory,
-                scoped_refptr<base::SequencedTaskRunner> database_task_runner,
-                storage::QuotaManagerProxy* quota_manager_proxy,
-                storage::SpecialStoragePolicy* special_storage_policy,
-                ChromeBlobStorageContext* blob_context,
-                URLLoaderFactoryGetter* url_loader_factory_getter,
-                std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
-                    non_network_loader_factory_bundle_info_for_update_check);
-  void ShutdownOnIO();
+  void InitOnCoreThread(
+      const base::FilePath& user_data_directory,
+      scoped_refptr<base::SequencedTaskRunner> database_task_runner,
+      storage::QuotaManagerProxy* quota_manager_proxy,
+      storage::SpecialStoragePolicy* special_storage_policy,
+      ChromeBlobStorageContext* blob_context,
+      URLLoaderFactoryGetter* url_loader_factory_getter,
+      std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+          non_network_loader_factory_bundle_info_for_update_check);
+  void ShutdownOnCoreThread();
 
   // If |include_installing_version| is true, |callback| is called if there is
   // an installing version with no waiting or active version.
@@ -543,7 +547,7 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
   base::ObserverList<ServiceWorkerContextObserver>::Unchecked observer_list_;
 
   const std::unique_ptr<ServiceWorkerProcessManager> process_manager_;
-  // Cleared in ShutdownOnIO():
+  // Cleared in ShutdownOnCoreThread():
   std::unique_ptr<ServiceWorkerContextCore> context_core_;
 
   // Initialized in Init(); true if the user data directory is empty.