ServiceWorker: add ServiceWorkerClients.claim() support (2/3).

content/browser/service_worker/service_worker_registration.cc

Index: content/browser/service_worker/service_worker_registration.cc
diff --git a/content/browser/service_worker/service_worker_registration.cc b/content/browser/service_worker/service_worker_registration.cc
index a4fac5c65873a9c9afe96c3b0e8b6fa92fe493b0..4301c85151ad223b8d17e43bfdad8ea3329b54ab 100644
--- a/content/browser/service_worker/service_worker_registration.cc
+++ b/content/browser/service_worker/service_worker_registration.cc
@@ -161,6 +161,40 @@ void ServiceWorkerRegistration::ActivateWaitingVersionWhenReady() {
     ActivateWaitingVersion();
 }
 
+void ServiceWorkerRegistration::ClaimClients() {
+  DCHECK(context_);
+  DCHECK(active_version());
+  std::vector<ServiceWorkerRegistrationInfo> registration_infos =
+      context_->GetAllLiveRegistrationInfo();
+  for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
+           context_->GetProviderHostIterator();
+       !it->IsAtEnd(); it->Advance()) {
+    ServiceWorkerProviderHost* host = it->GetProviderHost();
+    if (host->document_url().GetOrigin() != pattern_.GetOrigin())
+      continue;
+
+    LongestScopeMatcher matcher(host->document_url());
+    int64 match = kInvalidServiceWorkerRegistrationId;
+    for (std::vector<ServiceWorkerRegistrationInfo>::iterator it =

[michaeln, 2015/01/28 00:55:13]

This inner loop is not so nice. I think we can avoid looping over all
registrations. We only care about hosts for which we're a match, so that test
can go first, and then we only need to know if we're better than its current
match. Its easier to explain like this...

// first rejigger associations
foreach host
  if shouldclaim(host)
    if (host->reg != this) {
      host->DisassociateReg()
      host->AssociateReg(this);
    }

// notify associated hosts about being claimed by a new version
FOR_EACH_OBSERVER(Listener, listeners_,
                  OnClaimedByNewVersion(this));
return;

bool shouldclaim(host) {
   if (host->asssociated_reg == this)
     return true;  // it's one of ours and the new version should take over
   LongestScopeMatcher matcher(host->document_url());
   if (!matcher.Match(this->pattern))
     return false; // no point looking any further
   // see if we're a better match than its current association
   if (!host->associated_reg)
     return true;  // we're definitely better than nothing
   if (matcher.Match(host->associated_reg()->pattern))
     return false; // its current association wins
   return true; // hooray, we win
}

[xiang, 2015/01/28 05:38:35]

Yes, move out the inner loop is more cleaner.

However the from the spec: "Let registration be the result of running Match
Service Worker Registration algorithm passing client's creation url as the
argument". The "better match than its current association" way will make some
cases different.

For example: foobar.html is using "/foo" registration, now "/foob" & "/foobar"
registrations both registered and activated, then "/foob" triggered .claim().
From spec, the foobar.html should stilling using "/foo" instead of start using
"/foob". I feel this should be the right behavior even it maybe not a practical
scenario, wdyt?

[falken, 2015/01/28 08:59:49]

I think you're right, "/foob" shouldn't claim the page. For correctness, I'd opt
to keep the original algorithm, although it may be slow.

Actually I'm wondering now, could there be registrations in storage but not
existing as "live" registrations in memory that could possibly be the longest
match?

[michaeln, 2015/01/28 21:11:14]

Thats too bad because it adds code complexity and will take longer to compute.
Yes, i think you'll have to use FindRegistrationForDocument(), that's an async
process so its definitely more complicated.

Also notice the SetAllowAssociation() method and how it's used by
ServiceWorkerControlleeRequestHandler to coordinate with
ServiceWorkerRegisterJob about who has the rights to determine the association
(the request handler always trumps). Now that there's a third agency that make
this decision (claim method handler), we have to figure out the priority of this
new agency?

Idk, we might want to suggest the spec change to make Claim a purely synch
decision making process (by just comparing the current associations to the new
claimee)? That would make it easier for vendors to impl and to actually match
one another in behavior.

If we could ensure that FindRegForDocument() completely syncly for a particular
origin when any association was active for that ori, that might help.

[xiang, 2015/01/29 02:58:37]

I thought about FindRegistrationForDocument() when wrote these code, maybe a
comment should be added to describe the trick. AFAIK there should no better
match for a document in storage than in living registrations. The living
registrations contain both stored registrations found in document navigation and
newly registered ones. So we don't need to go back to DB, is this correct?

Thanks for mention the SetAllowAssociation(), I should check
CanAssociateRegistration() before association when claim happens.

I think claim() will not influence current ServiceWorkerControlleeRequestHandler
decision on whether an association should be happened or not, as claim() only
effects when worker is "active" which means the corresponding registration data
must already update to DB. If FindForDocumentInDB() send before a claim
registration WriteRegistrationInDB() the later claim() will work; if
FindForDocumentInDB() send after a claim registration WriteRegistrationInDB()
the registration will be picked.

[falken, 2015/01/29 03:28:50]

I suspected this might be the case too. Michael, what do you think? Definitely a
comment should be added about the "trick".

+             registration_infos.begin();
+         it != registration_infos.end(); ++it) {
+      ServiceWorkerRegistration* registration =
+          context_->GetLiveRegistration(it->registration_id);
+      DCHECK(registration);
+      if (registration->is_uninstalled())
+        continue;
+      if (matcher.MatchLongest(it->pattern))
+        match = it->registration_id;
+    }
+    if (match != registration_id_)
+      continue;
+    if (host->controlling_version() == active_version())
+      continue;
+
+    host->SetController(this);
+  }
+}
+
 void ServiceWorkerRegistration::ClearWhenReady() {
   DCHECK(context_);
   if (is_uninstalling_)