ServiceWorker: Support longest-prefix-match for registration scope

content/browser/service_worker/service_worker_storage.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_storage.h"
 
 #include <string>
 
 #include "base/bind_helpers.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 541 matching lines @@
     ServiceWorkerDatabase::Status status) {
   DCHECK(registrations);
   if (status != ServiceWorkerDatabase::STATUS_OK) {
     // TODO(nhiroki): Handle database error (http://crbug.com/371675).
     callback.Run(SERVICE_WORKER_ERROR_FAILED,
                  scoped_refptr<ServiceWorkerRegistration>());
     return;
   }
 
   // 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));
-      return;
+    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.
   // TODO(michaeln): Should be mixed in with the stored registrations
   // for this test.
   scoped_refptr<ServiceWorkerRegistration> installing_registration =
       FindInstallingRegistrationForDocument(document_url);
   if (installing_registration) {
     callback.Run(SERVICE_WORKER_OK, installing_registration);
     return;
   }
 
@@ skipping 137 matching lines @@
     NOTREACHED();
   // TODO(michaeln): Hmmm, what if DeleteReg was invoked after
   // the Find result we're returning here? NOTREACHED condition?
 
   return registration;
 }
 
 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*
 ServiceWorkerStorage::FindInstallingRegistrationForPattern(
     const GURL& scope) {
   for (RegistrationRefsById::const_iterator it =
            installing_registrations_.begin();
        it != installing_registrations_.end(); ++it) {
     if (it->second->pattern() == scope)
       return it->second;
@@ skipping 18 matching lines @@
   // TODO(michaeln): Store data on disk and do error checking.
   disk_cache_.reset(new ServiceWorkerDiskCache);
   int rv = disk_cache_->InitWithMemBackend(
       kMaxMemDiskCacheSize,
       base::Bind(&EmptyCompletionCallback));
   DCHECK_EQ(net::OK, rv);
   return disk_cache_.get();
 }
 
 }  // namespace content