ServiceWorker: Ensure live registration during starting worker

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 "base/command_line.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
@@ skipping 73 matching lines @@
 template <typename IDMAP, typename... Params>
 void RunIDMapCallbacks(IDMAP* callbacks, const Params&... params) {
   typename IDMAP::iterator iter(callbacks);
   while (!iter.IsAtEnd()) {
     iter.GetCurrentValue()->Run(params...);
     iter.Advance();
   }
   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,
     const StatusCallback& error_callback,
     const base::Closure& task,
     ServiceWorkerStatusCode status) {
   if (status != SERVICE_WORKER_OK) {
     if (!error_callback.is_null())
       error_callback.Run(status);
     return;
@@ skipping 266 matching lines @@
       embedded_worker()->worker_devtools_agent_route_id());
 }
 
 void ServiceWorkerVersion::StartWorker(const StatusCallback& callback) {
   StartWorker(false, 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) {
   if (running_status() == STOPPED) {
     RunSoon(base::Bind(callback, SERVICE_WORKER_OK));
     return;
   }
   if (stop_callbacks_.empty()) {
     ServiceWorkerStatusCode status = embedded_worker_->Stop();
     if (status != SERVICE_WORKER_OK) {
@@ skipping 367 matching lines @@
     DoomInternal();
 }
 
 void ServiceWorkerVersion::SetDevToolsAttached(bool attached) {
   embedded_worker()->set_devtools_attached(attached);
   if (attached) {
     // Set to null time so we don't record the startup time metric.
     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(
     const net::HttpResponseInfo& http_info) {
   main_script_http_info_.reset(new net::HttpResponseInfo(http_info));
 }
 
 const net::HttpResponseInfo*
 ServiceWorkerVersion::GetMainScriptHttpResponseInfo() {
   return main_script_http_info_.get();
@@ skipping 603 matching lines @@
     callback.Run(SERVICE_WORKER_ERROR_ABORT);
     return;
   }
   registration->ClaimClients(callback);
 }
 
 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) {
     embedded_worker_->SendMessage(ServiceWorkerMsg_ClaimClientsError(
         request_id, blink::WebServiceWorkerError::ErrorTypeState,
         base::ASCIIToUTF16(kClaimClientsStateErrorMesage)));
     return;
   }
   if (status == SERVICE_WORKER_ERROR_ABORT) {
     embedded_worker_->SendMessage(ServiceWorkerMsg_ClaimClientsError(
@@ skipping 189 matching lines @@
     int request_id) {
   callbacks->Remove(request_id);
   if (is_doomed_) {
     // The stop should be already scheduled, but try to stop immediately, in
     // order to release worker resources soon.
     StopWorkerIfIdle();
   }
 }
 
 }  // namespace content