Make fieldtrial annotation work with features specified in command line

Before this CL, when FieldTrials.collectFieldTrials() is invoked,
mergeFeatureLists() has not been called, so enabled features are not
correctly populated. This makes collectFieldTrials() produce incorrect
data, which leads to the crash in applyFieldTrials().

This CL makes sure the enabled/disabled features specified in the
command line arguments are visible to FieldTrials.

Bug: 1113847, 1102633
Change-Id: I5c49c41c87fd0219c73a967db2ee33dc8237f52f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2344805Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796552}

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/Features.java

@@ -12,6 +12,7 @@ import org.chromium.base.CommandLine;
 import org.chromium.base.test.util.AnnotationRule;
 import org.chromium.chrome.browser.flags.CachedFeatureFlags;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 
 import java.lang.annotation.Annotation;
 import java.lang.annotation.Retention;
@@ -145,16 +146,10 @@ public class Features {
 
     /**
      * Feature processor intended to be used in instrumentation tests with native library, like
-     * {@link ChromeBrowserTestRule}. The collected feature states would be applied to
-     * {@link CommandLine}.
+     * those run with {@link ChromeJUnit4ClassRunner}. The collected feature states would be applied
+     * to {@link CommandLine}.
      */
     public static class InstrumentationProcessor extends Processor {
-        @Override
-        protected void collectFeatures() {
-            super.collectFeatures();
-            FieldTrials.getInstance().collectFieldTrials();
-        }
-
         @Override
         protected void applyFeatures() {
             getInstance().applyForInstrumentation();

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/FieldTrials.java

@@ -6,7 +6,6 @@ package org.chromium.chrome.test.util.browser;
 
 import org.chromium.base.BaseSwitches;
 import org.chromium.base.CommandLine;
-import org.chromium.base.Log;
 import org.chromium.chrome.browser.flags.CachedFeatureFlags;
 import org.chromium.chrome.browser.flags.CachedFieldTrialParameter;
 
@@ -22,7 +21,6 @@ import java.util.Set;
  */
 public class FieldTrials {
     private static FieldTrials sInstance;
-    private final Set<String> mEnabledFeatures = new HashSet<>();
     private final Map<String, Map<String, String>> mTrialToParamValueMap = new HashMap<>();
     private final Map<String, Set<String>> mTrialToFeatureNameMap = new HashMap<>();
     private static final String TAG = "FieldTrials";
@@ -34,42 +32,6 @@ public class FieldTrials {
         return sInstance;
     }
 
-    /**
-     * Collects field trial info from the CommandLine.
-     */
-    public void collectFieldTrials() {
-        CommandLine commandLine = CommandLine.getInstance();
-        String forceFieldTrials =
-                commandLine.getSwitchValue(BaseSwitches.FORCE_FIELD_TRIALS_SWITCH);
-        String forceFieldTrialParams =
-                commandLine.getSwitchValue(BaseSwitches.FORCE_FIELD_TRIAL_PARAMS_SWITCH);
-        String enableFeatures = commandLine.getSwitchValue(BaseSwitches.ENABLE_FEATURES);
-
-        Set<String> enableFeaturesSet = new HashSet<>();
-        if (enableFeatures != null) {
-            Collections.addAll(enableFeaturesSet, enableFeatures.split(","));
-            for (String enabledFeature : enableFeaturesSet) {
-                mEnabledFeatures.add(cleanupFeatureName(enabledFeature));
-            }
-        }
-
-        if (forceFieldTrials == null || forceFieldTrialParams == null || enableFeatures == null) {
-            return;
-        }
-
-        try {
-            updateTrialToParamValueMap(forceFieldTrialParams.split(","));
-            updateTrialFeatureMap(forceFieldTrials.split("/"), enableFeaturesSet);
-        } catch (Exception e) {
-            Log.e(TAG,
-                    "The format of field trials parameters declared isn't correct:"
-                            + BaseSwitches.FORCE_FIELD_TRIALS_SWITCH + "=" + forceFieldTrials + ", "
-                            + BaseSwitches.FORCE_FIELD_TRIAL_PARAMS_SWITCH + "="
-                            + forceFieldTrialParams + ".");
-            return;
-        }
-    }
-
     private static String cleanupFeatureName(String featureNameWithTrial) {
         if (!featureNameWithTrial.contains("<")) return featureNameWithTrial;
         return featureNameWithTrial.split("<")[0];
@@ -145,20 +107,47 @@ public class FieldTrials {
      * in CachedFeatureFlags.
      */
     public void applyFieldTrials() {
-        for (String featureName : mEnabledFeatures) {
-            CachedFeatureFlags.setForTesting(featureName, true);
+        CommandLine commandLine = CommandLine.getInstance();
+        String forceFieldTrials =
+                commandLine.getSwitchValue(BaseSwitches.FORCE_FIELD_TRIALS_SWITCH);
+        String forceFieldTrialParams =
+                commandLine.getSwitchValue(BaseSwitches.FORCE_FIELD_TRIAL_PARAMS_SWITCH);
+        String enableFeatures = commandLine.getSwitchValue(BaseSwitches.ENABLE_FEATURES);
+
+        Set<String> enableFeaturesSet = new HashSet<>();
+        if (enableFeatures != null) {
+            Collections.addAll(enableFeaturesSet, enableFeatures.split(","));
+            for (String enabledFeature : enableFeaturesSet) {
+                CachedFeatureFlags.setForTesting(cleanupFeatureName(enabledFeature), true);
+            }
         }
 
-        for (Map.Entry<String, Map<String, String>> entry : mTrialToParamValueMap.entrySet()) {
-            String trialName = entry.getKey();
-            Set<String> featureSet = mTrialToFeatureNameMap.get(trialName);
-            for (String featureName : featureSet) {
-                Map<String, String> params = entry.getValue();
-                for (Map.Entry<String, String> param : params.entrySet()) {
-                    CachedFieldTrialParameter.setForTesting(
-                            featureName, param.getKey(), param.getValue());
+        if (forceFieldTrials == null || forceFieldTrialParams == null || enableFeatures == null) {
+            return;
+        }
+
+        try {
+            updateTrialToParamValueMap(forceFieldTrialParams.split(","));
+            updateTrialFeatureMap(forceFieldTrials.split("/"), enableFeaturesSet);
+
+            for (Map.Entry<String, Map<String, String>> entry : mTrialToParamValueMap.entrySet()) {
+                String trialName = entry.getKey();
+                Set<String> featureSet = mTrialToFeatureNameMap.get(trialName);
+                if (featureSet == null) continue;
+                for (String featureName : featureSet) {
+                    Map<String, String> params = entry.getValue();
+                    for (Map.Entry<String, String> param : params.entrySet()) {
+                        CachedFieldTrialParameter.setForTesting(
+                                featureName, param.getKey(), param.getValue());
+                    }
                 }
             }
+        } catch (Exception e) {
+            assert false : e.toString() + "\n"
+                    + "The format of field trials parameters declared isn't correct:"
+                    + BaseSwitches.FORCE_FIELD_TRIALS_SWITCH + "=" + forceFieldTrials + ", "
+                    + BaseSwitches.FORCE_FIELD_TRIAL_PARAMS_SWITCH + "=" + forceFieldTrialParams
+                    + ", " + BaseSwitches.ENABLE_FEATURES + "=" + enableFeatures + ".";
         }
     }
 

testing/buildbot/filters/bfcache.chrome_public_test_apk.filter

 # These tests currently fail when run with --enable-features=BackForwardCache
-
-# https://crbug.com/1102633
-# Needs investigation.
--org.chromium.chrome.features.start_surface.StartSurfaceTest.*
--org.chromium.chrome.features.start_surface.StartSurfaceLayoutTest.*
--org.chromium.chrome.browser.tasks.ReturnToChromeTest.*
--org.chromium.chrome.features.start_surface.InstantStartTest.*
--org.chromium.chrome.browser.datareduction.DataReductionSavingsMilestonePromoTest.*
--org.chromium.chrome.browser.tasks.tab_management.TabSuggestionMessageCardTest.*
--org.chromium.chrome.browser.feed.FeedContentStorageConformanceTest.*
--org.chromium.chrome.browser.status_indicator.StatusIndicatorTest.*