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

content/browser/service_worker/service_worker_version.cc

 // Copyright 2013 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_version.h"
 
 #include <stddef.h>
 
 #include <map>
 #include <string>
@@ skipping 488 matching lines @@
   PendingRequest<StatusCallback>* request = custom_requests_.Lookup(request_id);
   if (!request)
     return false;
   ServiceWorkerMetrics::RecordEventDuration(
       request->event_type, base::TimeTicks::Now() - request->start_time);
   RemoveCallbackAndStopIfRedundant(&custom_requests_, request_id);
   return true;
 }
 
 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.

 }
 
 void ServiceWorkerVersion::DispatchMessageEvent(
     const base::string16& message,
     const std::vector<TransferredMessagePort>& sent_message_ports,
     const StatusCallback& callback) {
   for (const TransferredMessagePort& port : sent_message_ports) {
     MessagePortService::GetInstance()->HoldMessages(port.id);
   }
 
@@ skipping 1455 matching lines @@
 void ServiceWorkerVersion::OnBeginEvent() {
   if (should_exclude_from_uma_ || running_status() != RUNNING ||
       idle_time_.is_null()) {
     return;
   }
   ServiceWorkerMetrics::RecordTimeBetweenEvents(base::TimeTicks::Now() -
                                                 idle_time_);
 }
 
 }  // namespace content