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 f293237f780dfb87e9759bc9b155a0173e8ed379..a21cbb25b31a7666024269335c00c35ae3f6333f 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),
@@ -28,9 +31,17 @@ ServiceWorkerRegisterJob::ServiceWorkerRegisterJob(
 
 ServiceWorkerRegisterJob::~ServiceWorkerRegisterJob() {}
 
-void ServiceWorkerRegisterJob::AddCallback(
-    const RegistrationCallback& callback) {
+void ServiceWorkerRegisterJob::AddCallback(const RegistrationCallback& callback,
+                                           int process_id) {
+  // if we've created a pending version, associate source_provider it with
+  // that, otherwise queue it up
   callbacks_.push_back(callback);
+  DCHECK(process_id != -1);
+  if (pending_version_) {
+    pending_version_->AddProcessToWorker(process_id);
+  } else {
+    pending_process_ids_.push_back(process_id);
+  }
 }
 
 void ServiceWorkerRegisterJob::Start() {
@@ -49,13 +60,18 @@ 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(
+  StatusCallback 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,
@@ -80,6 +96,32 @@ void ServiceWorkerRegisterJob::StartUnregister() {
   storage_->FindRegistrationForPattern(pattern_, unregister);
 }
 
+void ServiceWorkerRegisterJob::StartWorkerAndContinue(
+    const StatusCallback& callback,
+    ServiceWorkerStatusCode status,
+    const scoped_refptr<ServiceWorkerRegistration>& registration) {
+  if (registration->active_version()) {
+    // We have an active version, so we can complete immediately, even
+    // if the service worker isn't running.
+    callback.Run(registration, SERVICE_WORKER_OK);

[kinuko, 2014/01/29 09:56:20]

This is going to dispatch install event when it's wired?

+    return;
+  }
+
+  pending_version_ = new ServiceWorkerVersion(
+      registration, worker_registry_, registration->next_version_id());

[kinuko, 2014/01/29 09:56:20]

Oh, is a version ID going to be a registration-local?

[alecflett, 2014/01/30 01:51:13]

I don't have strong feelings about it either way.

+  LOG(ERROR) << "StartWorkerAndContinue...";
+  for (std::vector<int>::const_iterator it = pending_process_ids_.begin();
+       it != pending_process_ids_.end();
+       ++it)
+    pending_version_->AddProcessToWorker(*it);
+
+  // TODO(alecflett): We need to begin the installation process (which
+  // dispatches the install and activate events) and then attach the
+  // version and run the callback when that completes.
+  registration->set_active_version(pending_version_);

[kinuko, 2014/01/29 09:56:20]

This should be set_pending_version() until we complete Install, shouldn't it?

[alecflett, 2014/01/30 01:51:13]

in the final code yes - in the short term (until all the install event stuff
lands) I'm just skipping the pending state.

+  pending_version_->StartWorker(base::Bind(callback, registration));
+}
+
 void ServiceWorkerRegisterJob::RegisterPatternAndContinue(
     const RegistrationCallback& callback,
     ServiceWorkerStatusCode previous_status) {
@@ -97,6 +139,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, SERVICE_WORKER_OK,
@@ -109,6 +152,8 @@ void ServiceWorkerRegisterJob::UnregisterPatternAndContinue(
     ServiceWorkerStatusCode previous_status,
     const scoped_refptr<ServiceWorkerRegistration>& previous_registration) {
 
+  LOG(ERROR) << "UnregisterPatternAndContinue(callback, " << found << ", "
+             << previous_status << ", " << previous_registration << ");";
   // The previous registration may not exist, which is ok.
   if (previous_status == SERVICE_WORKER_OK && found &&
       (script_url_.is_empty() ||
@@ -118,6 +163,9 @@ void ServiceWorkerRegisterJob::UnregisterPatternAndContinue(
     // 'callback'.
     storage_->UnregisterInternal(pattern_);
     DCHECK(previous_registration->is_shutdown());
+  } else {
+    // TODO(alecflett): We have an existing registration, we should
+    // schedule an update.

[kinuko, 2014/01/29 09:56:20]

This comment seems to say more specific situation than the cases that can fall
into this 'else'

   }
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE, base::Bind(callback, previous_status));
@@ -132,10 +180,11 @@ void ServiceWorkerRegisterJob::RunCallbacks(
     it->Run(status, registration);
   }
 }
+
 void ServiceWorkerRegisterJob::RegisterComplete(
-    ServiceWorkerStatusCode status,
-    const scoped_refptr<ServiceWorkerRegistration>& registration) {
-  RunCallbacks(status, registration);
+    const scoped_refptr<ServiceWorkerRegistration>& registration,
+    ServiceWorkerStatusCode start_status) {
+  RunCallbacks(start_status, registration);
   coordinator_->FinishJob(pattern_, this);
 }