Decouple script_url from ServiceWorkerRegistration

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 "base/message_loop/message_loop.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
@@ skipping 26 matching lines @@
       is_promise_resolved_(false),
       promise_resolved_status_(SERVICE_WORKER_OK),
       weak_factory_(this) {}
 
 ServiceWorkerRegisterJob::ServiceWorkerRegisterJob(
     base::WeakPtr<ServiceWorkerContextCore> context,
     ServiceWorkerRegistration* registration)
     : context_(context),
       job_type_(UPDATE_JOB),
       pattern_(registration->pattern()),
-      script_url_(registration->script_url()),

[michaeln, 2014/08/25 22:00:58]

Can this data member can remain const and be reliably set at all times?

  script_url_(registration()->GetNewestVersion()->script_url());

Othewise the ServiceWorkerRegisterJob::Equals comparison function does not work.
I think we can assert that GetNewestVersion() is not null since if there is no
newest version, there can't be anything to update.

[falken, 2014/08/26 01:02:27]

Great catch. But we must handle this scenario:

1. Queue a Register job for same-scope, new script URL
2. Queue an Update job, it sets script_url_ to current script
3. Register job makes a waiting version with new script
4. Update job will make an installing version with the old script

I think Update can just abort when it starts if its script_url_ is not the
newest version's.

       phase_(INITIAL),
       is_promise_resolved_(false),
       promise_resolved_status_(SERVICE_WORKER_OK),
       weak_factory_(this) {
+  DCHECK(registration);
   internal_.registration = registration;
 }
 
 ServiceWorkerRegisterJob::~ServiceWorkerRegisterJob() {
   DCHECK(!context_ ||
          phase_ == INITIAL || phase_ == COMPLETE || phase_ == ABORT)
       << "Jobs should only be interrupted during shutdown.";
 }
 
 void ServiceWorkerRegisterJob::AddCallback(const RegistrationCallback& callback,
@@ skipping 121 matching lines @@
 
   if (!existing_registration) {
     RegisterAndContinue(SERVICE_WORKER_OK);
     return;
   }
 
   // "Set registration.[[Uninstalling]] to false."
   existing_registration->AbortPendingClear();
 
   // "If scriptURL is equal to registration.[[ScriptURL]], then:"
-  if (existing_registration->script_url() == script_url_) {
+  if (existing_registration->GetNewestVersion()->script_url() == script_url_) {
     // Spec says to resolve with registration.[[GetNewestWorker]]. We come close
     // by resolving with the active version.
     set_registration(existing_registration);
 
     if (!existing_registration->active_version()) {
       UpdateAndContinue();
       return;
     }
 
     ResolvePromise(
         status, existing_registration, existing_registration->active_version());
     Complete(SERVICE_WORKER_OK);
     return;
   }
 
   // "Set registration.[[ScriptURL]] to scriptURL." We accomplish this by
   // deleting the existing registration and registering a new one.
   // TODO(michaeln): Deactivate the live existing_registration object and
   // eventually call storage->DeleteVersionResources() when it no longer has any
   // controllees.
   context_->storage()->DeleteRegistration(
       existing_registration->id(),
-      existing_registration->script_url().GetOrigin(),
+      existing_registration->pattern().GetOrigin(),
       base::Bind(&ServiceWorkerRegisterJob::RegisterAndContinue,
                  weak_factory_.GetWeakPtr()));
 }
 
 void ServiceWorkerRegisterJob::ContinueWithUpdate(
     ServiceWorkerStatusCode status,
     const scoped_refptr<ServiceWorkerRegistration>& existing_registration) {
   DCHECK_EQ(UPDATE_JOB, job_type_);
   if (status != SERVICE_WORKER_OK) {
     Complete(status);
     return;
   }
 
   if (existing_registration != registration()) {
     Complete(SERVICE_WORKER_ERROR_NOT_FOUND);
     return;
   }
 
+  // A previous job may have unregistered this registration.
+  if (registration()->is_uninstalling()) {
+    Complete(SERVICE_WORKER_ERROR_NOT_FOUND);
+    return;
+  }
+
+  script_url_ = registration()->GetNewestVersion()->script_url();
+
   // TODO(michaeln): If the last update check was less than 24 hours
   // ago, depending on the freshness of the cached worker script we
   // may be able to complete the update job right here.
 
   UpdateAndContinue();
 }
 
 // Creates a new ServiceWorkerRegistration.
 void ServiceWorkerRegisterJob::RegisterAndContinue(
     ServiceWorkerStatusCode status) {
   SetPhase(REGISTER);
   if (status != SERVICE_WORKER_OK) {
     // Abort this registration job.
     Complete(status);
     return;
   }
 
   set_registration(new ServiceWorkerRegistration(
-      pattern_, script_url_, context_->storage()->NewRegistrationId(),
-      context_));
+      pattern_, context_->storage()->NewRegistrationId(), context_));
   AssociateProviderHostsToRegistration(registration());
   UpdateAndContinue();
 }
 
 // This function corresponds to the spec's [[Update]] algorithm.
 void ServiceWorkerRegisterJob::UpdateAndContinue() {
   SetPhase(UPDATE);
   context_->storage()->NotifyInstallingRegistration(registration());
 
   // TODO(falken): "If serviceWorkerRegistration.installingWorker is not null.."
   // then terminate the installing worker. It doesn't make sense to implement
   // yet since we always activate the worker if install completed, so there can
   // be no installing worker at this point.
 
   // "Let serviceWorker be a newly-created ServiceWorker object..." and start
   // the worker.
-  set_new_version(new ServiceWorkerVersion(
-      registration(), context_->storage()->NewVersionId(), context_));
+  set_new_version(new ServiceWorkerVersion(registration(),
+                                           script_url_,
+                                           context_->storage()->NewVersionId(),
+                                           context_));
 
   bool pause_after_download = job_type_ == UPDATE_JOB;
   if (pause_after_download)
     new_version()->embedded_worker()->AddListener(this);
   new_version()->StartWorkerWithCandidateProcesses(
       pending_process_ids_,
       pause_after_download,
       base::Bind(&ServiceWorkerRegisterJob::OnStartWorkerFinished,
                  weak_factory_.GetWeakPtr()));
 }
@@ skipping 96 matching lines @@
     if (registration()) {
       if (new_version()) {
         registration()->UnsetVersion(new_version());
         new_version()->Doom();
       }
       if (!registration()->waiting_version() &&
           !registration()->active_version()) {
         registration()->NotifyRegistrationFailed();
         context_->storage()->DeleteRegistration(
             registration()->id(),
-            registration()->script_url().GetOrigin(),
+            registration()->pattern().GetOrigin(),
             base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
       }
     }
     if (!is_promise_resolved_)
       ResolvePromise(status, NULL, NULL);
   }
   DCHECK(callbacks_.empty());
   if (registration()) {
     context_->storage()->NotifyDoneInstallingRegistration(
         registration(), new_version(), status);
@@ skipping 77 matching lines @@
     ServiceWorkerProviderHost* host = it->GetProviderHost();
     if (ServiceWorkerUtils::ScopeMatches(registration->pattern(),
                                          host->document_url())) {
       if (host->CanAssociateRegistration(registration))
         host->AssociateRegistration(registration);
     }
   }
 }
 
 }  // namespace content