Check if there is a SW controlling the start page for app banner.

content/browser/service_worker/service_worker_context_wrapper.cc

Index: content/browser/service_worker/service_worker_context_wrapper.cc
diff --git a/content/browser/service_worker/service_worker_context_wrapper.cc b/content/browser/service_worker/service_worker_context_wrapper.cc
index db0b4e9fffe6c94a3dcb2da3194f3e4629a666a8..802cf90893469ddf653f107946ed6d766a74a70f 100644
--- a/content/browser/service_worker/service_worker_context_wrapper.cc
+++ b/content/browser/service_worker/service_worker_context_wrapper.cc
@@ -21,11 +21,13 @@
 #include "content/browser/service_worker/service_worker_process_manager.h"
 #include "content/browser/service_worker/service_worker_quota_client.h"
 #include "content/browser/service_worker/service_worker_request_handler.h"
+#include "content/browser/service_worker/service_worker_utils.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/service_worker_context.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_util.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "storage/browser/blob/blob_storage_context.h"
 #include "storage/browser/quota/quota_manager_proxy.h"
@@ -278,6 +280,24 @@ void ServiceWorkerContextWrapper::DidGetAllRegistrationsForGetAllOrigins(
   callback.Run(usage_infos);
 }
 
+void ServiceWorkerContextWrapper::
+    DidFindRegistrationForCheckHasSameServiceWorker(
+        const GURL& other_url,
+        const CheckHasSameServiceWorkerCallback& callback,
+        ServiceWorkerStatusCode status,
+        const scoped_refptr<ServiceWorkerRegistration>& registration) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  bool has_same = status == SERVICE_WORKER_OK;
+  if (has_same) {
+    DCHECK(registration);
+    has_same =
+        ServiceWorkerUtils::ScopeMatches(registration->pattern(), other_url);

[falken, 2015/02/05 10:26:11]

There might be a bug here depending on the use case. If you have registration A
for "/foo" and registration B for "/foo/bar", then URL "/foo/x" will fall under
registration A and "foo/bar/x" will fall under B, yet
CheckHasSameServiceWorker("/foo", "/foo/bar") will return true.

Another bug depending on use case is the registration might not have an active
worker, if it's still installing, or the waiting version is in the process of
being promoted to active. You might want to also check if registration.active()
is non-null?

[benwells, 2015/02/05 12:38:29]

That's interesting, I didn't realise that could be a thing. After talking to
Alex Russel about the desired behavior, turns out this is actually how we want
it. This makes it simple and keeps the behavior the same regardless of what
other pages have been visited or the order pages are visted in.
OK, done.

+  }
+
+  callback.Run(has_same);

[falken, 2015/02/05 10:26:11]

subjective style nit: the use of has_same is a bit tricky. Early return on
status != OK is a bit more idiomatic in SW code:

if (status != OK) {
  callback.Run(false);
  return;
}
DCHECK(registration);
callback.Run(ServiceWorkerUtils::ScopeMatches(...));

[benwells, 2015/02/05 12:38:29]

Done.

+}
+
 namespace {
 void StatusCodeToBoolCallbackAdapter(
     const ServiceWorkerContext::ResultCallback& callback,
@@ -309,6 +329,31 @@ void ServiceWorkerContextWrapper::DeleteForOrigin(const GURL& origin_url) {
   DeleteForOrigin(origin_url, base::Bind(&EmptySuccessCallback));
 }
 
+void ServiceWorkerContextWrapper::CheckHasSameServiceWorker(
+    const GURL& url,
+    const GURL& other_url,
+    const CheckHasSameServiceWorkerCallback& callback) {

[no sievers, 2015/02/04 19:46:05]

This is a bit odd: It can be called from any thread, but the callback is invoked
from the IO thread.

[benwells, 2015/02/05 02:09:25]

Fair enough. I've outlined some options below, it would be awesome if you could
indicate what you think is the best option. If none of these are good feel free
to give me another.

Looking at the other functions on this class (e.g. RegisterSericeWorker), it can
be called on any thread but the callback can is invoked from the UI thread. That
has similar weirdness but at least is consistent with the other functions on the
interface and would make the calling code simpler.

Having it so that the callback goes to the same thread as the caller would
probably be more consistent with the way it is called (i.e. from any thread),
but that feels overly complicated.

Another alternative is to make the function only callable from the IO thread,
but I think that would mean making the base class (ServiceWorkerContext) ref
counted which I wanted to avoid.

[falken, 2015/02/05 10:26:11]

I chatted a bit offline with Ben, I think it's OK to keep the current code
(perhaps making callback on UI thread if it makes sense for the caller) to be
consistent with the existing functions. The code also predates this chromium-dev
thread
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/uWGNk-hVlGc...
and kinuko@ mentions on https://codereview.chromium.org/796393005/ the class is
intended to be thread-safe so it's OK to keep the pattern. 

[benwells, 2015/02/05 12:38:29]

I've changed it to reply on the UI thread.

[kinuko, 2015/02/05 14:24:24]

Yep, we ended up with the current code after a few revisions but then we found
that it's become a bit messy as you write (i.e. allows to be called on any
thread but actually assumes it comes from UI thread).
For now I think this feels more consistent, thanks. (We can fix these at once
later)

+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&ServiceWorkerContextWrapper::CheckHasSameServiceWorker,
+                   this, url, other_url, callback));
+    return;
+  }
+  if (!context_core_.get()) {
+    LOG(ERROR) << "ServiceWorkerContextCore is no longer alive.";
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(callback, false));
+    return;
+  }
+  GURL stripped_url = net::SimplifyUrlForRequest(url);
+  context()->storage()->FindRegistrationForDocument(
+      stripped_url,
+      base::Bind(&ServiceWorkerContextWrapper::
+                     DidFindRegistrationForCheckHasSameServiceWorker,
+                 this, other_url, callback));
+}
+
 void ServiceWorkerContextWrapper::AddObserver(
     ServiceWorkerContextObserver* observer) {
   observer_list_->AddObserver(observer);