[Sheriff] Revert "Make custom protocol handlers work with service workers' fetch event"

This reverts commit c007dfe1.

Reason for revert: RegisterProtocolHandlerAndServiceWorkerInterceptor.DoNotRegisterFetchListener is very flaky on mac (and possibly elsewhere).

Sample failure:
[ RUN      ] RegisterProtocolHandlerAndServiceWorkerInterceptor.DoNotRegisterFetchListener
[23479:30231:1019/055809.421386:WARNING:notification_platform_bridge_mac.mm(374)] AlertNotificationService: XPC connection invalidated.
[23479:775:1019/055809.917320:ERROR:device_event_log_impl.cc(211)] [05:58:09.906] FIDO: touch_id_context.mm:127 Touch ID authenticator unavailable because keychain-access-group entitlement is missing or incorrect
[23479:96771:1019/055810.044487:WARNING:embedded_test_server.cc(667)] Request not handled. Returning 404: /favicon.ico
[23479:775:1019/055810.218450:INFO:CONSOLE(0)] "Failed to launch 'web+html:path' because the scheme does not have a registered handler.", source:  (0)
../../content/public/test/browser_test_base.cc:702: Failure
Failed
RunLoop::Run() timed out.
Stack trace:
0   browser_tests                       0x0000000110d3e9c2 _ZN4base8internal7InvokerINS0_9BindStateIZNS_4test20ScopedRunLoopTimeoutC1ERKNS_8LocationENS_9TimeDeltaENS_17RepeatingCallbackIFNSt3__112basic_stringIcNSA_11char_traitsIcEENSA_9allocatorIcEEEEvEEEE3$_0JS5_SI_EEEFvvEE3RunEPNS0_13BindStateBaseE + 290
1   browser_tests                       0x0000000110c5916f base::(anonymous namespace)::OnRunLoopTimeout(base::RunLoop*, base::OnceCallback<void ()>) + 31
2   browser_tests                       0x0000000110c5a1a3 base::internal::Invoker<base::internal::BindState<void (*)(base::RunLoop*, base::OnceCallback<void ()>), base::internal::UnretainedWrapper<base::RunLoop>, base::RepeatingCallback<void ()> >, void ()>::RunOnce(base::internal::BindStateBase*) + 67
3   browser_tests                       0x000000010cf23a83 void base::internal::CancelableCallbackImpl<base::OnceCallback<void ()> >::ForwardOnce<>() + 35
4   browser_tests                       0x000000010cf23b34 base::internal::Invoker<base::internal::BindState<void (base::internal::CancelableCallbackImpl<base::OnceCallback<void ()> >::*)(), base::WeakPtr<base::internal::CancelableCallbackImpl<base::OnceCallback<void ()> > > >, void ()>::RunOnce(base::internal::BindStateBase*) + 148
5   browser_tests                       0x0000000110c7a946 base::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 310
6   browser_tests                       0x0000000110c8fb55 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::sequence_manager::LazyNow*) + 405
7   browser_tests                       0x0000000110c8f858 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 168
8   browser_tests                       0x0000000110ce29a1 ___ZN4base24MessagePumpCFRunLoopBase13RunWorkSourceEPv_block_invoke + 49
9   browser_tests                       0x0000000110cdb662 base::mac::CallWithEHFrame(void () block_pointer) + 10
10  browser_tests                       0x0000000110ce223f base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 63
11  CoreFoundation                      0x00007fff38600884 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17

Original change's description:
> 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}

TBR=falken@chromium.org,fwang@igalia.com,dominickn@chromium.org

Change-Id: I026705f0116f3e0d684c8b1d80847aec85ec36db
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 522370
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485837Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818511}

chrome/browser/chrome_content_browser_client.cc

@@ -1807,18 +1807,6 @@ 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,8 +185,6 @@ 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,6 +17,7 @@
 #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"
@@ -31,30 +32,6 @@
 
 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() { }
@@ -177,10 +154,13 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest, Basic) {
 #if defined(OS_MAC)
   ASSERT_TRUE(test::RegisterAppWithLaunchServices());
 #endif
-  permissions::PermissionRequestManager::FromWebContents(
-      browser()->tab_strip_model()->GetActiveWebContents())
-      ->set_auto_response_for_test(
-          permissions::PermissionRequestManager::ACCEPT_ALL);
+  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(
+      permissions::PermissionRequestManager::ACCEPT_ALL);
 
   const extensions::Extension* extension =
       LoadExtension(test_data_dir_.AppendASCII("protocol_handler"));
@@ -190,87 +170,16 @@ 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');"));
-    waiter.Wait();
-  }
+  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();
 
   // 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,7 +19,6 @@
 #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"
@@ -175,17 +174,7 @@ void MaybeCreateLoaderOnCoreThread(
                      std::move(fallback_callback)));
 }
 
-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;
-
+bool SchemeMaySupportRedirectingToHTTPS(const GURL& url) {
 #if defined(OS_CHROMEOS)
   return url.SchemeIs(kExternalFileScheme);
 #else   // OS_CHROMEOS
@@ -213,8 +202,7 @@ ServiceWorkerMainResourceLoaderInterceptor::CreateForNavigation(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (!ShouldCreateForNavigation(
-          url, request_info.begin_params->request_destination,
-          navigation_handle->context_wrapper()->browser_context())) {
+          url, request_info.begin_params->request_destination)) {
     return nullptr;
   }
 
@@ -389,8 +377,7 @@ ServiceWorkerMainResourceLoaderInterceptor::
 // static
 bool ServiceWorkerMainResourceLoaderInterceptor::ShouldCreateForNavigation(
     const GURL& url,
-    network::mojom::RequestDestination request_destination,
-    BrowserContext* browser_context) {
+    network::mojom::RequestDestination request_destination) {
   // <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 ||
@@ -398,10 +385,10 @@ bool ServiceWorkerMainResourceLoaderInterceptor::ShouldCreateForNavigation(
     return false;
   }
 
-  // Create the interceptor even for insecure HTTP since it's used in the
+  // Create the handler even for insecure HTTP since it's used in the
   // case of redirect to HTTPS.
   return url.SchemeIsHTTPOrHTTPS() || OriginCanAccessServiceWorkers(url) ||
-         SchemeMaySupportRedirectingToHTTPS(browser_context, url);
+         SchemeMaySupportRedirectingToHTTPS(url);
 }
 
 void ServiceWorkerMainResourceLoaderInterceptor::RequestHandlerWrapper(

content/browser/service_worker/service_worker_main_resource_loader_interceptor.h

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

content/browser/service_worker/service_worker_main_resource_loader_interceptor_unittest.cc

@@ -4,8 +4,6 @@
 
 #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"
 
@@ -13,28 +11,12 @@ 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,
-                                  browser_context_.get());
+        ShouldCreateForNavigation(url, request_destination);
   }
-
- private:
-  BrowserTaskEnvironment task_environment_{BrowserTaskEnvironment::IO_MAINLOOP};
-  std::unique_ptr<TestBrowserContext> browser_context_;
 };
 
 TEST_F(ServiceWorkerMainResourceLoaderInterceptorTest,

content/public/browser/content_browser_client.cc

@@ -200,12 +200,6 @@ 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,13 +463,6 @@ 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