Start EmbeddedWorker during registration - take 2

content/browser/service_worker/service_worker_register_job.cc

Index: content/browser/service_worker/service_worker_register_job.cc
diff --git a/content/browser/service_worker/service_worker_register_job.cc b/content/browser/service_worker/service_worker_register_job.cc
index 016855bab14aa0a627edcce99d0c884d574275a7..1f9343e3936646ef0413a2a988301f31642603fe 100644
--- a/content/browser/service_worker/service_worker_register_job.cc
+++ b/content/browser/service_worker/service_worker_register_job.cc
@@ -15,12 +15,15 @@ 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),
@@ -29,8 +32,17 @@ ServiceWorkerRegisterJob::ServiceWorkerRegisterJob(
 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() {
@@ -49,13 +61,22 @@ bool ServiceWorkerRegisterJob::Equals(ServiceWorkerRegisterJob* job) {
 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,
@@ -97,6 +118,7 @@ void ServiceWorkerRegisterJob::RegisterPatternAndContinue(
   // 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));
@@ -122,6 +144,24 @@ void ServiceWorkerRegisterJob::UnregisterPatternAndContinue(
       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) {
@@ -133,8 +173,21 @@ void ServiceWorkerRegisterJob::RunCallbacks(
 }
 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);
 }