service worker: Teach Blink more about no fetch handler controllers.

When the network service or NetS13nSW is enabled, requests
skip service worker loaders entirely when the controller has
no fetch event handler. This was broke assumptions in
DocumentThreadableLoader, since it expected service worker
machinery to return "fallback for CORS" responses for
certain requests when there was a controller.

Bug: 850839
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I32b058caacb14cba2e53dd8667fe9fc930a776e8
Reviewed-on: https://chromium-review.googlesource.com/1107435Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568796}

content/child/blink_platform_impl.cc

@@ -36,6 +36,7 @@
 #include "content/app/resources/grit/content_resources.h"
 #include "content/app/strings/grit/content_strings.h"
 #include "content/child/child_thread_impl.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
@@ -680,6 +681,10 @@ bool BlinkPlatformImpl::AllowScriptExtensionForServiceWorker(
   return GetContentClient()->AllowScriptExtensionForServiceWorker(scriptUrl);
 }
 
+bool BlinkPlatformImpl::IsServiceWorkerNetServicificationEnabled() {
+  return ServiceWorkerUtils::IsServicificationEnabled();
+}
+
 blink::WebCrypto* BlinkPlatformImpl::Crypto() {
   return &web_crypto_;
 }

content/child/blink_platform_impl.h

@@ -97,6 +97,7 @@ class CONTENT_EXPORT BlinkPlatformImpl : public blink::Platform {
       const blink::WebSize& cumulative_scroll) override;
   bool AllowScriptExtensionForServiceWorker(
       const blink::WebURL& script_url) override;
+  bool IsServiceWorkerNetServicificationEnabled() override;
   blink::WebCrypto* Crypto() override;
   const char* GetBrowserServiceName() const override;
   blink::WebMediaCapabilitiesClient* MediaCapabilitiesClient() override;

content/renderer/service_worker/worker_fetch_context_impl.cc

@@ -83,6 +83,10 @@ class WorkerFetchContextImpl::Factory : public blink::WebURLLoaderFactory {
 
   void SetServiceWorkerURLLoaderFactory(
       network::mojom::URLLoaderFactoryPtr service_worker_loader_factory) {
+    if (!service_worker_loader_factory) {
+      service_worker_loader_factory_ = nullptr;
+      return;
+    }
     service_worker_loader_factory_ =
         base::MakeRefCounted<network::WrapperSharedURLLoaderFactory>(
             std::move(service_worker_loader_factory));
@@ -368,8 +372,8 @@ void WorkerFetchContextImpl::ResetServiceWorkerURLLoaderFactory() {
   DCHECK(ServiceWorkerUtils::IsServicificationEnabled());
   if (!web_loader_factory_)
     return;
-  if (IsControlledByServiceWorker() ==
-      blink::mojom::ControllerServiceWorkerMode::kNoController) {
+  if (IsControlledByServiceWorker() !=
+      blink::mojom::ControllerServiceWorkerMode::kControlled) {
     web_loader_factory_->SetServiceWorkerURLLoaderFactory(nullptr);
     return;
   }

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

@@ -6,7 +6,6 @@ Bug(none) external/wpt/clear-site-data/navigation.https.html [ Timeout ]
 Bug(none) external/wpt/html/browsers/offline/appcache/workers/appcache-worker.html [ Timeout ]
 crbug.com/829417 external/wpt/html/browsers/offline/appcache/workers/appcache-worker.https.html [ Timeout ]
 crbug.com/771118 external/wpt/service-workers/service-worker/mime-sniffing.https.html [ Failure ]
-crbug.com/850839 external/wpt/service-workers/service-worker/controller-with-no-fetch-event-handler.https.html [ Failure Crash ]
 
 # Passes on NetworkService and fails on non-NetworkService because
 # NetworkService isn't affected by https://crbug.com/595993.

third_party/WebKit/LayoutTests/TestExpectations

@@ -1754,7 +1754,7 @@ crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/workers/cross
 
 # Failing tests in dictionary order.
 crbug.com/802835 virtual/outofblink-cors/external/wpt/fetch/corb/img-mime-types-coverage.tentative.sub.html [ Failure ]
-crbug.com/850839 virtual/outofblink-cors/external/wpt/service-workers/service-worker/controller-with-no-fetch-event-handler.https.html [ Failure ]
+crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/controller-with-no-fetch-event-handler.https.html [ Failure ]
 crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-canvas-tainting-image-cache.https.html [ Failure ]
 crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-canvas-tainting-image.https.html [ Failure ]
 crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-canvas-tainting-video-cache.https.html [ Failure ]
@@ -2279,8 +2279,6 @@ crbug.com/524160 [ Debug ] http/tests/media/media-source/stream_memory_tests/med
 crbug.com/630342 virtual/mse-1mb-buffers/http/tests/media/media-source/stream_memory_tests/mediasource-appendbuffer-quota-exceeded-default-buffers.html [ Skip ]
 crbug.com/630342 http/tests/media/media-source/stream_memory_tests/mediasource-appendbuffer-quota-exceeded-1mb-buffers.html [ Skip ]
 
-crbug.com/850839 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/controller-with-no-fetch-event-handler.https.html [ Failure Crash ]
-
 # On these platforms (all but Android) media tests don't currently use gpu-accelerated (proprietary) codecs, so no
 # benefit to running them again with gpu acceleration enabled.
 crbug.com/555703 [ Linux Mac Win Fuchsia ] virtual/media-gpu-accelerated [ Skip ]

third_party/blink/public/platform/platform.h

@@ -626,6 +626,12 @@ class BLINK_PLATFORM_EXPORT Platform {
   virtual bool AllowScriptExtensionForServiceWorker(const WebURL& script_url) {
     return false;
   }
+  // Whether the new service worker glue for NetworkService is enabled (i.e.,
+  // the NetworkService or ServiceWorkerServicification feature is enabled).
+  // TODO(falken): Make ServiceWorkerServicification a base::Feature inside
+  // the blink namespace, then this function can move out of platform.h and be
+  // implemented directly in blink.
+  virtual bool IsServiceWorkerNetServicificationEnabled() { return false; }
 
   // WebCrypto ----------------------------------------------------------
 

third_party/blink/renderer/core/loader/document_threadable_loader.cc

@@ -337,6 +337,33 @@ void DocumentThreadableLoader::Start(const ResourceRequest& request) {
   if (should_bypass_service_worker)
     new_request.SetSkipServiceWorker(true);
 
+  // In S13nServiceWorker, if the controller service worker has no fetch event
+  // handler, it's skipped entirely, so we should treat that case the same as
+  // having no controller. In non-S13nServiceWorker, we can't do that since we
+  // don't know which service worker will handle the request since it's
+  // determined on the browser process and skipWaiting() can happen in the
+  // meantime.
+  //
+  // TODO(crbug.com/715640): When non-S13nServiceWorker is removed,
+  // is_controlled_by_service_worker is the same as
+  // ControllerServiceWorkerMode::kControlled, so this code can be simplified.
+  bool is_controlled_by_service_worker = false;
+  switch (
+      loading_context_->GetResourceFetcher()->IsControlledByServiceWorker()) {
+    case blink::mojom::ControllerServiceWorkerMode::kControlled:
+      is_controlled_by_service_worker = true;
+      break;
+    case blink::mojom::ControllerServiceWorkerMode::kNoFetchEventHandler:
+      if (Platform::Current()->IsServiceWorkerNetServicificationEnabled())
+        is_controlled_by_service_worker = false;
+      else
+        is_controlled_by_service_worker = true;
+      break;
+    case blink::mojom::ControllerServiceWorkerMode::kNoController:
+      is_controlled_by_service_worker = false;
+      break;
+  }
+
   // Process the CORS protocol inside the DocumentThreadableLoader for the
   // following cases:
   //
@@ -360,8 +387,7 @@ void DocumentThreadableLoader::Start(const ResourceRequest& request) {
   if (!async_ || new_request.GetSkipServiceWorker() ||
       !SchemeRegistry::ShouldTreatURLSchemeAsAllowingServiceWorkers(
           new_request.Url().Protocol()) ||
-      loading_context_->GetResourceFetcher()->IsControlledByServiceWorker() ==
-          blink::mojom::ControllerServiceWorkerMode::kNoController) {
+      !is_controlled_by_service_worker) {
     DispatchInitialRequest(new_request);
     return;
   }