Make custom protocol handlers work with service workers' fetch event

When a service worker intercepts a page that is later registered as a
handler for a custom scheme via navigator.registerProtocolHandler, and
if one clicks a link with this custom scheme, then the service worker
is only able to intercept the page after reload. This CL fixes that
bug by allowing the service worker to intercept the page after
registering the custom scheme.

A new browser unit test is added to verify this fix. This also makes
the following manual WPT tests pass (*):
  protocol-handler-fragment-manual.https.html
  protocol-handler-path-manual.https.html
  protocol-handler-query-manual.https.html

(*) https://w3c-test.org/html/webappapis/system-state-and-capabilities/the-navigator-object/

Bug: 522370
Change-Id: I95eda9025885838665e9735ac9f47a3d65d89aa8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379672
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818416}

chrome/browser/chrome_content_browser_client.cc

@@ -1807,6 +1807,18 @@ bool ChromeContentBrowserClient::IsHandledURL(const GURL& url) {
   return ProfileIOData::IsHandledURL(url);
 }
 
+bool ChromeContentBrowserClient::HasCustomSchemeHandler(
+    content::BrowserContext* browser_context,
+    const std::string& scheme) {
+  if (ProtocolHandlerRegistry* protocol_handler_registry =
+          ProtocolHandlerRegistryFactory::GetForBrowserContext(
+              browser_context)) {
+    return protocol_handler_registry->IsHandledProtocol(scheme);
+  }
+
+  return false;
+}
+
 bool ChromeContentBrowserClient::CanCommitURL(
     content::RenderProcessHost* process_host,
     const GURL& url) {

chrome/browser/chrome_content_browser_client.h

@@ -185,6 +185,8 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   bool LogWebUIUrl(const GURL& web_ui_url) override;
   bool IsWebUIAllowedToMakeNetworkRequests(const url::Origin& origin) override;
   bool IsHandledURL(const GURL& url) override;
+  bool HasCustomSchemeHandler(content::BrowserContext* browser_context,
+                              const std::string& scheme) override;
   bool CanCommitURL(content::RenderProcessHost* process_host,
                     const GURL& url) override;
   void OverrideNavigationParams(

chrome/browser/custom_handlers/protocol_handler_registry_browsertest.cc

@@ -17,7 +17,6 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/permissions/permission_request_manager.h"
-#include "components/permissions/test/mock_permission_prompt_factory.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
@@ -32,6 +31,30 @@
 
 using content::WebContents;
 
+namespace {
+
+class ProtocolHandlerChangeWaiter : public ProtocolHandlerRegistry::Observer {
+ public:
+  explicit ProtocolHandlerChangeWaiter(ProtocolHandlerRegistry* registry) {
+    registry_observer_.Add(registry);
+  }
+  ProtocolHandlerChangeWaiter(const ProtocolHandlerChangeWaiter&) = delete;
+  ProtocolHandlerChangeWaiter& operator=(const ProtocolHandlerChangeWaiter&) =
+      delete;
+  ~ProtocolHandlerChangeWaiter() override = default;
+
+  void Wait() { run_loop_.Run(); }
+  // ProtocolHandlerRegistry::Observer:
+  void OnProtocolHandlerRegistryChanged() override { run_loop_.Quit(); }
+
+ private:
+  ScopedObserver<ProtocolHandlerRegistry, ProtocolHandlerRegistry::Observer>
+      registry_observer_{this};
+  base::RunLoop run_loop_;
+};
+
+}  // namespace
+
 class RegisterProtocolHandlerBrowserTest : public InProcessBrowserTest {
  public:
   RegisterProtocolHandlerBrowserTest() { }
@@ -154,12 +177,9 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest, Basic) {
 #if defined(OS_MAC)
   ASSERT_TRUE(test::RegisterAppWithLaunchServices());
 #endif
-  permissions::PermissionRequestManager* manager =
   permissions::PermissionRequestManager::FromWebContents(
-          browser()->tab_strip_model()->GetActiveWebContents());
-  auto prompt_factory =
-      std::make_unique<permissions::MockPermissionPromptFactory>(manager);
-  prompt_factory->set_response_type(
+      browser()->tab_strip_model()->GetActiveWebContents())
+      ->set_auto_response_for_test(
           permissions::PermissionRequestManager::ACCEPT_ALL);
 
   const extensions::Extension* extension =
@@ -170,16 +190,87 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest, Basic) {
       "chrome-extension://" + extension->id() + "/test.html";
 
   // Register the handler.
+  {
+    ProtocolHandlerRegistry* registry =
+        ProtocolHandlerRegistryFactory::GetForBrowserContext(
+            browser()->profile());
+    ProtocolHandlerChangeWaiter waiter(registry);
     ui_test_utils::NavigateToURL(browser(), GURL(handler_url));
     ASSERT_TRUE(content::ExecuteScript(
         browser()->tab_strip_model()->GetActiveWebContents(),
         "navigator.registerProtocolHandler('geo', 'test.html?%s', 'test');"));
-
-  // Wait until the prompt is "displayed" and "accepted".
-  base::RunLoop().RunUntilIdle();
+    waiter.Wait();
+  }
 
   // Test the handler.
   ui_test_utils::NavigateToURL(browser(), GURL("geo:test"));
   ASSERT_EQ(GURL(handler_url + "?geo%3Atest"),
             browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
 }
+
+class RegisterProtocolHandlerAndServiceWorkerInterceptor
+    : public InProcessBrowserTest {
+ public:
+  void SetUpOnMainThread() override {
+    ASSERT_TRUE(embedded_test_server()->Start());
+
+    // Navigate to the test page.
+    ui_test_utils::NavigateToURL(
+        browser(), embedded_test_server()->GetURL(
+                       "/protocol_handler/service_workers/"
+                       "test_protocol_handler_and_service_workers.html"));
+
+    // Bypass permission dialogs for registering new protocol handlers.
+    permissions::PermissionRequestManager::FromWebContents(
+        browser()->tab_strip_model()->GetActiveWebContents())
+        ->set_auto_response_for_test(
+            permissions::PermissionRequestManager::ACCEPT_ALL);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerAndServiceWorkerInterceptor,
+                       DoNotRegisterFetchListener) {
+  WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  {
+    // Register a HTML handler with a user gesture.
+    ProtocolHandlerRegistry* registry =
+        ProtocolHandlerRegistryFactory::GetForBrowserContext(
+            browser()->profile());
+    ProtocolHandlerChangeWaiter waiter(registry);
+    ASSERT_TRUE(content::ExecJs(web_contents, "registerHTMLHandler();"));
+    waiter.Wait();
+  }
+
+  // Verify that no interception by a fetch listener happens.
+  EXPECT_EQ(false,
+            content::EvalJs(web_contents,
+                            "pageWithCustomSchemeHandledByServiceWorker();"));
+}
+
+IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerAndServiceWorkerInterceptor,
+                       RegisterFetchListenerForHTMLHandler) {
+  WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Register a service worker intercepting requests to the HTML handler.
+  EXPECT_EQ(true, content::EvalJs(web_contents,
+                                  "registerFetchListenerForHTMLHandler();"));
+
+  {
+    // Register a HTML handler with a user gesture.
+    ProtocolHandlerRegistry* registry =
+        ProtocolHandlerRegistryFactory::GetForBrowserContext(
+            browser()->profile());
+    ProtocolHandlerChangeWaiter waiter(registry);
+    ASSERT_TRUE(content::ExecJs(web_contents, "registerHTMLHandler();"));
+    waiter.Wait();
+  }
+
+  // Verify that a page with the registered scheme is managed by the service
+  // worker, not the HTML handler.
+  EXPECT_EQ(true,
+            content::EvalJs(web_contents,
+                            "pageWithCustomSchemeHandledByServiceWorker();"));
+}

chrome/test/data/protocol_handler/service_workers/fetch_listener_for_html_handler.js

+// Copyright 2020 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', function(event) {
+  if (!event.request.url.includes('handler.html'))
+    return;
+
+  event.respondWith(new Response(
+      `<script>
+window.opener.postMessage({handled_by_service_worker: true}, '*');</script>`,
+      {headers: {'Content-Type': 'text/html'}}));
+});

chrome/test/data/protocol_handler/service_workers/handler.html

+<script>
+  window.opener.postMessage({handled_by_service_worker: false}, '*');
+</script>

chrome/test/data/protocol_handler/service_workers/test_protocol_handler_and_service_workers.html

+<script src="test_protocol_handler_and_service_workers.js"></script>

chrome/test/data/protocol_handler/service_workers/test_protocol_handler_and_service_workers.js

+// Copyright 2020 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.
+
+// browser_tests
+// --gtest_filter=ProtocolHandlerBrowserTest.*
+
+async function registerFetchListenerForHTMLHandler() {
+  await navigator.serviceWorker.register(
+      '/protocol_handler/service_workers/fetch_listener_for_html_handler.js');
+  await navigator.serviceWorker.ready;
+  return true;
+}
+
+function absoluteURL(path) {
+  return `${location.origin}/protocol_handler/service_workers/${path}`;
+}
+
+function registerHTMLHandler() {
+  navigator.registerProtocolHandler(
+      'web+html', absoluteURL('handler.html?url=%s'), 'title');
+}
+
+async function handledByServiceWorker(url) {
+  const a = document.body.appendChild(document.createElement('a'));
+  a.href = url;
+  a.target = '_blank';
+  let handled_by_service_worker;
+  await new Promise(resolve => {
+    window.addEventListener('message', function(event) {
+      handled_by_service_worker = event.data.handled_by_service_worker;
+      event.source.close();
+      resolve();
+    }, {once: true})
+    a.click();
+  });
+  return handled_by_service_worker;
+}
+
+function pageWithCustomSchemeHandledByServiceWorker() {
+  return handledByServiceWorker('web+html:path');
+}

content/browser/service_worker/service_worker_main_resource_loader_interceptor.cc

@@ -19,6 +19,7 @@
 #include "content/browser/service_worker/service_worker_main_resource_handle.h"
 #include "content/browser/service_worker/service_worker_main_resource_handle_core.h"
 #include "content/public/browser/browser_task_traits.h"
+#include "content/public/common/content_client.h"
 #include "content/public/common/origin_util.h"
 #include "content/public/common/url_constants.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
@@ -174,7 +175,17 @@ void MaybeCreateLoaderOnCoreThread(
                      std::move(fallback_callback)));
 }
 
-bool SchemeMaySupportRedirectingToHTTPS(const GURL& url) {
+bool SchemeMaySupportRedirectingToHTTPS(BrowserContext* browser_context,
+                                        const GURL& url) {
+  // If there is a registered protocol handler for this scheme, the embedder is
+  // expected to redirect `url` to a registered URL in a URLLoaderThrottle, and
+  // the interceptor will operate on the registered URL. Note that the HTML
+  // specification requires that the registered URL is HTTPS.
+  // https://html.spec.whatwg.org/multipage/system-state.html#normalize-protocol-handler-parameters
+  if (GetContentClient()->browser()->HasCustomSchemeHandler(browser_context,
+                                                            url.scheme()))
+    return true;
+
 #if defined(OS_CHROMEOS)
   return url.SchemeIs(kExternalFileScheme);
 #else   // OS_CHROMEOS
@@ -202,7 +213,8 @@ ServiceWorkerMainResourceLoaderInterceptor::CreateForNavigation(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (!ShouldCreateForNavigation(
-          url, request_info.begin_params->request_destination)) {
+          url, request_info.begin_params->request_destination,
+          navigation_handle->context_wrapper()->browser_context())) {
     return nullptr;
   }
 
@@ -377,7 +389,8 @@ ServiceWorkerMainResourceLoaderInterceptor::
 // static
 bool ServiceWorkerMainResourceLoaderInterceptor::ShouldCreateForNavigation(
     const GURL& url,
-    network::mojom::RequestDestination request_destination) {
+    network::mojom::RequestDestination request_destination,
+    BrowserContext* browser_context) {
   // <embed> and <object> navigations must bypass the service worker, per the
   // discussion in https://w3c.github.io/ServiceWorker/#implementer-concerns.
   if (request_destination == network::mojom::RequestDestination::kEmbed ||
@@ -385,10 +398,10 @@ bool ServiceWorkerMainResourceLoaderInterceptor::ShouldCreateForNavigation(
     return false;
   }
 
-  // Create the handler even for insecure HTTP since it's used in the
+  // Create the interceptor even for insecure HTTP since it's used in the
   // case of redirect to HTTPS.
   return url.SchemeIsHTTPOrHTTPS() || OriginCanAccessServiceWorkers(url) ||
-         SchemeMaySupportRedirectingToHTTPS(url);
+         SchemeMaySupportRedirectingToHTTPS(browser_context, url);
 }
 
 void ServiceWorkerMainResourceLoaderInterceptor::RequestHandlerWrapper(

content/browser/service_worker/service_worker_main_resource_loader_interceptor.h

@@ -96,7 +96,8 @@ class CONTENT_EXPORT ServiceWorkerMainResourceLoaderInterceptor final
   // created for a navigation to |url|.
   static bool ShouldCreateForNavigation(
       const GURL& url,
-      network::mojom::RequestDestination request_destination);
+      network::mojom::RequestDestination request_destination,
+      BrowserContext* browser_context);
 
   // Given as a callback to NavigationURLLoaderImpl.
   void RequestHandlerWrapper(

content/browser/service_worker/service_worker_main_resource_loader_interceptor_unittest.cc

@@ -4,6 +4,8 @@
 
 #include "content/browser/service_worker/service_worker_main_resource_loader_interceptor.h"
 
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
@@ -11,12 +13,28 @@ namespace content {
 
 class ServiceWorkerMainResourceLoaderInterceptorTest : public testing::Test {
  public:
+  ServiceWorkerMainResourceLoaderInterceptorTest() = default;
+  ServiceWorkerMainResourceLoaderInterceptorTest(
+      const ServiceWorkerMainResourceLoaderInterceptorTest&) = delete;
+  ServiceWorkerMainResourceLoaderInterceptorTest& operator=(
+      const ServiceWorkerMainResourceLoaderInterceptorTest&) = delete;
+  ~ServiceWorkerMainResourceLoaderInterceptorTest() override = default;
+
+  void SetUp() override {
+    browser_context_ = std::make_unique<TestBrowserContext>();
+  }
+
   bool ShouldCreateForNavigation(
       const GURL& url,
       network::mojom::RequestDestination request_destination) {
     return ServiceWorkerMainResourceLoaderInterceptor::
-        ShouldCreateForNavigation(url, request_destination);
+        ShouldCreateForNavigation(url, request_destination,
+                                  browser_context_.get());
   }
+
+ private:
+  BrowserTaskEnvironment task_environment_{BrowserTaskEnvironment::IO_MAINLOOP};
+  std::unique_ptr<TestBrowserContext> browser_context_;
 };
 
 TEST_F(ServiceWorkerMainResourceLoaderInterceptorTest,

content/public/browser/content_browser_client.cc

@@ -200,6 +200,12 @@ bool ContentBrowserClient::IsHandledURL(const GURL& url) {
   return false;
 }
 
+bool ContentBrowserClient::HasCustomSchemeHandler(
+    content::BrowserContext* browser_context,
+    const std::string& scheme) {
+  return false;
+}
+
 bool ContentBrowserClient::CanCommitURL(RenderProcessHost* process_host,
                                         const GURL& site_url) {
   return true;

content/public/browser/content_browser_client.h

@@ -463,6 +463,13 @@ class CONTENT_EXPORT ContentBrowserClient {
   // protocol handlers.
   virtual bool IsHandledURL(const GURL& url);
 
+  // Returns whether a custom handler is registered for the scheme of the
+  // specified URL scheme.
+  // https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers
+  // TODO(crbug.com/1139176) Move custom protocol handler code to content.
+  virtual bool HasCustomSchemeHandler(content::BrowserContext* browser_context,
+                                      const std::string& scheme);
+
   // Returns whether the given process is allowed to commit |url|.  This is a
   // more conservative check than IsSuitableHost, since it is used after a
   // navigation has committed to ensure that the process did not exceed its