Remove ChromeContentBrowserClientExtensionsPart::OverrideNavigationParams

Why it is okay to remove this method
====================================

The ChromeContentBrowserClientExtensionsPart::OverrideNavigationParams
method ensured that the referrer used in navigations never points at an
extension-specific URL.  This method is not needed - even without this
method, the referrer is never set to an extension-specific - this is
because of the following places in the code (*all* 4 places need to be
tweaked to start using extension-specific URL in the newly added browser
test):

1. //content: blink::mojom::ReferrerPtr Referrer::SanitizeForRequest:

  if (!request.SchemeIsHTTPOrHTTPS() ||
      !sanitized_referrer->url.SchemeIsValidForReferrer()) {
    sanitized_referrer->url = GURL();
    return sanitized_referrer;
  }

2. //third_party/blink:
2a. String KURL::StrippedForUseAsReferrer()

  if (!ProtocolIsInHTTPFamily())
    return String();

2b. URLSchemesRegistry

  allowed_in_referrer_schemes({"http", "https"})

4. //url: GURL GURL::GetAsReferrer()

  if (!SchemeIsValidForReferrer())
    return GURL();


Why new test is desirable
=========================

This CL adds a new end-to-end test:
ExtensionBrowserTest.NoExtensionsInRefererHeader.

This test is desirable, because after tweaking all 4 places above, all
other tests continued to pass, except for a
ReferrerSanitizerTest.OnlyHTTPFamilyReferrer unit test.


Why removing the method is desirable
====================================

If we can remove ContentBrowserClient::OverrideNavigationParams
(overrides deal with extensions and NTP), then we may be able to proceed
with removal of ContentRendererClient::ShouldFork.  Removing the
extension-specific part of OverrideNavigationParams helps toward these
goals.


Bug: 1003957
Change-Id: I31399d62cf86d1caeb935b0218faf18a7f590248
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834890Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704368}

chrome/browser/chrome_content_browser_client.cc

@@ -1680,12 +1680,6 @@ void ChromeContentBrowserClient::OverrideNavigationParams(
     *referrer = content::Referrer();
     *initiator_origin = base::nullopt;
   }
-
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-  ChromeContentBrowserClientExtensionsPart::OverrideNavigationParams(
-      site_instance, transition, is_renderer_initiated, referrer,
-      initiator_origin);
-#endif
 }
 
 bool ChromeContentBrowserClient::ShouldStayInParentProcessForNTP(

chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

@@ -594,29 +594,6 @@ bool ChromeContentBrowserClientExtensionsPart::AllowServiceWorkerOnUI(
   return AllowServiceWorker(scope, script_url, extension);
 }
 
-// static
-void ChromeContentBrowserClientExtensionsPart::OverrideNavigationParams(
-    content::SiteInstance* site_instance,
-    ui::PageTransition* transition,
-    bool* is_renderer_initiated,
-    content::Referrer* referrer,
-    base::Optional<url::Origin>* initiator_origin) {
-  const Extension* extension =
-      ExtensionRegistry::Get(site_instance->GetBrowserContext())
-          ->enabled_extensions()
-          .GetExtensionOrAppByURL(site_instance->GetSiteURL());
-  if (!extension)
-    return;
-
-  // Hide the |referrer| for extension pages. We don't want sites to see a
-  // referrer of chrome-extension://<...>.
-  //
-  // OTOH, don't change |initiator_origin| - SameSite-cookies and Sec-Fetch-Site
-  // should still see the request as cross-site.
-  if (extension->is_extension())
-    *referrer = content::Referrer();
-}
-
 // static
 std::vector<url::Origin> ChromeContentBrowserClientExtensionsPart::
     GetOriginsRequiringDedicatedProcess() {

chrome/browser/extensions/chrome_content_browser_client_extensions_part.h

@@ -18,7 +18,6 @@
 #include "ui/base/page_transition_types.h"
 
 namespace content {
-struct Referrer;
 class RenderFrameHost;
 class RenderProcessHost;
 class ResourceContext;
@@ -77,12 +76,6 @@ class ChromeContentBrowserClientExtensionsPart
                                      const GURL& first_party_url,
                                      const GURL& script_url,
                                      content::BrowserContext* context);
-  static void OverrideNavigationParams(
-      content::SiteInstance* site_instance,
-      ui::PageTransition* transition,
-      bool* is_renderer_initiated,
-      content::Referrer* referrer,
-      base::Optional<url::Origin>* initiator_origin);
   static std::vector<url::Origin> GetOriginsRequiringDedicatedProcess();
 
   // Helper function to call InfoMap::SetSigninProcess().

chrome/browser/extensions/navigation_observer_browsertest.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/extensions/navigation_observer.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/no_renderer_crashes_assertion.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/extension_dialog_auto_confirm.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -132,4 +133,45 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest,
   }
 }
 
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, NoExtensionsInRefererHeader) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  scoped_refptr<const Extension> extension =
+      ChromeTestExtensionLoader(profile()).LoadExtension(
+          test_data_dir_.AppendASCII("simple_with_file"));
+  ASSERT_TRUE(extension);
+  GURL page_url = extension->GetResourceURL("file.html");
+  ui_test_utils::NavigateToURL(browser(), page_url);
+
+  // Click a link in the extension.
+  GURL target_url = embedded_test_server()->GetURL("/echoheader?referer");
+  const char kScriptTemplate[] = R"(
+      let a = document.createElement('a');
+      a.href = $1;
+      document.body.appendChild(a);
+      a.click();
+  )";
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::TestNavigationObserver nav_observer(web_contents, 1);
+  ExecuteScriptAsync(web_contents,
+                     content::JsReplace(kScriptTemplate, target_url));
+
+  // Wait for navigation to complete and verify it was successful.
+  nav_observer.WaitForNavigationFinished();
+  EXPECT_TRUE(nav_observer.last_navigation_succeeded());
+  EXPECT_EQ(target_url, nav_observer.last_navigation_url());
+  EXPECT_EQ(target_url, web_contents->GetLastCommittedURL());
+
+  // Verify that the Referrer header was not present (in particular, it should
+  // not reveal the identity of the extension).
+  content::WaitForLoadStop(web_contents);
+  EXPECT_EQ("None", content::EvalJs(web_contents, "document.body.innerText"));
+
+  // Verify that the initiator_origin was present and set to the extension.
+  ASSERT_TRUE(nav_observer.last_initiator_origin().has_value());
+  EXPECT_EQ(url::Origin::Create(page_url),
+            nav_observer.last_initiator_origin());
+}
+
 }  // namespace extensions