Start EmbeddedWorker during registration - take 2

content/browser/service_worker/service_worker_register_job.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_register_job.h"
 
 #include <vector>
 
 #include "content/browser/service_worker/service_worker_job_coordinator.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
 
 namespace content {
 
 ServiceWorkerRegisterJob::ServiceWorkerRegisterJob(
     ServiceWorkerStorage* storage,
+    EmbeddedWorkerRegistry* worker_registry,
     ServiceWorkerJobCoordinator* coordinator,
     const GURL& pattern,
     const GURL& script_url,
     RegistrationType type)
     : storage_(storage),
+      worker_registry_(worker_registry),
       coordinator_(coordinator),
+      pending_version_(NULL),
       pattern_(pattern),
       script_url_(script_url),
       type_(type),
       weak_factory_(this) {}
 
 ServiceWorkerRegisterJob::~ServiceWorkerRegisterJob() {}
 
 void ServiceWorkerRegisterJob::AddCallback(
-    const RegistrationCallback& callback) {
+    const RegistrationCallback& callback,
+    ServiceWorkerProviderHost* source_provider) {
+  // if we've created a pending version, associate source_provider it with
+  // that, otherwise queue it up
   callbacks_.push_back(callback);
+  DCHECK(source_provider);
+  if (pending_version_) {
+    pending_version_->OnAssociateProvider(source_provider);

[kinuko, 2014/01/20 09:39:09]

How do we relate this to ServiceWorkerRegistration's pending_ and
active_version, and ProviderHost's associated_version?

I think it'd be probably better to first clear up how ProviderHost -
Registration - Version should be connected with each other (while StartWorker
part is being blocked by my other patch).

I assume:
- Provider / ProviderHost's created per document and tracks what URL it's
rendering, and
- Provider / ProviderHost is associated with a particular ServiceWorkerVersion
if its document URL matches with a registration

And currently we don't have either part. Is it possible to plumb this part
first?

Also I assumed we'd like to run EW on a process that renders a document whose
navigator.serviceWorker is associated with the corresponding
ServiceWorkerVersion, but on the second thought it doesn't need to use the
current association information, we could just run it on any process that
belongs to the same origin (using the document URL information in SWPH)?
--otherwise we'll have a bootstrapping problem as this patch has exposed.

+  } else {
+    pending_provider_hosts_.push_back(source_provider);
+  }
 }
 
 void ServiceWorkerRegisterJob::Start() {
   if (type_ == REGISTER)
     StartRegister();
   else
     StartUnregister();
 }
 
 bool ServiceWorkerRegisterJob::Equals(ServiceWorkerRegisterJob* job) {
   return job->type_ == type_ &&
          (type_ == ServiceWorkerRegisterJob::UNREGISTER ||
           job->script_url_ == script_url_);
 }
 
 void ServiceWorkerRegisterJob::StartRegister() {
   // Set up a chain of callbacks, in reverse order. Each of these
   // callbacks may be called asynchronously by the previous callback.
-  RegistrationCallback finish_registration(base::Bind(
-      &ServiceWorkerRegisterJob::RegisterComplete, weak_factory_.GetWeakPtr()));
+  base::Callback<void(ServiceWorkerRegistrationStatus,
+                      const scoped_refptr<ServiceWorkerRegistration>&,
+                      ServiceWorkerVersionStatus)>
+      finish_registration(
+          base::Bind(&ServiceWorkerRegisterJob::RegisterComplete,
+                     weak_factory_.GetWeakPtr()));
+
+  RegistrationCallback start_worker(
+      base::Bind(&ServiceWorkerRegisterJob::StartWorkerAndContinue,
+                 weak_factory_.GetWeakPtr(),
+                 finish_registration));
 
   UnregistrationCallback register_new(
       base::Bind(&ServiceWorkerRegisterJob::RegisterPatternAndContinue,
                  weak_factory_.GetWeakPtr(),
-                 finish_registration));
+                 start_worker));
 
   ServiceWorkerStorage::FindRegistrationCallback unregister_old(
       base::Bind(&ServiceWorkerRegisterJob::UnregisterPatternAndContinue,
                  weak_factory_.GetWeakPtr(),
                  register_new));

[kinuko, 2014/01/28 06:34:55]

While revisiting the installation flow this just caught me-- we always
unregister registration (which unrefs the old registration in in-memory store in
the current code) before installing a new one, but if so how do we maintain the
'pending' and 'active' version for the same registration?

ServiceWorkerRegistration has active_version_ and pending_version_ fields, so it
seems it's supposed to keep both while update is ongoing?

 
   storage_->FindRegistrationForPattern(pattern_, unregister_old);
 }
 
 void ServiceWorkerRegisterJob::StartUnregister() {
   // Set up a chain of callbacks, in reverse order. Each of these
   // callbacks may be called asynchronously by the previous callback.
   UnregistrationCallback finish_unregistration(
       base::Bind(&ServiceWorkerRegisterJob::UnregisterComplete,
                  weak_factory_.GetWeakPtr()));
@@ skipping 16 matching lines @@
         base::Bind(callback,
                    previous_status,
                    scoped_refptr<ServiceWorkerRegistration>()));
     return;
   }
 
   // TODO: Eventually RegisterInternal will be replaced by an asynchronous
   // operation. Pass its resulting status through 'callback'.
   scoped_refptr<ServiceWorkerRegistration> registration =
       storage_->RegisterInternal(pattern_, script_url_);
+
   BrowserThread::PostTask(BrowserThread::IO,
                           FROM_HERE,
                           base::Bind(callback, REGISTRATION_OK, registration));
 }
 
 void ServiceWorkerRegisterJob::UnregisterPatternAndContinue(
     const UnregistrationCallback& callback,
     bool found,
     ServiceWorkerRegistrationStatus previous_status,
     const scoped_refptr<ServiceWorkerRegistration>& previous_registration) {
 
   // The previous registration may not exist, which is ok.
   if (previous_status == REGISTRATION_OK && found &&
       (script_url_.is_empty() ||
        previous_registration->script_url() != script_url_)) {
     // TODO: Eventually UnregisterInternal will be replaced by an
     // asynchronous operation. Pass its resulting status though
     // 'callback'.
     storage_->UnregisterInternal(pattern_);
     DCHECK(previous_registration->is_shutdown());
   }
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE, base::Bind(callback, previous_status));
 }
 
+void ServiceWorkerRegisterJob::StartWorkerAndContinue(
+    const base::Callback<void(ServiceWorkerRegistrationStatus,
+                              const scoped_refptr<ServiceWorkerRegistration>&,
+                              ServiceWorkerVersionStatus)> callback,
+    ServiceWorkerRegistrationStatus status,
+    const scoped_refptr<ServiceWorkerRegistration>& registration) {
+  pending_version_ = new ServiceWorkerVersion(
+      registration, worker_registry_, registration->next_version_id(), -1L);
+  LOG(ERROR) << "StartWorkerAndContinue...";
+  for (std::vector<ServiceWorkerProviderHost*>::const_iterator it =
+           pending_provider_hosts_.begin();
+       it != pending_provider_hosts_.end();
+       ++it)
+    pending_version_->OnAssociateProvider(*it);

[kinuko, 2014/01/20 09:39:09]

As I wrote above I feel this association should be probably done in two parts:

- for EW process assignment, which should be wired probably outside this
registration job
- for ServiceWorkerProviderHost - ServiceWorkerVersion association, which should
be done after installation/activation is done and a particular version is
associated to the ServiceWorkerProvider(Host). It'll going to be used to serve
onfetch() event for the document

+
+  pending_version_->StartWorker(base::Bind(callback, status, registration));
+}
+
 void ServiceWorkerRegisterJob::RunCallbacks(
     ServiceWorkerRegistrationStatus status,
     const scoped_refptr<ServiceWorkerRegistration>& registration) {
   for (std::vector<RegistrationCallback>::iterator it = callbacks_.begin();
        it != callbacks_.end();
        ++it) {
     it->Run(status, registration);
   }
 }
 void ServiceWorkerRegisterJob::RegisterComplete(
     ServiceWorkerRegistrationStatus status,
-    const scoped_refptr<ServiceWorkerRegistration>& registration) {
-  RunCallbacks(status, registration);
+    const scoped_refptr<ServiceWorkerRegistration>& registration,
+    ServiceWorkerVersionStatus version_status) {
+  ServiceWorkerRegistrationStatus registration_status;
+  switch (version_status) {
+    case SERVICE_WORKER_VERSION_OK:
+      registration_status = REGISTRATION_OK;
+      break;
+
+    case SERVICE_WORKER_VERSION_ERROR_FAILED:
+    case SERVICE_WORKER_VERSION_ERROR_START_FAILED:
+    case SERVICE_WORKER_VERSION_ERROR_INSTALL_FAILED:
+      registration_status = REGISTRATION_INSTALL_FAILED;
+      break;
+  }
+  RunCallbacks(registration_status, registration);
   coordinator_->FinishJob(pattern_, this);
 }
 
 void ServiceWorkerRegisterJob::UnregisterComplete(
     ServiceWorkerRegistrationStatus status) {
   RunCallbacks(status, NULL);
   coordinator_->FinishJob(pattern_, this);
 }
 
 }  // namespace content