Change RunAfterStartWorker to not start the worker if it is already running.

content/browser/service_worker/service_worker_version.cc

Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index b77ba3b06ec0ab61b2ac67e528e0bd38e8612d57..999391882054227e2f09867dd35390f742b38826 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -506,8 +506,12 @@ bool ServiceWorkerVersion::FinishRequest(int request_id) {
 }
 
 void ServiceWorkerVersion::RunAfterStartWorker(
-    const StatusCallback& error_callback,
-    const base::Closure& task) {
+    const base::Closure& task,
+    const StatusCallback& error_callback) {
+  if (running_status() == RUNNING) {
+    RunSoon(task);

[johnme, 2016/01/18 19:06:51]

Using RunSoon (and hence PostTask) introduces a race condition: the SW might
currently be running, but stop before the task runs. Instead, how about:

void ServiceWorkerVersion::RunAfterStartWorker(
    const base::Closure& task,
    const StatusCallback& error_callback) {
  RunSoon(base::Bind(&ServiceWorkerVersion::RunAfterStartWorkerInternal, ...);
}
void ServiceWorkerVersion::RunAfterStartWorkerInternal(
    const base::Closure& task) {
  if (running_status() == RUNNING) {
    task.Run();
  } else {
    StartWorker(base::Bind(&RunTaskAfterStartWorker, ...));
  }
}

[nhiroki, 2016/01/19 01:36:39]

Or, we could immediately run the callback when the worker is running.

void ServiceWorkerVersion::RunAfterStartWorker(
    const base::Closure& task,
    const StatusCallback& error_callback) {
  if (running_status() == RUNNING) {
    DCHECK(start_callbacks_.empty());
    task.Run();
    return;
  }
  StartWorker(...);
}

[Marijn Kruisselbrink, 2016/01/19 22:49:15]

Normally I don't like methods that sometimes call a callback synchronously and
sometimes async (how I ended up with the RunSoon in the first place), but I
think in this case it might actually be the best option, so I've changed it to
that (and expanded the documentation for RunAfterStartWorker to make it clear
that is the behavior).

+    return;
+  }
   StartWorker(base::Bind(&RunTaskAfterStartWorker, weak_factory_.GetWeakPtr(),
                          error_callback, task));

[nhiroki, 2016/01/19 01:36:39]

You may also want to swap arguments of RunTaskAfterStartWorker.

[Marijn Kruisselbrink, 2016/01/19 22:49:15]

I could, but I don't think it's worth it yet (given all the other use
RunTaskAfterStartWorker still has). But I'll definitely keep it in mind for the
cleanup after all events have been migrated.

 }