Mostly manual refactoring of ServiceWorkerRequestHandler into a base class and two derived classes,…

content/browser/service_worker/service_worker_provider_host.cc

Index: content/browser/service_worker/service_worker_provider_host.cc
diff --git a/content/browser/service_worker/service_worker_provider_host.cc b/content/browser/service_worker/service_worker_provider_host.cc
index df789a354bedeef7ad258e3841d33cab0ae12a79..ce531535b22bc36c98ea1735e28e03740cd724d6 100644
--- a/content/browser/service_worker/service_worker_provider_host.cc
+++ b/content/browser/service_worker/service_worker_provider_host.cc
@@ -6,6 +6,8 @@
 
 #include "base/stl_util.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_context_request_handler.h"
+#include "content/browser/service_worker/service_worker_controllee_request_handler.h"
 #include "content/browser/service_worker/service_worker_dispatcher_host.h"
 #include "content/browser/service_worker/service_worker_handle.h"
 #include "content/browser/service_worker/service_worker_utils.h"
@@ -113,19 +115,21 @@ bool ServiceWorkerProviderHost::SetHostedVersionId(int64 version_id) {
   return true;
 }
 
-bool ServiceWorkerProviderHost::ShouldHandleRequest(
-    ResourceType::Type resource_type) const {
-  if (ServiceWorkerUtils::IsMainResourceType(resource_type))
-    return true;
-
-  if (active_version())
-    return true;
-
-  // TODO(kinuko): Handle ServiceWorker cases.
-  // For now we always return false here, so that we don't handle
-  // requests for ServiceWorker (either for the main or sub resources).
-
-  return false;
+scoped_ptr<ServiceWorkerRequestHandler>
+ServiceWorkerProviderHost::CreateRequestHandler(
+    ResourceType::Type resource_type) {
+  if (hosted_version_) {

[jsbell, 2014/04/24 21:22:24]

Whoah, just cluing into the fact that ServiceWorkerProviderHost hosts eithe a
document or a SW itself.

If I'm not misunderstanding that, can we (here or another CL) (1) update the
docs in the header file (which just talk about the document case), and (2) maybe
do this test via a helper e.g. isContextHost() / isControlleeHost() ?

[michaeln, 2014/04/24 21:49:17]

Good idea, I'll do both in this CL.

[michaeln, 2014/04/24 21:59:18]

Having a bit of a hard time with terminology, hows this.

// Note this class can also host a running service worker, in which
// case it will observe resource loads made directly by the service worker.
class CONTENT_EXPORT ServiceWorkerProviderHost

bool IsHostToRunningServiceWorker();

+    return scoped_ptr<ServiceWorkerRequestHandler>(
+        new ServiceWorkerContextRequestHandler(
+            context_, AsWeakPtr(), resource_type));
+  }
+  if (ServiceWorkerUtils::IsMainResourceType(resource_type) ||
+      active_version()) {
+    return scoped_ptr<ServiceWorkerRequestHandler>(
+        new ServiceWorkerControlleeRequestHandler(
+            context_, AsWeakPtr(), resource_type));
+  }
+  return scoped_ptr<ServiceWorkerRequestHandler>();
 }
 
 }  // namespace content