service worker: Make threading of ServiceWorkerProcessManager more explicit.

This CL makes RemoveWorkerProcess() match AllocateWorkerProcess(). Now
they both must be called on the UI thread.

Change-Id: I5e6711a1aadd10aeee1cd896c81db3b1a4cf0225
Reviewed-on: https://chromium-review.googlesource.com/562848
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485220}

content/browser/service_worker/embedded_worker_instance.cc

@@ -238,8 +238,8 @@ class EmbeddedWorkerInstance::DevToolsProxy {
   DISALLOW_COPY_AND_ASSIGN(DevToolsProxy);
 };
 
-// A handle for a worker process managed by ServiceWorkerProcessManager on the
-// UI thread.
+// A handle for a renderer process managed by ServiceWorkerProcessManager on the
+// UI thread. Lives on the IO thread.
 class EmbeddedWorkerInstance::WorkerProcessHandle {
  public:
   WorkerProcessHandle(const base::WeakPtr<ServiceWorkerContextCore>& context,
@@ -250,12 +250,19 @@ class EmbeddedWorkerInstance::WorkerProcessHandle {
         embedded_worker_id_(embedded_worker_id),
         process_id_(process_id),
         is_new_process_(is_new_process) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
     DCHECK_NE(ChildProcessHost::kInvalidUniqueID, process_id_);
   }
 
   ~WorkerProcessHandle() {
-    if (context_)
-      context_->process_manager()->ReleaseWorkerProcess(embedded_worker_id_);
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    if (!context_)
+      return;
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::BindOnce(&ServiceWorkerProcessManager::ReleaseWorkerProcess,
+                       context_->process_manager()->AsWeakPtr(),
+                       embedded_worker_id_));
   }
 
   int process_id() const { return process_id_; }
@@ -290,6 +297,7 @@ class EmbeddedWorkerInstance::StartTask {
         is_installed_(false),
         started_during_browser_startup_(false),
         weak_factory_(this) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
     TRACE_EVENT_NESTABLE_ASYNC_BEGIN1("ServiceWorker",
                                       "EmbeddedWorkerInstance::Start",
                                       instance_, "Script", script_url.spec());
@@ -309,8 +317,11 @@ class EmbeddedWorkerInstance::StartTask {
         break;
       case ProcessAllocationState::ALLOCATING:
         // Abort half-baked process allocation on the UI thread.
-        instance_->context_->process_manager()->ReleaseWorkerProcess(
-            instance_->embedded_worker_id());
+        BrowserThread::PostTask(
+            BrowserThread::UI, FROM_HERE,
+            base::BindOnce(&ServiceWorkerProcessManager::ReleaseWorkerProcess,
+                           instance_->context_->process_manager()->AsWeakPtr(),
+                           instance_->embedded_worker_id()));
         break;
       case ProcessAllocationState::ALLOCATED:
         // Otherwise, the process will be released by EmbeddedWorkerInstance.
@@ -461,6 +472,7 @@ bool EmbeddedWorkerInstance::Listener::OnMessageReceived(
 }
 
 EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(status_ == EmbeddedWorkerStatus::STOPPING ||
          status_ == EmbeddedWorkerStatus::STOPPED)
       << static_cast<int>(status_);
@@ -584,7 +596,9 @@ EmbeddedWorkerInstance::EmbeddedWorkerInstance(
       instance_host_binding_(this),
       devtools_attached_(false),
       network_accessed_for_script_(false),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+}
 
 void EmbeddedWorkerInstance::OnProcessAllocated(
     std::unique_ptr<WorkerProcessHandle> handle,

content/browser/service_worker/embedded_worker_instance.h

@@ -47,6 +47,8 @@ class ServiceWorkerContextCore;
 // This gives an interface to control one EmbeddedWorker instance, which
 // may be 'in-waiting' or running in one of the child processes added by
 // AddProcessReference().
+//
+// Owned by ServiceWorkerVersion. Lives on the IO thread.
 class CONTENT_EXPORT EmbeddedWorkerInstance
     : NON_EXPORTED_BASE(public mojom::EmbeddedWorkerInstanceHost) {
  public:

content/browser/service_worker/service_worker_process_manager.cc

@@ -237,16 +237,7 @@ ServiceWorkerStatusCode ServiceWorkerProcessManager::AllocateWorkerProcess(
 }
 
 void ServiceWorkerProcessManager::ReleaseWorkerProcess(int embedded_worker_id) {
-  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    BrowserThread::PostTask(
-        BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(&ServiceWorkerProcessManager::ReleaseWorkerProcess,
-                   weak_this_,
-                   embedded_worker_id));
-    return;
-  }
-
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (process_id_for_test_ != ChildProcessHost::kInvalidUniqueID) {
     // Unittests don't increment or decrement the worker refcount of a
     // RenderProcessHost.
@@ -267,7 +258,7 @@ void ServiceWorkerProcessManager::ReleaseWorkerProcess(int embedded_worker_id) {
   if (info == instance_info_.end())
     return;
 
-  RenderProcessHost* rph = NULL;
+  RenderProcessHost* rph = nullptr;
   if (info->second.site_instance.get()) {
     rph = info->second.site_instance->GetProcess();
     DCHECK_EQ(info->second.process_id, rph->GetID())

content/browser/service_worker/service_worker_process_manager.h

@@ -60,9 +60,11 @@ class CONTENT_EXPORT ServiceWorkerProcessManager {
   // Returns a reference to a running process suitable for starting the service
   // worker described by |emdedded_worker_id|, |pattern|, and |script_url|.
   //
-  // AllocateWorkerProcess() tries to return an existing process. If one is not
-  // available, or |can_use_existing_process| is false, it will create a new
-  // one.
+  // AllocateWorkerProcess() tries to return a process that already exists in
+  // this ServiceWorkerProcessManager's map of processes, populated via
+  // AddProcessReferenceToPattern().  If one is not found, or
+  // |can_use_existing_process| is false, it will instead use SiteInstance to
+  // get a process, possibly creating a new one.
   //
   // If SERVICE_WORKER_OK is returned, |out_info| contains information about the
   // process.
@@ -79,6 +81,8 @@ class CONTENT_EXPORT ServiceWorkerProcessManager {
 
   // Drops a reference to a process that was running a Service Worker, and its
   // SiteInstance.  This must match a call to AllocateWorkerProcess.
+  //
+  // Called on the UI thread.
   void ReleaseWorkerProcess(int embedded_worker_id);
 
   // Sets a single process ID that will be used for all embedded workers.  This