ServiceWorker: Support longest-prefix-match for registration scope

content/browser/service_worker/service_worker_storage.cc

Index: content/browser/service_worker/service_worker_storage.cc
diff --git a/content/browser/service_worker/service_worker_storage.cc b/content/browser/service_worker/service_worker_storage.cc
index a9a5b774ac0597ced39ad398f1b67815c82a5a26..e7950d28ad622ccda4c4542fc84a3f932f97a112 100644
--- a/content/browser/service_worker/service_worker_storage.cc
+++ b/content/browser/service_worker/service_worker_storage.cc
@@ -559,20 +559,35 @@ void ServiceWorkerStorage::DidGetRegistrationsForDocument(
   }
 
   // Find one with a pattern match.
+  ServiceWorkerDatabase::RegistrationData match;
   for (RegistrationList::const_iterator it = registrations->begin();
        it != registrations->end(); ++it) {
-    // TODO(michaeln): if there are multiple matches the one with
-    // the longest scope should win.
-    if (ServiceWorkerUtils::ScopeMatches(it->scope, document_url)) {
-      scoped_refptr<ServiceWorkerRegistration> registration =
-          context_->GetLiveRegistration(it->registration_id);
-      if (registration) {
-        callback.Run(SERVICE_WORKER_OK, registration);
-        return;
-      }
-      callback.Run(SERVICE_WORKER_OK, CreateRegistration(*it));
+    ServiceWorkerUtils::MatchResult result =

[dominicc (has gone to gerrit), 2014/05/22 05:03:31]

From an API design point of view would it make sense for the caller to provide
an iterator of URLs and have the method return the best match? Or have an object
that you can feed URLs and ask it the best match?

It looks to me like many callers will use this pattern and it is a lot of lines
of code to produce it here. I'm not sure the caller needs to know about
CompareScopePriorities, for example, that should be an implementation detail of
the method/class/whatever.

+        ServiceWorkerUtils::ScopeMatches(it->scope, document_url);
+    if (result == ServiceWorkerUtils::SCOPE_NO_MATCH)
+      continue;
+    if (result == ServiceWorkerUtils::SCOPE_EXACT_MATCH) {

[jsbell, 2014/05/21 19:20:03]

Same as previous, exact vs. longest-wildcard

[michaeln, 2014/05/22 00:56:29]

agree with jsbell length of scope trumps exact vs not

+      match = *it;
+      break;
+    }
+
+    DCHECK_EQ(ServiceWorkerUtils::SCOPE_WILDCARD_MATCH, result);
+    if (match.scope.is_empty() ||
+        ServiceWorkerUtils::CompareScopePriorities(
+            it->scope, match.scope) >= 0) {

[jsbell, 2014/05/21 19:20:03]

Same: if comparison == 0, the scopes must be the same; can we assert anything?

+      match = *it;
+    }
+  }
+
+  if (!match.scope.is_empty()) {
+    scoped_refptr<ServiceWorkerRegistration> registration =
+        context_->GetLiveRegistration(match.registration_id);
+    if (registration) {
+      callback.Run(SERVICE_WORKER_OK, registration);
       return;
     }
+    callback.Run(SERVICE_WORKER_OK, CreateRegistration(match));
+    return;
   }
 
   // Look for something currently being installed.
@@ -732,18 +747,26 @@ ServiceWorkerStorage::CreateRegistration(
 ServiceWorkerRegistration*
 ServiceWorkerStorage::FindInstallingRegistrationForDocument(
     const GURL& document_url) {
-  // TODO(michaeln): if there are multiple matches the one with
-  // the longest scope should win, and these should on equal footing
-  // with the stored registrations in FindRegistrationForDocument().
+  ServiceWorkerRegistration* match = NULL;
   for (RegistrationRefsById::const_iterator it =
            installing_registrations_.begin();
        it != installing_registrations_.end(); ++it) {
-    if (ServiceWorkerUtils::ScopeMatches(
-            it->second->pattern(), document_url)) {
-      return it->second;
+    ServiceWorkerUtils::MatchResult result =
+        ServiceWorkerUtils::ScopeMatches(it->second->pattern(), document_url);
+    if (result == ServiceWorkerUtils::SCOPE_NO_MATCH)
+      continue;
+    if (result == ServiceWorkerUtils::SCOPE_EXACT_MATCH) {
+      match = it->second;
+      break;
+    }
+
+    DCHECK_EQ(ServiceWorkerUtils::SCOPE_WILDCARD_MATCH, result);
+    if (!match || ServiceWorkerUtils::CompareScopePriorities(
+            it->second->pattern(), match->pattern()) >= 0) {
+      match = it->second;
     }
   }
-  return NULL;
+  return match;
 }
 
 ServiceWorkerRegistration*