Teach EmbeddedWorkerInstance to create a process when it needs one.

content/browser/service_worker/service_worker_process_manager.cc

Index: content/browser/service_worker/service_worker_process_manager.cc
diff --git a/content/browser/service_worker/service_worker_process_manager.cc b/content/browser/service_worker/service_worker_process_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a9bff94bb25ca19d3d31f5bb612c65ff3bbd8622
--- /dev/null
+++ b/content/browser/service_worker/service_worker_process_manager.cc
@@ -0,0 +1,139 @@
+// Copyright 2014 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/service_worker/service_worker_process_manager.h"
+
+#include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/site_instance.h"
+#include "url/gurl.h"
+
+namespace base {
+// Destroying ServiceWorkerProcessManagers only on the UI thread allows the
+// member WeakPtr to safely guard the object's lifetime when used on that
+// thread.
+void DefaultDeleter<content::ServiceWorkerProcessManager>::operator()(
+    content::ServiceWorkerProcessManager* ptr) const {
+  content::BrowserThread::DeleteSoon(
+      content::BrowserThread::UI, FROM_HERE, ptr);
+}
+}  // namespace base
+
+namespace content {
+
+ServiceWorkerProcessManager::ServiceWorkerProcessManager(
+    ServiceWorkerContextWrapper* context_wrapper,
+    const base::Callback<bool(int)>& increment_for_test,
+    const base::Callback<bool(int)>& decrement_for_test)
+    : context_wrapper_(context_wrapper),
+      increment_for_test_(increment_for_test),
+      decrement_for_test_(decrement_for_test),
+      weak_this_factory_(this),
+      weak_this_(weak_this_factory_.GetWeakPtr()) {
+}
+
+ServiceWorkerProcessManager::~ServiceWorkerProcessManager() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+}
+
+void ServiceWorkerProcessManager::AllocateWorkerProcess(
+    const std::vector<int>& process_ids,
+    const GURL& script_url,
+    const base::Callback<void(ServiceWorkerStatusCode, int process_id)>&
+        callback) const {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&ServiceWorkerProcessManager::AllocateWorkerProcess,
+                   weak_this_,

[kinuko, 2014/04/28 06:58:16]

nit: This doesn't seem necessary given that it's posted from IO -> UI as in the
same order when we destruct this instance, but well that's probably why you
commented 'double-check' in the header..

[Jeffrey Yasskin, 2014/04/28 20:47:40]

Yeah, I believe we don't need the WeakPtrFactory at all with the current
implementation, but I always hate tracing other people's code when they need the
order of calls to match in two separate parts of the code, so I didn't want to
cause anyone that problem in the future.

+                   process_ids,
+                   script_url,
+                   callback));
+    return;
+  }
+
+  for (std::vector<int>::const_iterator it = process_ids.begin();
+       it != process_ids.end();
+       ++it) {
+    if (IncrementWorkerRefcountByPid(*it)) {
+      BrowserThread::PostTask(BrowserThread::IO,
+                              FROM_HERE,
+                              base::Bind(callback, SERVICE_WORKER_OK, *it));
+      return;
+    }
+  }
+
+  if (!context_wrapper_->browser_context_) {
+    // Shutdown has started.

[kinuko, 2014/04/28 06:58:16]

Should this be checked before we loop over process_ids?

[Jeffrey Yasskin, 2014/04/28 20:47:40]

No, if we do that, a lot of the unittests would need to create a BrowserContext.
With the current ordering, they can override increment_for_test_, and cause the
allocation to succeed.

+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(callback, SERVICE_WORKER_ERROR_START_WORKER_FAILED, -1));
+  }
+  // No existing processes available; start a new one.
+  scoped_refptr<SiteInstance> site_instance = SiteInstance::CreateForURL(
+      context_wrapper_->browser_context_, script_url);
+  RenderProcessHost* rph = site_instance->GetProcess();
+  // This Init() call posts a task to the IO thread that adds the RPH's
+  // ServiceWorkerDispatcherHost to the
+  // EmbeddedWorkerRegistry::process_sender_map_.
+  if (!rph->Init()) {
+    LOG(ERROR) << "Couldn't start a new process!";
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(callback, SERVICE_WORKER_ERROR_START_WORKER_FAILED, -1));
+    return;
+  }
+
+  static_cast<RenderProcessHostImpl*>(rph)->IncrementWorkerRefCount();
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(callback, SERVICE_WORKER_OK, rph->GetID()));
+}
+
+void ServiceWorkerProcessManager::ReleaseWorkerProcess(int process_id) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&ServiceWorkerProcessManager::ReleaseWorkerProcess,
+                   weak_this_,
+                   process_id));
+    return;
+  }
+  if (!DecrementWorkerRefcountByPid(process_id)) {
+    DCHECK(false) << "DecrementWorkerRef(" << process_id
+                  << ") doesn't match a previous IncrementWorkerRef";
+  }
+}
+
+bool ServiceWorkerProcessManager::IncrementWorkerRefcountByPid(
+    int process_id) const {
+  if (!increment_for_test_.is_null())
+    return increment_for_test_.Run(process_id);
+
+  RenderProcessHost* rph = RenderProcessHost::FromID(process_id);
+  if (rph && !rph->FastShutdownStarted())
+    static_cast<RenderProcessHostImpl*>(rph)->IncrementWorkerRefCount();
+
+  return rph != NULL;

[kinuko, 2014/04/28 06:58:16]

Since we only increment worker-ref when line 121 is true, maybe we should

if (rph && !rph->FastShutdownStarted()) {
  rph->IncrementWorkerRefCount();
  return true;
}
return false;

(or vice versa)

[Jeffrey Yasskin, 2014/04/28 20:47:40]

Yep, I forgot to update enough of this when I added FastShutdownStarted.

+}
+
+bool ServiceWorkerProcessManager::DecrementWorkerRefcountByPid(
+    int process_id) const {
+  if (!decrement_for_test_.is_null())
+    return decrement_for_test_.Run(process_id);
+
+  RenderProcessHost* rph = RenderProcessHost::FromID(process_id);
+  if (rph)
+    static_cast<RenderProcessHostImpl*>(rph)->DecrementWorkerRefCount();
+
+  return rph != NULL;
+}
+
+}  // namespace content