Save running SW instance info, including its SiteInstance, into the ProcessManager.

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
index b6a2891e2d2254d718312c68c5daee83369051a3..355da22bdc95399ac99016de3077843d8460234b 100644
--- a/content/browser/service_worker/service_worker_process_manager.cc
+++ b/content/browser/service_worker/service_worker_process_manager.cc
@@ -12,9 +12,21 @@
 
 namespace content {
 
+static bool IncrementWorkerRefcountByPid(

[dominicc (has gone to gerrit), 2014/05/20 02:01:18]

The casing of Refcount is inconsistent with IncrementWorkerRefCount. Which one
is right?

[Jeffrey Yasskin, 2014/05/20 05:20:10]

IncrementWorkerRefCount is pre-existing, so I've fixed this one.

+    int process_id) {
+  RenderProcessHost* rph = RenderProcessHost::FromID(process_id);
+  if (rph && !rph->FastShutdownStarted()) {

[dominicc (has gone to gerrit), 2014/05/20 02:01:18]

Would it be more readable to make this an early return w/ false?

[Jeffrey Yasskin, 2014/05/20 05:20:10]

It looks pretty similar.

+    static_cast<RenderProcessHostImpl*>(rph)->IncrementWorkerRefCount();
+    return true;
+  }
+
+  return false;
+}
+
 ServiceWorkerProcessManager::ServiceWorkerProcessManager(
     ServiceWorkerContextWrapper* context_wrapper)
     : context_wrapper_(context_wrapper),
+      process_id_for_test_(-1),
       weak_this_factory_(this),
       weak_this_(weak_this_factory_.GetWeakPtr()) {
 }
@@ -24,26 +36,43 @@ ServiceWorkerProcessManager::~ServiceWorkerProcessManager() {
 }
 
 void ServiceWorkerProcessManager::AllocateWorkerProcess(
+    int embedded_worker_id,
     const std::vector<int>& process_ids,
     const GURL& script_url,
     const base::Callback<void(ServiceWorkerStatusCode, int process_id)>&
-        callback) const {
+        callback) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     BrowserThread::PostTask(
         BrowserThread::UI,
         FROM_HERE,
         base::Bind(&ServiceWorkerProcessManager::AllocateWorkerProcess,
                    weak_this_,
+                   embedded_worker_id,
                    process_ids,
                    script_url,
                    callback));
     return;
   }
 
+  if (process_id_for_test_ != -1) {
+    // Let tests specify the returned process ID.  Note: We may need to be able

[dominicc (has gone to gerrit), 2014/05/20 02:01:18]

One space after .?

[Jeffrey Yasskin, 2014/05/20 05:20:10]

Done.

+    // to specify the error code too.
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(callback, SERVICE_WORKER_OK, process_id_for_test_));
+    return;
+  }
+
+  DCHECK(!ContainsKey(instance_info_, embedded_worker_id))
+      << embedded_worker_id << " already has a process allocated";
+
   for (std::vector<int>::const_iterator it = process_ids.begin();
        it != process_ids.end();
        ++it) {
     if (IncrementWorkerRefcountByPid(*it)) {
+      instance_info_.insert(
+          std::make_pair(embedded_worker_id, ProcessInfo(*it)));
       BrowserThread::PostTask(BrowserThread::IO,
                               FROM_HERE,
                               base::Bind(callback, SERVICE_WORKER_OK, *it));
@@ -75,6 +104,9 @@ void ServiceWorkerProcessManager::AllocateWorkerProcess(
     return;
   }
 
+  instance_info_.insert(
+      std::make_pair(embedded_worker_id, ProcessInfo(site_instance)));
+
   static_cast<RenderProcessHostImpl*>(rph)->IncrementWorkerRefCount();
   BrowserThread::PostTask(
       BrowserThread::IO,
@@ -82,53 +114,49 @@ void ServiceWorkerProcessManager::AllocateWorkerProcess(
       base::Bind(callback, SERVICE_WORKER_OK, rph->GetID()));
 }
 
-void ServiceWorkerProcessManager::ReleaseWorkerProcess(int process_id) {
+void ServiceWorkerProcessManager::ReleaseWorkerProcess(int embedded_worker_id) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     BrowserThread::PostTask(
         BrowserThread::UI,
         FROM_HERE,
         base::Bind(&ServiceWorkerProcessManager::ReleaseWorkerProcess,
                    weak_this_,
-                   process_id));
+                   embedded_worker_id));
+    return;
+  }
+  if (process_id_for_test_ != -1) {
+    // Unittests don't increment or decrement the worker refcount of a
+    // RenderProcessHost.
     return;
   }
-  if (!DecrementWorkerRefcountByPid(process_id)) {
-    DCHECK(false) << "DecrementWorkerRef(" << process_id
-                  << ") doesn't match a previous IncrementWorkerRef";
+  std::map<int, ProcessInfo>::iterator info =
+      instance_info_.find(embedded_worker_id);
+  DCHECK(info != instance_info_.end());
+  RenderProcessHost* rph = NULL;
+  if (info->second.site_instance) {
+    rph = info->second.site_instance->GetProcess();
+    DCHECK_EQ(info->second.process_id, rph->GetID())
+        << "A SiteInstance's process shouldn't get destroyed while we're "
+           "holding a reference to it. Was the reference actually held?";
+  } else {
+    rph = RenderProcessHost::FromID(info->second.process_id);
+    DCHECK(rph)
+        << "Process " << info->second.process_id
+        << " was destroyed unexpectedly. Did we actually hold a reference?";
   }
+  static_cast<RenderProcessHostImpl*>(rph)->DecrementWorkerRefCount();
+  instance_info_.erase(info);
 }
 
-void ServiceWorkerProcessManager::SetProcessRefcountOpsForTest(
-    const base::Callback<bool(int)>& increment_for_test,
-    const base::Callback<bool(int)>& decrement_for_test) {
-  increment_for_test_ = increment_for_test;
-  decrement_for_test_ = decrement_for_test;
+ServiceWorkerProcessManager::ProcessInfo::ProcessInfo(
+    const scoped_refptr<SiteInstance>& site_instance)
+    : site_instance(site_instance),
+      process_id(site_instance->GetProcess()->GetID()) {
 }
-
-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 true;
-  }
-
-  return false;
+ServiceWorkerProcessManager::ProcessInfo::ProcessInfo(int process_id)
+    : process_id(process_id) {
 }
-
-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;
+ServiceWorkerProcessManager::ProcessInfo::~ProcessInfo() {
 }
 
 }  // namespace content