ServiceWorker: Make skipWaiting only works on documents using current registration

content/browser/service_worker/service_worker_provider_host.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_provider_host.h"
 
 #include "base/stl_util.h"
 #include "content/browser/message_port_message_filter.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_request_handler.h"
@@ skipping 66 matching lines @@
 
 void ServiceWorkerProviderHost::OnRegistrationFailed(
     ServiceWorkerRegistration* registration) {
   DCHECK_EQ(associated_registration_.get(), registration);
   DisassociateRegistration();
 }
 
 void ServiceWorkerProviderHost::OnSkippedWaiting(
     ServiceWorkerRegistration* registration) {
   DCHECK_EQ(associated_registration_.get(), registration);
+  if (!controlling_version_)

[falken, 2014/12/15 02:29:25]

I think some explanation here using comments or function names would be good.
The key points are:
1. A client is using a registration if it is controlled by the active worker of
the registration (definition).
2. skippedWaiting alters a registration but does not cause a client to start
using the registration.

Maybe a helper function like ServiceWorkerProviderHost::IsUsingRegistration()
along with a comment?

[xiang, 2014/12/15 06:00:52]

I added these comments. I think a IsUsingRegistration() method maybe not proper
here as it is in a transition state, the controlling_version_ is not equal to
registration->active_version().

[falken, 2014/12/15 06:18:01]

My imagination was something like { return
associated_registration_->active_version() == controlling_version_; }

[falken, 2014/12/15 06:20:20]

Ah I totally missed the point of your comment. OK, just comment is fine.

+    return;
   ServiceWorkerVersion* active_version = registration->active_version();
   DCHECK_EQ(active_version->status(), ServiceWorkerVersion::ACTIVATING);
   SetControllerVersionAttribute(active_version);
 }
 
 void ServiceWorkerProviderHost::SetDocumentUrl(const GURL& url) {
   DCHECK(!url.has_ref());
   document_url_ = url;
 }
 
@@ skipping 248 matching lines @@
     context_->process_manager()->RemoveProcessReferenceFromPattern(
         pattern, render_process_id_);
   }
 }
 
 bool ServiceWorkerProviderHost::IsContextAlive() {
   return context_ != NULL;
 }
 
 }  // namespace content