Fix the timing of activation of Site Isolation field trials.

Bug: 825325
Change-Id: I17d46070a5fc5a060b0881a2ba58f52ffce07f1d
Reviewed-on: https://chromium-review.googlesource.com/978674
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545640}

components/printing/browser/print_manager_utils.cc

@@ -33,15 +33,27 @@ void CreateCompositeClientIfNeeded(content::WebContents* web_contents) {
   // where OOPIF is used such as isolate-extensions, but should be good for
   // feature testing purpose. Eventually, we will remove this check and use pdf
   // compositor service by default for printing.
+
+  bool is_site_isolation_enabled;
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kSitePerProcess) ||
       base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kIsolateOrigins) ||
-      ((base::FeatureList::IsEnabled(::features::kSitePerProcess) ||
-        base::FeatureList::IsEnabled(::features::kIsolateOrigins)) &&
-       !base::CommandLine::ForCurrentProcess()->HasSwitch(
-           switches::kDisableSiteIsolationTrials)) ||
-      base::FeatureList::IsEnabled(::features::kTopDocumentIsolation) ||
+          switches::kIsolateOrigins)) {
+    is_site_isolation_enabled = true;
+  } else if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+                 switches::kDisableSiteIsolationTrials)) {
+    is_site_isolation_enabled = false;
+  } else {
+    // The features need to be checked last, because checking a feature
+    // activates the field trial and assigns the client either to a control or
+    // an experiment group.
+    is_site_isolation_enabled =
+        base::FeatureList::IsEnabled(::features::kSitePerProcess) ||
+        base::FeatureList::IsEnabled(::features::kIsolateOrigins) ||
+        base::FeatureList::IsEnabled(::features::kTopDocumentIsolation);
+  }
+
+  if (is_site_isolation_enabled ||
       base::FeatureList::IsEnabled(
           printing::features::kUsePdfCompositorServiceForPrint)) {
     PrintCompositeClient::CreateForWebContents(web_contents);

content/browser/site_isolation_policy.cc

@@ -26,19 +26,29 @@ namespace content {
 
 // static
 bool SiteIsolationPolicy::UseDedicatedProcessesForAllSites() {
-  return base::CommandLine::ForCurrentProcess()->HasSwitch(
-             switches::kSitePerProcess) ||
-         (base::FeatureList::IsEnabled(features::kSitePerProcess) &&
-          !base::CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kDisableSiteIsolationTrials));
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSitePerProcess)) {
+    return true;
+  }
+
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableSiteIsolationTrials)) {
+    return false;
+  }
+
+  // The feature needs to be checked last, because checking the feature
+  // activates the field trial and assigns the client either to a control or an
+  // experiment group.
+  return base::FeatureList::IsEnabled(features::kSitePerProcess);
 }
 
 // static
 SiteIsolationPolicy::CrossSiteDocumentBlockingEnabledState
 SiteIsolationPolicy::IsCrossSiteDocumentBlockingEnabled() {
   if (base::FeatureList::IsEnabled(
-          ::features::kCrossSiteDocumentBlockingAlways))
+          ::features::kCrossSiteDocumentBlockingAlways)) {
     return XSDB_ENABLED_UNCONDITIONALLY;
+  }
 
   if (base::FeatureList::IsEnabled(
           ::features::kCrossSiteDocumentBlockingIfIsolating)) {
@@ -54,16 +64,33 @@ bool SiteIsolationPolicy::IsTopDocumentIsolationEnabled() {
   if (UseDedicatedProcessesForAllSites())
     return false;
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableSiteIsolationTrials)) {
+    return false;
+  }
+
+  // The feature needs to be checked last, because checking the feature
+  // activates the field trial and assigns the client either to a control or an
+  // experiment group.
   return base::FeatureList::IsEnabled(::features::kTopDocumentIsolation);
 }
 
 // static
 bool SiteIsolationPolicy::AreIsolatedOriginsEnabled() {
-  return base::CommandLine::ForCurrentProcess()->HasSwitch(
-             switches::kIsolateOrigins) ||
-         (base::FeatureList::IsEnabled(features::kIsolateOrigins) &&
-          !base::CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kDisableSiteIsolationTrials));
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kIsolateOrigins)) {
+    return true;
+  }
+
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableSiteIsolationTrials)) {
+    return false;
+  }
+
+  // The feature needs to be checked last, because checking the feature
+  // activates the field trial and assigns the client either to a control or an
+  // experiment group.
+  return base::FeatureList::IsEnabled(features::kIsolateOrigins);
 }
 
 // static
@@ -80,15 +107,20 @@ SiteIsolationPolicy::GetIsolatedOriginsFromEnvironment() {
     return cmdline_origins;
   }
 
-  if (base::FeatureList::IsEnabled(features::kIsolateOrigins) &&
-      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableSiteIsolationTrials)) {
+    return std::vector<url::Origin>();
+  }
+
+  // The feature needs to be checked last, because checking the feature
+  // activates the field trial and assigns the client either to a control or an
+  // experiment group.
+  if (base::FeatureList::IsEnabled(features::kIsolateOrigins)) {
     std::string field_trial_arg = base::GetFieldTrialParamValueByFeature(
         features::kIsolateOrigins,
         features::kIsolateOriginsFieldTrialParamName);
     return ParseIsolatedOrigins(field_trial_arg);
   }
-
   return std::vector<url::Origin>();
 }