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

content/browser/service_worker/service_worker_provider_host.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_provider_host.h"
 
 #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"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/common/service_worker/service_worker_messages.h"
 
 namespace content {
 
 ServiceWorkerProviderHost::ServiceWorkerProviderHost(
     int process_id, int provider_id,
@@ skipping 87 matching lines @@
       info.process_id != process_id_) {
     // If we aren't trying to start this version in our process
     // something is amiss.
     return false;
   }
 
   hosted_version_ = live_version;
   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