ServiceWorker: Add support for .skipWaiting and controllerchange event(2/3)

content/browser/service_worker/service_worker_registration.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_registration.h"
 
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_info.h"
 #include "content/browser/service_worker/service_worker_register_job.h"
 #include "content/browser/service_worker/service_worker_utils.h"
@@ skipping 136 matching lines @@
   } else if (active_version_.get() == version) {
     active_version_->RemoveListener(this);
     active_version_ = NULL;
     mask->add(ChangedVersionAttributesMask::ACTIVE_VERSION);
   }
 }
 
 void ServiceWorkerRegistration::ActivateWaitingVersionWhenReady() {
   DCHECK(waiting_version());
   should_activate_when_ready_ = true;
-  if (!active_version() || !active_version()->HasControllee())
+  if (!active_version() || !active_version()->HasControllee() ||
+      waiting_version()->skip_waiting())
     ActivateWaitingVersion();
 }
 
 void ServiceWorkerRegistration::ClearWhenReady() {
   DCHECK(context_);
   if (is_uninstalling_)
     return;
   is_uninstalling_ = true;
 
   context_->storage()->NotifyUninstallingRegistration(this);
@@ skipping 23 matching lines @@
   context_->storage()->StoreRegistration(
       this,
       most_recent_version.get(),
       base::Bind(&ServiceWorkerRegistration::OnRestoreFinished,
                  this,
                  callback,
                  most_recent_version));
 }
 
 void ServiceWorkerRegistration::OnNoControllees(ServiceWorkerVersion* version) {
-  DCHECK_EQ(active_version(), version);

[falken, 2014/12/01 02:29:59]

I guess this DCHECK can fail now if OnNoControllees gets called by the worker
that was replaced by a skip waiting worker? It would seem in that case we
shouldn't proceed with the rest of the function. Should we instead early-return
if version != active_version()?

[xiang, 2014/12/01 07:20:07]

Thanks for point this out, I think we can remove the listener in
SetVersionInternal(), how do you think?

[falken, 2014/12/01 07:27:52]

Looks right, nice idea.

   if (is_uninstalling_)
     Clear();
   else if (should_activate_when_ready_)
     ActivateWaitingVersion();
   is_uninstalling_ = false;
   should_activate_when_ready_ = false;
 }
 
 void ServiceWorkerRegistration::ActivateWaitingVersion() {
   DCHECK(context_);
   DCHECK(waiting_version());
   DCHECK(should_activate_when_ready_);
   should_activate_when_ready_ = false;
   scoped_refptr<ServiceWorkerVersion> activating_version = waiting_version();
   scoped_refptr<ServiceWorkerVersion> exiting_version = active_version();
 
   if (activating_version->is_doomed() ||
       activating_version->status() == ServiceWorkerVersion::REDUNDANT) {
     return;  // Activation is no longer relevant.
   }
 
-  // "4. If exitingWorker is not null,
+  // "5. If exitingWorker is not null,
   if (exiting_version.get()) {
-    DCHECK(!exiting_version->HasControllee());
     // TODO(michaeln): should wait for events to be complete
     // "1. Wait for exitingWorker to finish handling any in-progress requests."
     // "2. Terminate exitingWorker."
     exiting_version->StopWorker(
         base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
     // "3. Run the [[UpdateState]] algorithm passing exitingWorker and
     // "redundant" as the arguments."
     exiting_version->SetStatus(ServiceWorkerVersion::REDUNDANT);
   }
 
-  // "5. Set serviceWorkerRegistration.activeWorker to activatingWorker."
-  // "6. Set serviceWorkerRegistration.waitingWorker to null."
+  // "6. Set serviceWorkerRegistration.activeWorker to activatingWorker."
+  // "7. Set serviceWorkerRegistration.waitingWorker to null."
   SetActiveVersion(activating_version.get());
 
-  // "7. Run the [[UpdateState]] algorithm passing registration.activeWorker and
+  // "8. Run the [[UpdateState]] algorithm passing registration.activeWorker and
   // "activating" as arguments."
   activating_version->SetStatus(ServiceWorkerVersion::ACTIVATING);
+  // "9. Fire a simple event named controllerchange..."
+  // Notify associated provider hosts to change the controller.
+  if (activating_version->skip_waiting())
+    FOR_EACH_OBSERVER(Listener, listeners_, OnSkippedWaiting(this));
 
-  // TODO(nhiroki): "8. Fire a simple event named controllerchange..."
-
-  // "9. Queue a task to fire an event named activate..."
+  // "10. Queue a task to fire an event named activate..."
   activating_version->DispatchActivateEvent(
       base::Bind(&ServiceWorkerRegistration::OnActivateEventFinished,
                  this, activating_version));
 }
 
 void ServiceWorkerRegistration::OnActivateEventFinished(
     ServiceWorkerVersion* activating_version,
     ServiceWorkerStatusCode status) {
   if (!context_ || activating_version != active_version())
     return;
@@ skipping 71 matching lines @@
   if (!context_) {
     callback.Run(SERVICE_WORKER_ERROR_ABORT);
     return;
   }
   context_->storage()->NotifyDoneInstallingRegistration(
       this, version.get(), status);
   callback.Run(status);
 }
 
 }  // namespace content