Merge isolated origins from cmdline flag and field trials.

Previously, specifying the --isolate-origins command-line flag
prevented any field trials involving the IsolateOrigins feature from
specifying additional isolated origins.  This meant that it's not
possible to experiment with additional isolated origins via field
trials when an IsolateOrigins enterprise policy is in effect.  This CL
changes the logic to instead merge any isolated origins specified in
field trials with those specified by the cmdline flag.

Bug: 894535
Change-Id: If38fe3280dad7f4af3d8b5383556239493ffc7f3
Reviewed-on: https://chromium-review.googlesource.com/c/1279414Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599730}

content/browser/isolated_origin_browsertest.cc

@@ -1115,6 +1115,42 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginFieldTrialTest, Test) {
       policy->IsIsolatedOrigin(url::Origin::Create(GURL("https://bar.com/"))));
 }
 
+class IsolatedOriginCommandLineAndFieldTrialTest
+    : public IsolatedOriginFieldTrialTest {
+ public:
+  IsolatedOriginCommandLineAndFieldTrialTest() = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitchASCII(
+        switches::kIsolateOrigins,
+        "https://cmd.line.com/,https://cmdline.com/");
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(IsolatedOriginCommandLineAndFieldTrialTest);
+};
+
+// Verify that the lists of isolated origins specified via --isolate-origins
+// and via field trials are merged.  See https://crbug.com/894535.
+IN_PROC_BROWSER_TEST_F(IsolatedOriginCommandLineAndFieldTrialTest, Test) {
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  // --isolate-origins should take effect regardless of the
+  //   kDisableSiteIsolationTrials opt-out flag.
+  EXPECT_TRUE(policy->IsIsolatedOrigin(
+      url::Origin::Create(GURL("https://cmd.line.com/"))));
+  EXPECT_TRUE(policy->IsIsolatedOrigin(
+      url::Origin::Create(GURL("https://cmdline.com/"))));
+
+  // Field trial origins should also take effect, but only if the opt-out flag
+  // is not present.
+  bool expected_to_isolate = !base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kDisableSiteIsolationTrials);
+  EXPECT_EQ(expected_to_isolate, policy->IsIsolatedOrigin(url::Origin::Create(
+                                     GURL("https://field.trial.com/"))));
+  EXPECT_EQ(
+      expected_to_isolate,
+      policy->IsIsolatedOrigin(url::Origin::Create(GURL("https://bar.com/"))));
+}
+
 // This is a regresion test for https://crbug.com/793350 - the long list of
 // origins to isolate used to be unnecessarily propagated to the renderer
 // process, trigerring a crash due to exceeding kZygoteMaxMessageLength.

content/public/browser/site_isolation_policy.cc

@@ -112,17 +112,18 @@ SiteIsolationPolicy::GetIsolatedOriginsFromEnvironment() {
   std::string cmdline_arg =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kIsolateOrigins);
+  std::vector<url::Origin> origins;
   if (!cmdline_arg.empty()) {
-    std::vector<url::Origin> cmdline_origins =
-        ParseIsolatedOrigins(cmdline_arg);
+    origins = ParseIsolatedOrigins(cmdline_arg);
     UMA_HISTOGRAM_COUNTS_1000("SiteIsolation.IsolateOrigins.Size",
-                              cmdline_origins.size());
-    return cmdline_origins;
+                              origins.size());
   }
 
+  // --isolate-origins (both command-line flag and enterprise policy) trumps
+  // the opt-out flag.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableSiteIsolationTrials)) {
-    return std::vector<url::Origin>();
+    return origins;
   }
 
   // The feature needs to be checked last, because checking the feature
@@ -132,9 +133,13 @@ SiteIsolationPolicy::GetIsolatedOriginsFromEnvironment() {
     std::string field_trial_arg = base::GetFieldTrialParamValueByFeature(
         features::kIsolateOrigins,
         features::kIsolateOriginsFieldTrialParamName);
-    return ParseIsolatedOrigins(field_trial_arg);
+    std::vector<url::Origin> field_trial_origins =
+        ParseIsolatedOrigins(field_trial_arg);
+    origins.reserve(origins.size() + field_trial_origins.size());
+    std::move(field_trial_origins.begin(), field_trial_origins.end(),
+              std::back_inserter(origins));
   }
-  return std::vector<url::Origin>();
+  return origins;
 }
 
 // static