ServiceWorker: Ensure live registration during starting worker

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 53f05f35587fccc65f3b26b5fb4fadef4e6cd0c0..ac0c676c1bf438774efcd73170cc20a55ff3721c 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -91,6 +91,13 @@ void RunIDMapCallbacks(IDMAP* callbacks, const Params&... params) {
   callbacks->Clear();
 }
 
+void RunStartWorkerCallback(
+    const StatusCallback& callback,
+    scoped_refptr<ServiceWorkerRegistration> protect,
+    ServiceWorkerStatusCode status) {
+  callback.Run(status);
+}
+
 // A callback adapter to start a |task| after StartWorker.
 void RunTaskAfterStartWorker(
     base::WeakPtr<ServiceWorkerVersion> version,
@@ -377,30 +384,27 @@ void ServiceWorkerVersion::StartWorker(const StatusCallback& callback) {
 void ServiceWorkerVersion::StartWorker(
     bool pause_after_download,
     const StatusCallback& callback) {
-  if (is_doomed()) {
+  if (!context_) {
     RunSoon(base::Bind(callback, SERVICE_WORKER_ERROR_START_WORKER_FAILED));
     return;
   }
-  switch (running_status()) {
-    case RUNNING:
-      RunSoon(base::Bind(callback, SERVICE_WORKER_OK));
-      return;
-    case STOPPING:
-    case STOPPED:
-    case STARTING:
-      if (!timeout_timer_.IsRunning())
-        StartTimeoutTimer();
-      start_callbacks_.push_back(callback);
-      if (running_status() == STOPPED) {
-        DCHECK(!cache_listener_.get());
-        cache_listener_.reset(new ServiceWorkerCacheListener(this, context_));
-        embedded_worker_->Start(
-            version_id_, scope_, script_url_, pause_after_download,
-            base::Bind(&ServiceWorkerVersion::OnStartSentAndScriptEvaluated,
-                       weak_factory_.GetWeakPtr()));
-      }
-      return;
+
+  // Ensure the live registration during starting worker so that the worker can
+  // get associated with it ServiceWorkerDispatcherHost::OnSetHostedVersionId().
+  scoped_refptr<ServiceWorkerRegistration> protect =
+      context_->GetLiveRegistration(registration_id_);

[michaeln, 2015/03/12 19:34:02]

drive by: This call and early return adds no value, FindRegistrationForId() has
a fast return path (see CompleteFindNow) when there is a 'live' reg in memory.

[nhiroki, 2015/03/13 01:21:31]

Done.

+  if (protect) {
+    DidEnsureLiveRegistrationForStartWorker(
+        pause_after_download, callback, SERVICE_WORKER_OK, protect);
+    return;
   }
+  context_->storage()->FindRegistrationForId(
+      registration_id_,
+      scope_.GetOrigin(),
+      base::Bind(&ServiceWorkerVersion::DidEnsureLiveRegistrationForStartWorker,
+                 weak_factory_.GetWeakPtr(),
+                 pause_after_download,
+                 callback));
 }
 
 void ServiceWorkerVersion::StopWorker(const StatusCallback& callback) {
@@ -788,8 +792,6 @@ void ServiceWorkerVersion::SetDevToolsAttached(bool attached) {
     ClearTick(&start_time_);
     return;
   }
-  if (!timeout_timer_.IsRunning())
-    StartTimeoutTimer();

[kinuko, 2015/03/12 15:50:34]

nit: Could we do this in another CL, as it's not related to this change?

[nhiroki, 2015/03/13 01:21:30]

Reverted this and uploaded a separate patch:
https://codereview.chromium.org/1004913003/

 }
 
 void ServiceWorkerVersion::SetMainScriptHttpResponseInfo(
@@ -1413,6 +1415,40 @@ void ServiceWorkerVersion::OnPongFromWorker() {
   ClearTick(&ping_time_);
 }
 
+void ServiceWorkerVersion::DidEnsureLiveRegistrationForStartWorker(
+    bool pause_after_download,
+    const StatusCallback& callback,
+    ServiceWorkerStatusCode status,
+    const scoped_refptr<ServiceWorkerRegistration>& protect) {
+  if (status != SERVICE_WORKER_OK || is_doomed()) {
+    RunSoon(base::Bind(callback, SERVICE_WORKER_ERROR_START_WORKER_FAILED));
+    return;
+  }
+
+  switch (running_status()) {
+    case RUNNING:
+      RunSoon(base::Bind(callback, SERVICE_WORKER_OK));
+      return;
+    case STOPPING:
+    case STOPPED:
+    case STARTING:
+      if (!timeout_timer_.IsRunning())
+        StartTimeoutTimer();
+      // Start callbacks keep the live registration.
+      start_callbacks_.push_back(
+          base::Bind(&RunStartWorkerCallback, callback, protect));
+      if (running_status() == STOPPED) {
+        DCHECK(!cache_listener_.get());
+        cache_listener_.reset(new ServiceWorkerCacheListener(this, context_));
+        embedded_worker_->Start(
+            version_id_, scope_, script_url_, pause_after_download,
+            base::Bind(&ServiceWorkerVersion::OnStartSentAndScriptEvaluated,
+                       weak_factory_.GetWeakPtr()));
+      }
+      return;
+  }
+}
+
 void ServiceWorkerVersion::DidClaimClients(
     int request_id, ServiceWorkerStatusCode status) {
   if (status == SERVICE_WORKER_ERROR_STATE) {