Skip downloading by using a flag in ResourceResponse

When NetworkService/ServiceWorkerServicification is on, MimeSniffingThrottle is
used for responses from service workers. Previously, downloading is skipped when
any of throttles handle the response, so using MimeSniffingThrottle undesirably
prevented downloading. This CL is to make downloading work properly when a
service worker serves the response.

Also, when a response is NOT served from ResourceDispatcherHost,
DownloadManagerImpl needs to be created in NavigationRequest. Currently it
happens only when NetworkService is on. This CL adds a condition where S13nSW is
on and a service worker serves the response there.

TBR=jam@chromium.org

Bug: 896696
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I222d8148cf65d15492393ebcd6ed582deb14022f
Reviewed-on: https://chromium-review.googlesource.com/c/1290434
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601901}

chrome/browser/plugins/plugin_response_interceptor_url_loader_throttle.cc

@@ -82,6 +82,7 @@ void PluginResponseInterceptorURLLoaderThrottle::WillProcessResponse(
   transferrable_loader->url_loader = original_loader.PassInterface();
   transferrable_loader->url_loader_client = std::move(original_client);
   transferrable_loader->head = std::move(deep_copied_response->head);
+  transferrable_loader->head.intercepted_by_plugin = true;
 
   bool embedded = resource_type_ != content::RESOURCE_TYPE_MAIN_FRAME;
   base::PostTaskWithTraits(

content/browser/download/download_browsertest.cc

@@ -944,6 +944,12 @@ class DownloadContentTest : public ContentBrowserTest {
     return inject_error_callback_;
   }
 
+  void RegisterServiceWorker(Shell* shell, const std::string& worker_url) {
+    NavigateToURL(
+        shell, embedded_test_server()->GetURL("/register_service_worker.html"));
+    EXPECT_EQ("DONE", EvalJs(shell, "register('" + worker_url + "')"));
+  }
+
  private:
   // Location of the downloads directory for these tests
   base::ScopedTempDir downloads_directory_;
@@ -1185,6 +1191,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, MultiDownload) {
 #if BUILDFLAG(ENABLE_PLUGINS)
 // Content served with a MIME type of application/octet-stream should be
 // downloaded even when a plugin can be found that handles the file type.
+// See https://crbug.com/104331 for the details.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadOctetStream) {
   const char kTestPluginName[] = "TestPlugin";
   const char kTestMimeType[] = "application/x-test-mime-type";
@@ -1199,9 +1206,90 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadOctetStream) {
 
   // The following is served with a Content-Type of application/octet-stream.
   NavigateToURLAndWaitForDownload(
-      shell(), embedded_test_server()->GetURL("/download/download-test.lib"),
+      shell(), embedded_test_server()->GetURL("/download/octet-stream.abc"),
+      download::DownloadItem::COMPLETE);
+}
+
+// Content served with a MIME type of application/octet-stream should be
+// downloaded even when a plugin can be found that handles the file type.
+// See https://crbug.com/104331 for the details.
+// In this test, the url is in scope of a service worker but the response is
+// served from network.
+// This is regression test for https://crbug.com/896696.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest,
+                       DownloadOctetStream_PassThroughServiceWorker) {
+  const char kTestPluginName[] = "TestPlugin";
+  const char kTestMimeType[] = "application/x-test-mime-type";
+  const char kTestFileType[] = "abc";
+
+  RegisterServiceWorker(shell(), "/fetch_event_passthrough.js");
+
+  WebPluginInfo plugin_info;
+  plugin_info.name = base::ASCIIToUTF16(kTestPluginName);
+  plugin_info.mime_types.push_back(
+      WebPluginMimeType(kTestMimeType, kTestFileType, ""));
+  plugin_info.type = WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS;
+  PluginServiceImpl::GetInstance()->RegisterInternalPlugin(plugin_info, false);
+
+  // The following is served with a Content-Type of application/octet-stream.
+  NavigateToURLAndWaitForDownload(
+      shell(), embedded_test_server()->GetURL("/download/octet-stream.abc"),
       download::DownloadItem::COMPLETE);
 }
+
+// Content served with a MIME type of application/octet-stream should be
+// downloaded even when a plugin can be found that handles the file type.
+// See https://crbug.com/104331 for the details.
+// In this test, the response will be served from a service worker.
+// This is regression test for https://crbug.com/896696.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest,
+                       DownloadOctetStream_OctetStreamServiceWorker) {
+  const char kTestPluginName[] = "TestPlugin";
+  const char kTestMimeType[] = "application/x-test-mime-type";
+  const char kTestFileType[] = "abc";
+
+  RegisterServiceWorker(shell(), "/fetch_event_octet_stream.js");
+
+  WebPluginInfo plugin_info;
+  plugin_info.name = base::ASCIIToUTF16(kTestPluginName);
+  plugin_info.mime_types.push_back(
+      WebPluginMimeType(kTestMimeType, kTestFileType, ""));
+  plugin_info.type = WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS;
+  PluginServiceImpl::GetInstance()->RegisterInternalPlugin(plugin_info, false);
+
+  // The following is served with a Content-Type of application/octet-stream.
+  NavigateToURLAndWaitForDownload(
+      shell(), embedded_test_server()->GetURL("/download/octet-stream.abc"),
+      download::DownloadItem::COMPLETE);
+}
+
+// Content served with a MIME type of application/octet-stream should be
+// downloaded even when a plugin can be found that handles the file type.
+// See https://crbug.com/104331 for the details.
+// In this test, the url is in scope of a service worker and the response is
+// served from the network via service worker.
+// This is regression test for https://crbug.com/896696.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest,
+                       DownloadOctetStream_RespondWithFetchServiceWorker) {
+  const char kTestPluginName[] = "TestPlugin";
+  const char kTestMimeType[] = "application/x-test-mime-type";
+  const char kTestFileType[] = "abc";
+
+  RegisterServiceWorker(shell(), "/fetch_event_respond_with_fetch.js");
+
+  WebPluginInfo plugin_info;
+  plugin_info.name = base::ASCIIToUTF16(kTestPluginName);
+  plugin_info.mime_types.push_back(
+      WebPluginMimeType(kTestMimeType, kTestFileType, ""));
+  plugin_info.type = WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS;
+  PluginServiceImpl::GetInstance()->RegisterInternalPlugin(plugin_info, false);
+
+  // The following is served with a Content-Type of application/octet-stream.
+  NavigateToURLAndWaitForDownload(
+      shell(), embedded_test_server()->GetURL("/download/octet-stream.abc"),
+      download::DownloadItem::COMPLETE);
+}
+
 #endif
 
 // Try to cancel just before we release the download file, by delaying final

content/browser/frame_host/navigation_request.cc

@@ -69,6 +69,7 @@
 #include "services/network/public/cpp/resource_response.h"
 #include "services/network/public/cpp/url_loader_completion_status.h"
 #include "third_party/blink/public/common/frame/sandbox_flags.h"
+#include "third_party/blink/public/common/service_worker/service_worker_utils.h"
 #include "third_party/blink/public/platform/modules/fetch/fetch_api_request.mojom.h"
 #include "third_party/blink/public/platform/resource_request_blocked_reason.h"
 #include "third_party/blink/public/platform/web_mixed_content_context_type.h"
@@ -1479,10 +1480,20 @@ void NavigationRequest::OnWillProcessResponseChecksComplete(
   // If the NavigationThrottles allowed the navigation to continue, have the
   // processing of the response resume in the network stack.
   if (result.action() == NavigationThrottle::PROCEED) {
-    // If this is a download, intercept the navigation response and pass it to
-    // DownloadManager, and cancel the navigation.
-    if (is_download_ &&
-        base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    // NetworkService doesn't use ResourceDispatcherHost.
+    bool served_via_resource_dispatcher_host =
+        !base::FeatureList::IsEnabled(network::features::kNetworkService);
+    // When S13nServiceWorker is on, it doesn't use ResourceDispatcherHost when
+    // a service worker serves the response.
+    served_via_resource_dispatcher_host =
+        served_via_resource_dispatcher_host &&
+        !(blink::ServiceWorkerUtils::IsServicificationEnabled() &&
+          response_->head.was_fetched_via_service_worker);
+
+    // NetworkService or S13nServiceWorker: If this is a download, intercept the
+    // navigation response and pass it to DownloadManager, and cancel the
+    // navigation.
+    if (is_download_ && !served_via_resource_dispatcher_host) {
       // TODO(arthursonzogni): Pass the real ResourceRequest. For the moment
       // only these 4 parameters will be used, but it may evolve quickly.
       auto resource_request = std::make_unique<network::ResourceRequest>();

content/browser/loader/navigation_url_loader_impl.cc

@@ -1093,12 +1093,8 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
 
     network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints;
 
-    // Currently only plugin handlers may intercept the response. Don't treat
-    // the response as download if it has been handled by plugins.
-    bool response_intercepted = false;
     if (url_loader_) {
       url_loader_client_endpoints = url_loader_->Unbind();
-      response_intercepted = url_loader_->response_intercepted();
     } else {
       url_loader_client_endpoints =
           network::mojom::URLLoaderClientEndpoints::New(
@@ -1126,19 +1122,21 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
           url_, head.headers.get(), head.mime_type);
       bool known_mime_type = blink::IsSupportedMimeType(head.mime_type);
 
-      bool is_download_if_not_handled_by_plugin =
-          !response_intercepted && (must_download || !known_mime_type);
-
 #if BUILDFLAG(ENABLE_PLUGINS)
-      if (!response_intercepted && !must_download && !known_mime_type) {
+      if (!head.intercepted_by_plugin && !must_download && !known_mime_type) {
+        // No plugin throttles intercepted the response. Ask if the plugin
+        // registered to PluginService wants to handle the request.
         CheckPluginAndContinueOnReceiveResponse(
             head, std::move(url_loader_client_endpoints),
-            is_download_if_not_handled_by_plugin, std::vector<WebPluginInfo>());
+            true /* is_download_if_not_handled_by_plugin */,
+            std::vector<WebPluginInfo>());
         return;
       }
 #endif
 
-      is_download = is_download_if_not_handled_by_plugin;
+      // When a plugin intercepted the response, we don't want to download it.
+      is_download =
+          !head.intercepted_by_plugin && (must_download || !known_mime_type);
       is_stream = false;
 
       // If NetworkService is on, or an interceptor handled the request, the
@@ -1163,7 +1161,7 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
     if (url_request) {
       ResourceRequestInfoImpl* info =
           ResourceRequestInfoImpl::ForRequest(url_request);
-      is_download = !response_intercepted && info->IsDownload();
+      is_download = !head.intercepted_by_plugin && info->IsDownload();
       is_stream = info->is_stream();
       if (rdh->delegate()) {
         NavigationData* navigation_data =

content/test/data/fetch_event_octet_stream.js

+// Copyright 2018 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.
+
+self.addEventListener('fetch', e => {
+  const headers = new Headers();
+  const filename = 'octet-stream.abc';
+  headers.append('Content-Type', 'application/octet-stream; charset=UTF-8');
+  headers.append('Content-Disposition',
+                 'attachment; filename="' + filename +
+                 '"; filename*=UTF-8\'\'' + filename);
+  e.respondWith(new Response('This is a binary', {headers}));
+});

content/test/data/fetch_event_passthrough.js

+// Copyright 2018 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.
+
+self.addEventListener('fetch', e => {});

content/test/data/fetch_event_respond_with_fetch.js

+// Copyright 2018 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.
+
+self.addEventListener('fetch', e => { e.respondWith(fetch(e.request)) });

content/test/data/register_service_worker.html

+<script>
+async function register(worker_url, scope) {
+  try {
+    await navigator.serviceWorker.register(worker_url);
+    await navigator.serviceWorker.ready;
+    return 'DONE';
+  } catch (error) {
+    return `${error}`;
+  }
+}
+</script>

services/network/public/cpp/resource_response.cc

@@ -63,6 +63,7 @@ scoped_refptr<ResourceResponse> ResourceResponse::DeepCopy() const {
   new_response->head.did_mime_sniff = head.did_mime_sniff;
   new_response->head.is_signed_exchange_inner_response =
       head.is_signed_exchange_inner_response;
+  new_response->head.intercepted_by_plugin = head.intercepted_by_plugin;
   return new_response;
 }
 

services/network/public/cpp/resource_response_info.h

@@ -192,6 +192,9 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceResponseInfo {
   // True if the response is an inner response of a signed exchange.
   bool is_signed_exchange_inner_response = false;
 
+  // True if the response was intercepted by a plugin.
+  bool intercepted_by_plugin = false;
+
   // NOTE: When adding or changing fields here, also update
   // ResourceResponse::DeepCopy in resource_response.cc.
 };