Set up an experiment for requiring a dedicated process for sign-in.

This CL introduces a new base::Feature to control whether the sign-in
origin (https://accounts.google.com) requires a dedicated process, to
be used for a Finch experiment.  As part of this, during
initialization content/ will call out to a new ContentBrowserClient
method to get a whitelist of origins requiring a dedicated process.
These origins will be process-isolated in all profiles for the
lifetime of the browser.

Bug: 713444, 739418
Change-Id: I308cd1eb209394a9990c5eb5793a95a78d76485a
Reviewed-on: https://chromium-review.googlesource.com/569519
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487263}

chrome/browser/chrome_content_browser_client.cc

@@ -195,6 +195,7 @@
 #include "device/usb/public/interfaces/chooser_service.mojom.h"
 #include "device/usb/public/interfaces/device_manager.mojom.h"
 #include "extensions/features/features.h"
+#include "google_apis/gaia/gaia_urls.h"
 #include "gpu/config/gpu_switches.h"
 #include "media/audio/audio_manager.h"
 #include "media/media_features.h"
@@ -1401,6 +1402,16 @@ bool ChromeContentBrowserClient::ShouldAssignSiteForURL(const GURL& url) {
   return !url.SchemeIs(chrome::kChromeNativeScheme);
 }
 
+std::vector<url::Origin>
+ChromeContentBrowserClient::GetOriginsRequiringDedicatedProcess() {
+  std::vector<url::Origin> isolated_origin_list;
+
+  if (base::FeatureList::IsEnabled(features::kSignInProcessIsolation))
+    isolated_origin_list.emplace_back(GaiaUrls::GetInstance()->gaia_url());
+
+  return isolated_origin_list;
+}
+
 namespace {
 
 bool IsAutoReloadEnabled() {

chrome/browser/chrome_content_browser_client.h

@@ -131,6 +131,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       const GURL& current_url,
       const GURL& new_url) override;
   bool ShouldAssignSiteForURL(const GURL& url) override;
+  std::vector<url::Origin> GetOriginsRequiringDedicatedProcess() override;
   void AppendExtraCommandLineSwitches(base::CommandLine* command_line,
                                       int child_process_id) override;
   std::string GetApplicationLocale() override;

chrome/browser/chrome_navigation_browsertest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
@@ -11,17 +12,20 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/network_session_configurator/common/network_switches.h"
 #include "components/url_formatter/url_formatter.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/context_menu_params.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "google_apis/gaia/gaia_switches.h"
 #include "net/dns/mock_host_resolver.h"
 
 class ChromeNavigationBrowserTest : public InProcessBrowserTest {
@@ -436,3 +440,70 @@ IN_PROC_BROWSER_TEST_F(ChromeNavigationBrowserTest,
   EXPECT_FALSE(navigation_observer.was_same_document());
   EXPECT_FALSE(navigation_observer.was_renderer_initiated());
 }
+
+class SignInIsolationBrowserTest : public ChromeNavigationBrowserTest {
+ public:
+  SignInIsolationBrowserTest()
+      : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}
+  ~SignInIsolationBrowserTest() override {}
+
+  void SetUp() override {
+    feature_list_.InitAndEnableFeature(features::kSignInProcessIsolation);
+    https_server_.ServeFilesFromSourceDirectory("chrome/test/data");
+    ASSERT_TRUE(https_server_.InitializeAndListen());
+    ChromeNavigationBrowserTest::SetUp();
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    // Override the sign-in URL so that it includes correct port from the test
+    // server.
+    command_line->AppendSwitchASCII(
+        ::switches::kGaiaUrl,
+        https_server()->GetURL("accounts.google.com", "/").spec());
+
+    // Ignore cert errors so that the sign-in URL can be loaded from a site
+    // other than localhost (the EmbeddedTestServer serves a certificate that
+    // is valid for localhost).
+    command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
+
+    ChromeNavigationBrowserTest::SetUpCommandLine(command_line);
+  }
+
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    https_server_.StartAcceptingConnections();
+    ChromeNavigationBrowserTest::SetUpOnMainThread();
+  }
+
+  net::EmbeddedTestServer* https_server() { return &https_server_; }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+  net::EmbeddedTestServer https_server_;
+
+  DISALLOW_COPY_AND_ASSIGN(SignInIsolationBrowserTest);
+};
+
+// This test ensures that the sign-in origin requires a dedicated process.  It
+// only ensures that the corresponding base::Feature works properly;
+// IsolatedOriginTest provides the main test coverage of origins whitelisted
+// for process isolation.  See https://crbug.com/739418.
+IN_PROC_BROWSER_TEST_F(SignInIsolationBrowserTest, NavigateToSignInPage) {
+  const GURL first_url =
+      embedded_test_server()->GetURL("google.com", "/title1.html");
+  const GURL signin_url =
+      https_server()->GetURL("accounts.google.com", "/title1.html");
+  ui_test_utils::NavigateToURL(browser(), first_url);
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  scoped_refptr<content::SiteInstance> first_instance(
+      web_contents->GetMainFrame()->GetSiteInstance());
+
+  // Make sure that a renderer-initiated navigation to the sign-in page swaps
+  // processes.
+  content::TestNavigationManager manager(web_contents, signin_url);
+  EXPECT_TRUE(
+      ExecuteScript(web_contents, "location = '" + signin_url.spec() + "';"));
+  manager.WaitForNavigationFinished();
+  EXPECT_NE(web_contents->GetMainFrame()->GetSiteInstance(), first_instance);
+}

content/browser/browser_main_loop.cc

@@ -875,6 +875,16 @@ int BrowserMainLoop::PreCreateThreads() {
     RenderProcessHost::SetRunRendererInProcess(true);
 #endif
 
+  // Initialize origins that are whitelisted for process isolation.  Must be
+  // done after base::FeatureList is initialized, but before any navigations
+  // can happen.
+  std::vector<url::Origin> origins =
+      GetContentClient()->browser()->GetOriginsRequiringDedicatedProcess();
+  ChildProcessSecurityPolicyImpl* policy =
+      ChildProcessSecurityPolicyImpl::GetInstance();
+  for (auto origin : origins)
+    policy->AddIsolatedOrigin(origin);
+
   EVP_set_buggy_rsa_parser(
       base::FeatureList::IsEnabled(features::kBuggyRSAParser));
 

content/public/browser/content_browser_client.cc

@@ -23,6 +23,7 @@
 #include "storage/browser/quota/quota_manager.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
+#include "url/origin.h"
 
 namespace content {
 
@@ -139,6 +140,11 @@ bool ContentBrowserClient::ShouldAssignSiteForURL(const GURL& url) {
   return true;
 }
 
+std::vector<url::Origin>
+ContentBrowserClient::GetOriginsRequiringDedicatedProcess() {
+  return std::vector<url::Origin>();
+}
+
 std::string ContentBrowserClient::GetApplicationLocale() {
   return "en-US";
 }

content/public/browser/content_browser_client.h

@@ -310,6 +310,10 @@ class CONTENT_EXPORT ContentBrowserClient {
   // current SiteInstance, if it does not yet have a site.
   virtual bool ShouldAssignSiteForURL(const GURL& url);
 
+  // Allows the embedder to provide a list of origins that require a dedicated
+  // process.
+  virtual std::vector<url::Origin> GetOriginsRequiringDedicatedProcess();
+
   // Allows the embedder to pass extra command line flags.
   // switches::kProcessType will already be set at this point.
   virtual void AppendExtraCommandLineSwitches(base::CommandLine* command_line,

content/public/common/content_features.cc

@@ -265,6 +265,11 @@ const base::Feature kServiceWorkerScriptStreaming{
 const base::Feature kSharedArrayBuffer{"SharedArrayBuffer",
                                        base::FEATURE_ENABLED_BY_DEFAULT};
 
+// An experiment to require process isolation for the sign-in origin,
+// https://accounts.google.com.  Launch bug: https://crbug.com/739418.
+const base::Feature kSignInProcessIsolation{"sign-in-process-isolation",
+                                            base::FEATURE_DISABLED_BY_DEFAULT};
+
 // An experiment for skipping compositing small scrollers.
 const base::Feature kSkipCompositingSmallScrollers{
     "SkipCompositingSmallScrollers", base::FEATURE_DISABLED_BY_DEFAULT};

content/public/common/content_features.h

@@ -71,6 +71,7 @@ CONTENT_EXPORT extern const base::Feature kScrollAnchoring;
 CONTENT_EXPORT extern const base::Feature kServiceWorkerNavigationPreload;
 CONTENT_EXPORT extern const base::Feature kServiceWorkerScriptStreaming;
 CONTENT_EXPORT extern const base::Feature kSharedArrayBuffer;
+CONTENT_EXPORT extern const base::Feature kSignInProcessIsolation;
 CONTENT_EXPORT extern const base::Feature kSkipCompositingSmallScrollers;
 CONTENT_EXPORT extern const base::Feature kSlimmingPaintInvalidation;
 CONTENT_EXPORT extern const base::Feature kTimerThrottlingForHiddenFrames;