Respect content settings for Service Worker registration

content/browser/service_worker/service_worker_dispatcher_host.cc

Index: content/browser/service_worker/service_worker_dispatcher_host.cc
diff --git a/content/browser/service_worker/service_worker_dispatcher_host.cc b/content/browser/service_worker/service_worker_dispatcher_host.cc
index 29017a4ce4dbb58774f2be5bab84ea6c74389ae9..89ef4bdba3a0d95419aadf88b7f43ff30598faf0 100644
--- a/content/browser/service_worker/service_worker_dispatcher_host.cc
+++ b/content/browser/service_worker/service_worker_dispatcher_host.cc
@@ -18,6 +18,8 @@
 #include "content/browser/service_worker/service_worker_utils.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/common/service_worker/service_worker_messages.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/common/content_client.h"
 #include "ipc/ipc_message_macros.h"
 #include "third_party/WebKit/public/platform/WebServiceWorkerError.h"
 #include "url/gurl.h"
@@ -30,6 +32,7 @@ namespace {
 
 const char kShutdownErrorMessage[] =
     "The Service Worker system has shutdown.";
+const char kDisabledErrorMessage[] = "The browser has disabled Service Worker.";
 
 const uint32 kFilteredMessageClasses[] = {
   ServiceWorkerMsgStart,
@@ -42,23 +45,17 @@ const uint32 kFilteredMessageClasses[] = {
 bool CanRegisterServiceWorker(const GURL& document_url,
                               const GURL& pattern,
                               const GURL& script_url) {
-  // TODO: Respect Chrome's content settings, if we add a setting for
-  // controlling whether Service Worker is allowed.

[falken, 2014/10/02 09:51:42]

Doing the check here would kill the renderer if the content settings disabled
service worker, which isn't appropriate here.

[michaeln, 2014/10/02 20:32:51]

i remember when these methods were originally added, they were intended to be
place holders for these 'allow' browser policy checks as opposed to origin
checking for spec compliance... c'est le vie :)

CanDoThisOrThat is a very cryptic way of saying IsSameOrigin.

[falken, 2014/10/03 08:30:42]

Acknowledged. Now it's doing a bit more, checking same and secure origin. I
can't think of a good name now, so leaving it as is.

   return document_url.GetOrigin() == pattern.GetOrigin() &&
          document_url.GetOrigin() == script_url.GetOrigin();
 }
 
 bool CanUnregisterServiceWorker(const GURL& document_url,
                                 const GURL& pattern) {
-  // TODO: Respect Chrome's content settings, if we add a setting for
-  // controlling whether Service Worker is allowed.
   return document_url.GetOrigin() == pattern.GetOrigin();
 }
 
 bool CanGetRegistration(const GURL& document_url,
                         const GURL& given_document_url) {
-  // TODO: Respect Chrome's content settings, if we add a setting for
-  // controlling whether Service Worker is allowed.

[falken, 2014/10/02 09:51:42]

Even if cookies are blocked/service worker is disabled, GetRegistration probably
should return no service workers (or a service worker that was registered before
the blocking and still exists), so it doesn't check content settings below.

[michaeln, 2014/10/02 20:32:51]

That's not consistent with how cookies or other things work. When blocking is
enabled, no reading/writing (no soup for) you. Pre-existing values are not
accessible. To be consistent everywhere, when blocked is turned on, content
should see no signs of sw's anywhere.

[falken, 2014/10/03 08:30:42]

OK, makes sense. Done.

   return document_url.GetOrigin() == given_document_url.GetOrigin();
 }
 
@@ -66,11 +63,13 @@ bool CanGetRegistration(const GURL& document_url,
 
 ServiceWorkerDispatcherHost::ServiceWorkerDispatcherHost(
     int render_process_id,
-    MessagePortMessageFilter* message_port_message_filter)
+    MessagePortMessageFilter* message_port_message_filter,
+    ResourceContext* resource_context)
     : BrowserMessageFilter(kFilteredMessageClasses,
                            arraysize(kFilteredMessageClasses)),
       render_process_id_(render_process_id),
       message_port_message_filter_(message_port_message_filter),
+      resource_context_(resource_context),
       channel_ready_(false) {
 }
 
@@ -246,6 +245,17 @@ void ServiceWorkerDispatcherHost::OnRegisterServiceWorker(
     BadMessageReceived();
     return;
   }
+
+  if (!GetContentClient()->browser()->AllowServiceWorker(
+          pattern, provider_host->document_url(), resource_context_)) {
+    Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
+        thread_id,
+        request_id,
+        WebServiceWorkerError::ErrorTypeDisabled,
+        base::ASCIIToUTF16(kDisabledErrorMessage)));
+    return;
+  }
+
   TRACE_EVENT_ASYNC_BEGIN2("ServiceWorker",
                            "ServiceWorkerDispatcherHost::RegisterServiceWorker",
                            request_id,
@@ -298,6 +308,16 @@ void ServiceWorkerDispatcherHost::OnUnregisterServiceWorker(
     return;
   }
 
+  if (!GetContentClient()->browser()->AllowServiceWorker(
+          pattern, provider_host->document_url(), resource_context_)) {
+    Send(new ServiceWorkerMsg_ServiceWorkerUnregistrationError(
+        thread_id,
+        request_id,
+        WebServiceWorkerError::ErrorTypeDisabled,
+        base::ASCIIToUTF16(kDisabledErrorMessage)));
+    return;
+  }
+
   TRACE_EVENT_ASYNC_BEGIN1(
       "ServiceWorker",
       "ServiceWorkerDispatcherHost::UnregisterServiceWorker",