Re-land: SW Servicification: Hook ScriptURLLoader only for script installation

Add some more code from ServiceWorkerContextRequestHandler to
ScriptURLLoaderFactory.  Also added small cleanup around IsInstalled.

Re-land of:
https://chromium-review.googlesource.com/544720 and
https://chromium-review.googlesource.com/547160

Bug: 715640
Change-Id: Iae432fb2619d1a3b05ba26cbce7563b93e86339d
Reviewed-on: https://chromium-review.googlesource.com/579030
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488702}

content/browser/service_worker/service_worker_context_request_handler.cc

@@ -21,26 +21,6 @@
 
 namespace content {
 
-namespace {
-
-bool IsInstalled(const ServiceWorkerVersion* version) {
-  switch (version->status()) {
-    case ServiceWorkerVersion::NEW:
-    case ServiceWorkerVersion::INSTALLING:
-      return false;
-    case ServiceWorkerVersion::INSTALLED:
-    case ServiceWorkerVersion::ACTIVATING:
-    case ServiceWorkerVersion::ACTIVATED:
-      return true;
-    case ServiceWorkerVersion::REDUNDANT:
-      return false;
-  }
-  NOTREACHED();
-  return false;
-}
-
-}  // namespace
-
 ServiceWorkerContextRequestHandler::ServiceWorkerContextRequestHandler(
     base::WeakPtr<ServiceWorkerContextCore> context,
     base::WeakPtr<ServiceWorkerProviderHost> provider_host,
@@ -110,7 +90,8 @@ net::URLRequestJob* ServiceWorkerContextRequestHandler::MaybeCreateJob(
       MaybeCreateJobImpl(request, network_delegate, &status);
   const bool is_main_script = resource_type_ == RESOURCE_TYPE_SERVICE_WORKER;
   ServiceWorkerMetrics::RecordContextRequestHandlerStatus(
-      status, IsInstalled(version_.get()), is_main_script);
+      status, ServiceWorkerVersion::IsInstalled(version_->status()),
+      is_main_script);
   if (job)
     return job;
 
@@ -171,7 +152,7 @@ net::URLRequestJob* ServiceWorkerContextRequestHandler::MaybeCreateJobImpl(
   int resource_id =
       version_->script_cache_map()->LookupResourceId(request->url());
   if (resource_id != kInvalidServiceWorkerResourceId) {
-    if (IsInstalled(version_.get())) {
+    if (ServiceWorkerVersion::IsInstalled(version_->status())) {
       // An installed worker is loading a stored script.
       if (is_main_script)
         version_->embedded_worker()->OnURLJobCreatedForMainScript();
@@ -187,7 +168,7 @@ net::URLRequestJob* ServiceWorkerContextRequestHandler::MaybeCreateJobImpl(
   }
 
   // An installed worker is importing a non-stored script.
-  if (IsInstalled(version_.get())) {
+  if (ServiceWorkerVersion::IsInstalled(version_->status())) {
     DCHECK(!is_main_script);
     *out_status = CreateJobStatus::ERROR_UNINSTALLED_SCRIPT_IMPORT;
     return nullptr;

content/browser/service_worker/service_worker_provider_host.cc

@@ -98,13 +98,51 @@ void RemoveProviderHost(base::WeakPtr<ServiceWorkerContextCore> context,
   context->RemoveProviderHost(process_id, provider_id);
 }
 
-// Used by a Service Worker for script loading (for all script loading for now,
-// but to be used only during installation once script streaming lands).
-// For now this is just a proxy loader for the network loader.
+// Wraps associated request for another associated request.
+// TODO(kinuko): This should be able to go away once URLLoader becomes
+// independent.
+class AssociatedURLLoaderRelay final : public mojom::URLLoader {
+ public:
+  static void CreateLoaderAndStart(
+      mojom::URLLoaderFactory* factory,
+      mojom::URLLoaderAssociatedRequest request,
+      int routing_id,
+      int request_id,
+      uint32_t options,
+      const ResourceRequest& resource_request,
+      mojom::URLLoaderClientPtr client,
+      const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) {
+    mojom::URLLoaderAssociatedPtr associated_ptr;
+    mojom::URLLoaderAssociatedRequest associated_request =
+        mojo::MakeRequest(&associated_ptr);
+    factory->CreateLoaderAndStart(std::move(associated_request), routing_id,
+                                  request_id, options, resource_request,
+                                  std::move(client), traffic_annotation);
+    mojo::MakeStrongAssociatedBinding(
+        base::MakeUnique<AssociatedURLLoaderRelay>(std::move(associated_ptr)),
+        std::move(request));
+  }
+
+  explicit AssociatedURLLoaderRelay(
+      mojom::URLLoaderAssociatedPtr associated_ptr)
+      : associated_ptr_(std::move(associated_ptr)) {}
+  ~AssociatedURLLoaderRelay() override {}
+  void FollowRedirect() override { associated_ptr_->FollowRedirect(); }
+  void SetPriority(net::RequestPriority priority,
+                   int intra_priority_value) override {
+    associated_ptr_->SetPriority(priority, intra_priority_value);
+  }
+
+ private:
+  mojom::URLLoaderAssociatedPtr associated_ptr_;
+  DISALLOW_COPY_AND_ASSIGN(AssociatedURLLoaderRelay);
+};
+
+// Used by a Service Worker for script loading only during the installation
+// time. For now this is just a proxy loader for the network loader.
 // Eventually this should replace the existing URLRequestJob-based request
 // interception for script loading, namely ServiceWorkerWriteToCacheJob.
-// TODO(kinuko): Implement this. Hook up the existing code in
-// ServiceWorkerContextRequestHandler.
+// TODO(kinuko): Implement this.
 class ScriptURLLoader : public mojom::URLLoader, public mojom::URLLoaderClient {
  public:
   ScriptURLLoader(
@@ -224,6 +262,18 @@ class ScriptURLLoaderFactory : public mojom::URLLoaderFactory {
                             mojom::URLLoaderClientPtr client,
                             const net::MutableNetworkTrafficAnnotationTag&
                                 traffic_annotation) override {
+    if (!ShouldHandleScriptRequest(resource_request)) {
+      // If the request should not be handled by ScriptURLLoader, just
+      // fallback to the network. This needs a relaying as we use different
+      // associated message pipes.
+      // TODO(kinuko): Record the reason like what we do with netlog in
+      // ServiceWorkerContextRequestHandler.
+      AssociatedURLLoaderRelay::CreateLoaderAndStart(
+          loader_factory_getter_->GetNetworkFactory()->get(),
+          std::move(request), routing_id, request_id, options, resource_request,
+          std::move(client), traffic_annotation);
+      return;
+    }
     mojo::MakeStrongAssociatedBinding(
         base::MakeUnique<ScriptURLLoader>(
             routing_id, request_id, options, resource_request,
@@ -240,6 +290,53 @@ class ScriptURLLoaderFactory : public mojom::URLLoaderFactory {
   }
 
  private:
+  bool ShouldHandleScriptRequest(const ResourceRequest& resource_request) {
+    if (!context_ || !provider_host_)
+      return false;
+
+    // We only use the script cache for main script loading and
+    // importScripts(), even if a cached script is xhr'd, we don't
+    // retrieve it from the script cache.
+    if (resource_request.resource_type != RESOURCE_TYPE_SERVICE_WORKER &&
+        resource_request.resource_type != RESOURCE_TYPE_SCRIPT) {
+      // TODO: Record bad message, we shouldn't come here for other
+      // request types.
+      return false;
+    }
+
+    scoped_refptr<ServiceWorkerVersion> version =
+        provider_host_->running_hosted_version();
+
+    // This could happen if browser-side has set the status to redundant but
+    // the worker has not yet stopped. The worker is already doomed so just
+    // reject the request. Handle it specially here because otherwise it'd be
+    // unclear whether "REDUNDANT" should count as installed or not installed
+    // when making decisions about how to handle the request and logging UMA.
+    if (!version || version->status() == ServiceWorkerVersion::REDUNDANT)
+      return false;
+
+    // TODO: Make sure we don't handle the redirected request.
+
+    // For installed worker we fallback to the network for now.
+    if (ServiceWorkerVersion::IsInstalled(version->status())) {
+      DCHECK(ServiceWorkerUtils::IsScriptStreamingEnabled());
+      return false;
+    }
+
+    // TODO: Make sure we come here only for new / unknown scripts
+    // once script streaming manager in the renderer side stops sending
+    // resource requests for the known script URLs, i.e. add DCHECK for
+    // version->script_cache_map()->LookupResourceId(url) ==
+    // kInvalidServiceWorkerResourceId.
+    //
+    // Currently this could be false for the installing worker that imports
+    // the same script twice (e.g. importScripts('dupe.js');
+    // importScripts('dupe.js');).
+
+    // Request should be served by ScriptURLLoader.
+    return true;
+  }
+
   base::WeakPtr<ServiceWorkerContextCore> context_;
   base::WeakPtr<ServiceWorkerProviderHost> provider_host_;
   base::WeakPtr<storage::BlobStorageContext> blob_storage_context_;

content/browser/service_worker/service_worker_version.cc

@@ -136,21 +136,6 @@ void ClearTick(base::TimeTicks* time) {
   *time = base::TimeTicks();
 }
 
-bool IsInstalled(ServiceWorkerVersion::Status status) {
-  switch (status) {
-    case ServiceWorkerVersion::NEW:
-    case ServiceWorkerVersion::INSTALLING:
-    case ServiceWorkerVersion::REDUNDANT:
-      return false;
-    case ServiceWorkerVersion::INSTALLED:
-    case ServiceWorkerVersion::ACTIVATING:
-    case ServiceWorkerVersion::ACTIVATED:
-      return true;
-  }
-  NOTREACHED() << "Unexpected status: " << status;
-  return false;
-}
-
 std::string VersionStatusToString(ServiceWorkerVersion::Status status) {
   switch (status) {
     case ServiceWorkerVersion::NEW:
@@ -1066,6 +1051,21 @@ void ServiceWorkerVersion::CountFeature(uint32_t feature) {
     provider_host_by_uuid.second->CountFeature(feature);
 }
 
+bool ServiceWorkerVersion::IsInstalled(ServiceWorkerVersion::Status status) {
+  switch (status) {
+    case ServiceWorkerVersion::NEW:
+    case ServiceWorkerVersion::INSTALLING:
+    case ServiceWorkerVersion::REDUNDANT:
+      return false;
+    case ServiceWorkerVersion::INSTALLED:
+    case ServiceWorkerVersion::ACTIVATING:
+    case ServiceWorkerVersion::ACTIVATED:
+      return true;
+  }
+  NOTREACHED() << "Unexpected status: " << status;
+  return false;
+}
+
 void ServiceWorkerVersion::OnOpenNewTab(int request_id, const GURL& url) {
   OnOpenWindow(request_id, url, WindowOpenDisposition::NEW_FOREGROUND_TAB);
 }

content/browser/service_worker/service_worker_version.h

@@ -406,6 +406,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
   }
   const std::set<uint32_t>& used_features() const { return used_features_; }
 
+  static bool IsInstalled(ServiceWorkerVersion::Status status);
+
  private:
   friend class base::RefCounted<ServiceWorkerVersion>;
   friend class ServiceWorkerReadFromCacheJobTest;

content/renderer/service_worker/service_worker_context_client.cc

@@ -76,6 +76,8 @@
 #include "third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextClient.h"
 #include "third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h"
 
+using blink::WebURLRequest;
+
 namespace content {
 
 namespace {
@@ -94,7 +96,7 @@ class WebServiceWorkerNetworkProviderImpl
 
   // Blink calls this method for each request starting with the main script,
   // we tag them with the provider id.
-  void WillSendRequest(blink::WebURLRequest& request) override {
+  void WillSendRequest(WebURLRequest& request) override {
     std::unique_ptr<RequestExtraData> extra_data(new RequestExtraData);
     extra_data->set_service_worker_provider_id(provider_->provider_id());
     extra_data->set_originated_from_service_worker(true);
@@ -109,7 +111,8 @@ class WebServiceWorkerNetworkProviderImpl
       base::SingleThreadTaskRunner* task_runner) override {
     RenderThreadImpl* child_thread = RenderThreadImpl::current();
     if (child_thread && provider_->script_loader_factory() &&
-        ServiceWorkerUtils::IsServicificationEnabled()) {
+        ServiceWorkerUtils::IsServicificationEnabled() &&
+        IsScriptRequest(request)) {
       return base::MakeUnique<WebURLLoaderImpl>(
           child_thread->resource_dispatcher(), task_runner,
           provider_->script_loader_factory());
@@ -120,6 +123,13 @@ class WebServiceWorkerNetworkProviderImpl
   int GetProviderID() const override { return provider_->provider_id(); }
 
  private:
+  static bool IsScriptRequest(const WebURLRequest& request) {
+    auto request_context = request.GetRequestContext();
+    return request_context == WebURLRequest::kRequestContextServiceWorker ||
+           request_context == WebURLRequest::kRequestContextScript ||
+           request_context == WebURLRequest::kRequestContextImport;
+  }
+
   std::unique_ptr<ServiceWorkerNetworkProvider> provider_;
 };
 
@@ -151,31 +161,28 @@ ServiceWorkerStatusCode EventResultToStatus(
   return SERVICE_WORKER_ERROR_FAILED;
 }
 
-blink::WebURLRequest::FetchRequestMode GetBlinkFetchRequestMode(
+WebURLRequest::FetchRequestMode GetBlinkFetchRequestMode(
     FetchRequestMode mode) {
-  return static_cast<blink::WebURLRequest::FetchRequestMode>(mode);
+  return static_cast<WebURLRequest::FetchRequestMode>(mode);
 }
 
-blink::WebURLRequest::FetchCredentialsMode GetBlinkFetchCredentialsMode(
+WebURLRequest::FetchCredentialsMode GetBlinkFetchCredentialsMode(
     FetchCredentialsMode credentials_mode) {
-  return static_cast<blink::WebURLRequest::FetchCredentialsMode>(
-      credentials_mode);
+  return static_cast<WebURLRequest::FetchCredentialsMode>(credentials_mode);
 }
 
-blink::WebURLRequest::FetchRedirectMode GetBlinkFetchRedirectMode(
+WebURLRequest::FetchRedirectMode GetBlinkFetchRedirectMode(
     FetchRedirectMode redirect_mode) {
-  return static_cast<blink::WebURLRequest::FetchRedirectMode>(redirect_mode);
+  return static_cast<WebURLRequest::FetchRedirectMode>(redirect_mode);
 }
 
-blink::WebURLRequest::RequestContext GetBlinkRequestContext(
+WebURLRequest::RequestContext GetBlinkRequestContext(
     RequestContextType request_context_type) {
-  return static_cast<blink::WebURLRequest::RequestContext>(
-      request_context_type);
+  return static_cast<WebURLRequest::RequestContext>(request_context_type);
 }
 
-blink::WebURLRequest::FrameType GetBlinkFrameType(
-    RequestContextFrameType frame_type) {
-  return static_cast<blink::WebURLRequest::FrameType>(frame_type);
+WebURLRequest::FrameType GetBlinkFrameType(RequestContextFrameType frame_type) {
+  return static_cast<WebURLRequest::FrameType>(frame_type);
 }
 
 blink::WebServiceWorkerClientInfo

testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

@@ -39,13 +39,6 @@
 # http://crbug.com/721398
 -ClearSiteDataThrottleBrowserTest.*
 
-# http://crbug.com/736362
--ServiceWorkerVersionBrowserTest.Activate_Rejected
--ServiceWorkerVersionBrowserTest.FetchEvent_Response
--ServiceWorkerVersionBrowserTest.Activate_NoEventListener
--ServiceWorkerVersionBrowserTest.FetchEvent_ResponseViaCache
--ServiceWorkerVersionBrowserTest.FetchEvent_respondWithRejection
-
 -AsyncResourceHandlerBrowserTest/AsyncResourceHandlerBrowserTest.UploadProgress*
 -BrowserSideNavigationBrowserTest.FailedNavigation
 -CrossSiteResourceHandlerTest.NoDeliveryToDetachedFrame