S13nSW: Check updateViaCache and bypass HTTP cache if needed

ShouldBypassCacheDueToUpdateViaCache() is moved to ServiceWorkerUtils
so that we can use the function in S13nSW code path.

This fixes test failures in
- http/tests/serviceworker/chromium.update-served-from-cache.html
- external/wpt/service-workers/service-worker/registration-updateviacache.https.html

Bug: 831998
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Iff73827268f37b4fa2aa5826ff153bd92b3a3c92
Reviewed-on: https://chromium-review.googlesource.com/1011527
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550556}

content/browser/service_worker/service_worker_context_request_handler.cc

@@ -12,6 +12,7 @@
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/browser/service_worker/service_worker_write_to_cache_job.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/content_switches.h"
 #include "net/base/load_flags.h"
 #include "net/log/net_log.h"
@@ -23,31 +24,6 @@
 
 namespace content {
 
-namespace {
-
-bool ShouldBypassCacheDueToUpdateViaCache(
-    bool is_main_script,
-    blink::mojom::ServiceWorkerUpdateViaCache cache_mode) {
-  // TODO(https://crbug.com/675540): Remove the command line check and always
-  // respect cache_mode when shipping updateViaCache flag to stable.
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableExperimentalWebPlatformFeatures)) {
-    return false;
-  }
-  switch (cache_mode) {
-    case blink::mojom::ServiceWorkerUpdateViaCache::kImports:
-      return is_main_script;
-    case blink::mojom::ServiceWorkerUpdateViaCache::kNone:
-      return true;
-    case blink::mojom::ServiceWorkerUpdateViaCache::kAll:
-      return false;
-  }
-  NOTREACHED() << static_cast<int>(cache_mode);
-  return false;
-}
-
-}  // namespace
-
 ServiceWorkerContextRequestHandler::ServiceWorkerContextRequestHandler(
     base::WeakPtr<ServiceWorkerContextCore> context,
     base::WeakPtr<ServiceWorkerProviderHost> provider_host,
@@ -220,8 +196,8 @@ net::URLRequestJob* ServiceWorkerContextRequestHandler::MaybeCreateJobImpl(
   base::TimeDelta time_since_last_check =
       base::Time::Now() - registration->last_update_check();
 
-  if (ShouldBypassCacheDueToUpdateViaCache(is_main_script,
-                                           registration->update_via_cache()) ||
+  if (ServiceWorkerUtils::ShouldBypassCacheDueToUpdateViaCache(
+          is_main_script, registration->update_via_cache()) ||
       time_since_last_check > kServiceWorkerScriptMaxCacheAge ||
       version_->force_bypass_cache_for_scripts()) {
     extra_load_flags = net::LOAD_BYPASS_CACHE;

content/browser/service_worker/service_worker_new_script_loader.cc

@@ -16,6 +16,7 @@
 #include "content/browser/service_worker/service_worker_write_to_cache_job.h"
 #include "content/browser/url_loader_factory_getter.h"
 #include "content/common/service_worker/service_worker_utils.h"
+#include "net/base/load_flags.h"
 #include "net/cert/cert_status_flags.h"
 #include "services/network/public/cpp/resource_response.h"
 #include "third_party/blink/public/common/mime_util/mime_util.h"
@@ -31,13 +32,14 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
     int32_t routing_id,
     int32_t request_id,
     uint32_t options,
-    const network::ResourceRequest& resource_request,
+    const network::ResourceRequest& original_request,
     network::mojom::URLLoaderClientPtr client,
     scoped_refptr<ServiceWorkerVersion> version,
     scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
-    : request_url_(resource_request.url),
-      resource_type_(static_cast<ResourceType>(resource_request.resource_type)),
+    : request_url_(original_request.url),
+      resource_type_(static_cast<ResourceType>(original_request.resource_type)),
+      resource_request_(new network::ResourceRequest(original_request)),
       version_(version),
       network_client_binding_(this),
       network_watcher_(FROM_HERE,
@@ -64,10 +66,13 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
   // |incumbent_cache_resource_id| is valid if the incumbent service worker
   // exists and it's required to do the byte-for-byte check.
   int64_t incumbent_cache_resource_id = kInvalidServiceWorkerResourceId;
-  if (resource_type_ == RESOURCE_TYPE_SERVICE_WORKER) {
   scoped_refptr<ServiceWorkerRegistration> registration =
       version_->context()->GetLiveRegistration(version_->registration_id());
+  // ServiceWorkerVersion keeps the registration alive while the service
+  // worker is starting up, and it must be starting up here.
   DCHECK(registration);
+  const bool is_main_script = resource_type_ == RESOURCE_TYPE_SERVICE_WORKER;
+  if (is_main_script) {
     ServiceWorkerVersion* stored_version = registration->waiting_version()
                                                ? registration->waiting_version()
                                                : registration->active_version();
@@ -80,6 +85,10 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
     }
   }
 
+  if (ServiceWorkerUtils::ShouldBypassCacheDueToUpdateViaCache(
+          is_main_script, registration->update_via_cache()))
+    resource_request_->load_flags |= net::LOAD_BYPASS_CACHE;
+
   // Create response readers only when we have to do the byte-for-byte check.
   std::unique_ptr<ServiceWorkerResponseReader> compare_reader;
   std::unique_ptr<ServiceWorkerResponseReader> copy_reader;
@@ -104,7 +113,7 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
   network_client_binding_.Bind(mojo::MakeRequest(&network_client));
   loader_factory_getter->GetNetworkFactory()->CreateLoaderAndStart(
       mojo::MakeRequest(&network_loader_), routing_id, request_id, options,
-      resource_request, std::move(network_client), traffic_annotation);
+      *resource_request_.get(), std::move(network_client), traffic_annotation);
 }
 
 ServiceWorkerNewScriptLoader::~ServiceWorkerNewScriptLoader() = default;

content/browser/service_worker/service_worker_new_script_loader.h

@@ -52,7 +52,7 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader
       int32_t routing_id,
       int32_t request_id,
       uint32_t options,
-      const network::ResourceRequest& resource_request,
+      const network::ResourceRequest& original_request,
       network::mojom::URLLoaderClientPtr client,
       scoped_refptr<ServiceWorkerVersion> version,
       scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
@@ -128,6 +128,8 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader
   // RESOURCE_TYPE_SCRIPT for an imported script.
   const ResourceType resource_type_;
 
+  std::unique_ptr<network::ResourceRequest> resource_request_;
+
   scoped_refptr<ServiceWorkerVersion> version_;
 
   std::unique_ptr<ServiceWorkerCacheWriter> cache_writer_;

content/common/service_worker/service_worker_utils.cc

@@ -206,6 +206,27 @@ bool ServiceWorkerUtils::ExtractSinglePartHttpRange(
   return true;
 }
 
+bool ServiceWorkerUtils::ShouldBypassCacheDueToUpdateViaCache(
+    bool is_main_script,
+    blink::mojom::ServiceWorkerUpdateViaCache cache_mode) {
+  // TODO(https://crbug.com/675540): Remove the command line check and always
+  // respect cache_mode when shipping updateViaCache flag to stable.
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableExperimentalWebPlatformFeatures)) {
+    return false;
+  }
+  switch (cache_mode) {
+    case blink::mojom::ServiceWorkerUpdateViaCache::kImports:
+      return is_main_script;
+    case blink::mojom::ServiceWorkerUpdateViaCache::kNone:
+      return true;
+    case blink::mojom::ServiceWorkerUpdateViaCache::kAll:
+      return false;
+  }
+  NOTREACHED() << static_cast<int>(cache_mode);
+  return false;
+}
+
 bool LongestScopeMatcher::MatchLongest(const GURL& scope) {
   if (!ServiceWorkerUtils::ScopeMatches(scope, url_))
     return false;

content/common/service_worker/service_worker_utils.h

@@ -71,6 +71,10 @@ class ServiceWorkerUtils {
                                          bool* has_range_out,
                                          uint64_t* offset_out,
                                          uint64_t* size_out);
+
+  static bool ShouldBypassCacheDueToUpdateViaCache(
+      bool is_main_script,
+      blink::mojom::ServiceWorkerUpdateViaCache cache_mode);
 };
 
 class CONTENT_EXPORT LongestScopeMatcher {

third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

@@ -16,7 +16,6 @@ crbug.com/807271 external/wpt/service-workers/service-worker/fetch-canvas-tainti
 Bug(none) external/wpt/service-workers/service-worker/fetch-header-visibility.https.html [ Pass Failure Crash ]
 Bug(none) external/wpt/service-workers/service-worker/fetch-request-xhr-sync-on-worker.https.html [ Timeout ]
 crbug.com/771118 external/wpt/service-workers/service-worker/mime-sniffing.https.html [ Failure ]
-Bug(none) external/wpt/service-workers/service-worker/registration-updateviacache.https.html [ Failure Timeout ]
 Bug(none) http/tests/appcache/top-frame-3.html [ Pass Timeout ]
 Bug(none) http/tests/appcache/top-frame-4.html [ Pass Timeout ]
 Bug(none) http/tests/appcache/local-content.html [ Pass Timeout ]
@@ -65,7 +64,6 @@ Bug(none) http/tests/security/mime-type-execute-as-html-01.html [ Failure ]
 Bug(none) http/tests/security/mime-type-execute-as-html-04.html [ Failure ]
 Bug(none) http/tests/security/mixedContent/filesystem-url-in-iframe.html [ Failure Timeout ]
 Bug(none) http/tests/security/offscreen-canvas-worker-read-blocked-by-setting.html [ Crash Pass Timeout ]
-Bug(none) http/tests/serviceworker/chromium.update-served-from-cache.html [ Failure ]
 Bug(none) plugins/iframe-plugin-bgcolor.html [ Timeout ]
 
 # Plugin throttle is not implemented.