Removing ShellContentBrowserClient::DoesSiteRequireDedicatedProcess.

Why remove DoesSiteRequireDedicatedProcess
==========================================

This CL removes
ShellContentBrowserClient::DoesSiteRequireDedicatedProcess.  We plan
to remove 2 other overrides of this ContentBrowserClient method in
other CLs.  We want to remove this ContentBrowserClient method
altogether, because
1) it is currently the only reason
   SiteInstanceImpl::DetermineProcessLockURL needs to take
   BrowserContext* as an argument (and therefore is problematic on
   threads other than UI thread)
2) the method was initially introduced to support --isolate-extensions
   which has been obsolete since shipping --site-per-process in M67.


Removal mechanics
=================

It seems that NavigationControllerOopifBrowserTest test never relied
on switches::kIsolateSitesForTesting.  In particular, I can't find
any references to ".is" in
https://codereview.chromium.org/1505343002/patch/100001/110001

IsolateIcelandFrameTreeBrowserTest can switch from using
switches::kIsolateSitesForTesting to using switches::kIsolateOrigins.


Bug: 898281
Change-Id: Idc8e45361e791ff30b9391666c55cf10159e793a
Reviewed-on: https://chromium-review.googlesource.com/c/1307876
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605137}

content/browser/frame_host/frame_tree_browsertest.cc

@@ -849,11 +849,15 @@ class IsolateIcelandFrameTreeBrowserTest : public ContentBrowserTest {
   IsolateIcelandFrameTreeBrowserTest() {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
-    // blink suppresses navigations to blob URLs of origins different from the
+    // Blink suppresses navigations to blob URLs of origins different from the
     // frame initiating the navigation. We disable those checks for this test,
     // to test what happens in a compromise scenario.
     command_line->AppendSwitch(switches::kDisableWebSecurity);
-    command_line->AppendSwitchASCII(switches::kIsolateSitesForTesting, "*.is");
+
+    // ProcessSwitchForIsolatedBlob test below requires that one of URLs used in
+    // the test (blob:http://b.is:2932/) belongs to an isolated origin.
+    command_line->AppendSwitchASCII(switches::kIsolateOrigins,
+                                    "http://b.is:2932/");
   }
 
   void SetUpOnMainThread() override {
@@ -879,7 +883,7 @@ IN_PROC_BROWSER_TEST_F(IsolateIcelandFrameTreeBrowserTest,
 
   // The navigation targets an invalid blob url; that's intentional to trigger
   // an error response. The response should commit in a process dedicated to
-  // http://b.is.
+  // http://b.is:2932.
   EXPECT_EQ(
       "done",
       EvalJs(
@@ -896,7 +900,7 @@ IN_PROC_BROWSER_TEST_F(IsolateIcelandFrameTreeBrowserTest,
       " Site A ------------ proxies for B\n"
       "   +--Site B ------- proxies for A\n"
       "Where A = http://a.com/\n"
-      "      B = http://b.is/",
+      "      B = http://b.is:2932/",
       FrameTreeVisualizer().DepictFrameTree(root));
 }
 

content/browser/frame_host/navigation_controller_impl_browsertest.cc

@@ -4797,25 +4797,9 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
   EXPECT_EQ(frame_entry->redirect_chain()[1], frame_final_url);
 }
 
-// Support a set of tests that isolate only a subset of sites with
-// out-of-process iframes (OOPIFs).
-class NavigationControllerOopifBrowserTest
-    : public NavigationControllerBrowserTest {
- public:
-  NavigationControllerOopifBrowserTest() {}
-
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    // Enable the OOPIF framework but only isolate sites from a single TLD.
-    command_line->AppendSwitchASCII(switches::kIsolateSitesForTesting, "*.is");
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(NavigationControllerOopifBrowserTest);
-};
-
 // Verify that restoring a NavigationEntry with cross-site subframes does not
 // create out-of-process iframes unless the current SiteIsolationPolicy says to.
-IN_PROC_BROWSER_TEST_F(NavigationControllerOopifBrowserTest,
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        RestoreWithoutExtraOopifs) {
   // 1. Start on a page with a data URL iframe.
   GURL main_url_a(embedded_test_server()->GetURL(

content/shell/browser/shell_content_browser_client.cc

@@ -15,7 +15,6 @@
 #include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/path_service.h"
-#include "base/strings/pattern.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "content/public/browser/client_certificate_delegate.h"
@@ -161,30 +160,6 @@ BrowserMainParts* ShellContentBrowserClient::CreateBrowserMainParts(
   return shell_browser_main_parts_;
 }
 
-bool ShellContentBrowserClient::DoesSiteRequireDedicatedProcess(
-    BrowserContext* browser_context,
-    const GURL& effective_site_url) {
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  if (!command_line->HasSwitch(switches::kIsolateSitesForTesting))
-    return false;
-  std::string pattern =
-      command_line->GetSwitchValueASCII(switches::kIsolateSitesForTesting);
-
-  url::Origin origin = url::Origin::Create(effective_site_url);
-
-  if (!origin.opaque()) {
-    // Schemes like blob or filesystem, which have an embedded origin, should
-    // already have been canonicalized to the origin site.
-    CHECK_EQ(origin.scheme(), effective_site_url.scheme())
-        << "a site url should have the same scheme as its origin.";
-  }
-
-  // Practically |origin.Serialize()| is the same as
-  // |effective_site_url.spec()|, except Origin serialization strips the
-  // trailing "/", which makes for cleaner wildcard patterns.
-  return base::MatchPattern(origin.Serialize(), pattern);
-}
-
 bool ShellContentBrowserClient::IsHandledURL(const GURL& url) {
   if (!url.is_valid())
     return false;
@@ -305,13 +280,6 @@ void ShellContentBrowserClient::AppendExtraCommandLineSwitches(
         base::CommandLine::ForCurrentProcess()->GetSwitchValuePath(
             switches::kCrashDumpsDir));
   }
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kIsolateSitesForTesting)) {
-    command_line->AppendSwitchASCII(
-        switches::kIsolateSitesForTesting,
-        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-            switches::kIsolateSitesForTesting));
-  }
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kRegisterFontFiles)) {
     command_line->AppendSwitchASCII(

content/shell/browser/shell_content_browser_client.h

@@ -33,8 +33,6 @@ class ShellContentBrowserClient : public ContentBrowserClient {
   // ContentBrowserClient overrides.
   BrowserMainParts* CreateBrowserMainParts(
       const MainFunctionParams& parameters) override;
-  bool DoesSiteRequireDedicatedProcess(BrowserContext* browser_context,
-                                       const GURL& effective_site_url) override;
   bool IsHandledURL(const GURL& url) override;
   void BindInterfaceRequestFromFrame(
       content::RenderFrameHost* render_frame_host,

content/shell/common/shell_switches.cc

@@ -20,14 +20,6 @@ const char kCrashDumpsDir[] = "crash-dumps-dir";
 // and debugging of layout tests that rely on it.
 const char kExposeInternalsForTesting[] = "expose-internals-for-testing";
 
-// Enable site isolation (--site-per-process style isolation) for a subset of
-// sites. The argument is a wildcard pattern which will be matched against the
-// site URL to determine which sites to isolate. This can be used to isolate
-// just one top-level domain, or just one scheme. Example usages:
-//     --isolate-sites-for-testing=*.com
-//     --isolate-sites-for-testing=https://*
-const char kIsolateSitesForTesting[] = "isolate-sites-for-testing";
-
 // Registers additional font files on Windows (for fonts outside the usual
 // %WINDIR%\Fonts location). Multiple files can be used by separating them
 // with a semicolon (;).

content/shell/common/shell_switches.h

@@ -15,7 +15,6 @@ namespace switches {
 extern const char kContentShellDataPath[];
 extern const char kCrashDumpsDir[];
 extern const char kExposeInternalsForTesting[];
-extern const char kIsolateSitesForTesting[];
 extern const char kRegisterFontFiles[];
 extern const char kContentShellHostWindowSize[];
 extern const char kContentShellHideToolbar[];