Make --force-fieldtrials not activate them in the browser process.

This is to make this testing flag more consistent with what happens
with a server-side field trial, so that the field trial doesn't
appear in about:version until client code explicitly queries its
state via a group() call.

Does not change the behaviour of the --force-fieldtrials flag
when passes to the renderer process, where it does need to
activate the trials so that they show up in crash reports, etc.

BUG=255142
TEST=Start a debug build of Chrome with the command line
flag --force-fieldtrials=Bat/Man/ and check that it doesn't
show up in chrome://version.

Review URL: https://chromiumcodereview.appspot.com/17945002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209795 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 77 insertions, 18 deletions

diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index cb1d81d..3b96040 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -366,7 +366,8 @@ void FieldTrialList::GetActiveFieldTrialGroups(
 }
 
 // static
-bool FieldTrialList::CreateTrialsFromString(const std::string& trials_string) {
+bool FieldTrialList::CreateTrialsFromString(const std::string& trials_string,
+                                            FieldTrialActivationMode mode) {
   DCHECK(global_);
   if (trials_string.empty() || !global_)
     return true;
@@ -388,10 +389,12 @@ bool FieldTrialList::CreateTrialsFromString(const std::string& trials_string) {
     FieldTrial* trial = CreateFieldTrial(name, group_name);
     if (!trial)
       return false;
-    // Call |group()| to mark the trial as "used" and notify observers, if any.
-    // This is needed to ensure the trial is properly reported in child process
-    // crash reports.
-    trial->group();
+    if (mode == ACTIVATE_TRIALS) {
+      // Call |group()| to mark the trial as "used" and notify observers, if
+      // any. This is useful to ensure that field trials created in child
+      // processes are properly reported in crash reports.
+      trial->group();
+    }
   }
   return true;
 }
diff --git a/base/metrics/field_trial.h b/base/metrics/field_trial.h
index 3c295ea..106bf8b 100644
--- a/base/metrics/field_trial.h
+++ b/base/metrics/field_trial.h
@@ -295,6 +295,13 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
 // Only one instance of this class exists.
 class BASE_EXPORT FieldTrialList {
  public:
+  // Specifies whether field trials should be activated (marked as "used"), when
+  // created using |CreateTrialsFromString()|.
+  enum FieldTrialActivationMode {
+    DONT_ACTIVATE_TRIALS,
+    ACTIVATE_TRIALS,
+  };
+
   // Define a separator character to use when creating a persistent form of an
   // instance.  This is intended for use as a command line argument, passed to a
   // second process to mimic our state (i.e., provide the same group name).
@@ -392,8 +399,10 @@ class BASE_EXPORT FieldTrialList {
   // used in a non-browser process, to carry randomly selected state in a
   // browser process into this non-browser process, but could also be invoked
   // through a command line argument to the browser process. The created field
-  // trials are marked as "used" for the purposes of active trial reporting.
-  static bool CreateTrialsFromString(const std::string& prior_trials);
+  // trials are marked as "used" for the purposes of active trial reporting if
+  // |mode| is ACTIVATE_TRIALS.
+  static bool CreateTrialsFromString(const std::string& prior_trials,
+                                     FieldTrialActivationMode mode);
 
   // Create a FieldTrial with the given |name| and using 100% probability for
   // the FieldTrial, force FieldTrial to have the same group string as
diff --git a/base/metrics/field_trial_unittest.cc b/base/metrics/field_trial_unittest.cc
index 8ab7132..d5bcd9c 100644
--- a/base/metrics/field_trial_unittest.cc
+++ b/base/metrics/field_trial_unittest.cc
@@ -393,7 +393,8 @@ TEST_F(FieldTrialTest, Restore) {
   ASSERT_FALSE(FieldTrialList::TrialExists("Some_name"));
   ASSERT_FALSE(FieldTrialList::TrialExists("xxx"));
 
-  FieldTrialList::CreateTrialsFromString("Some_name/Winner/xxx/yyyy/");
+  FieldTrialList::CreateTrialsFromString("Some_name/Winner/xxx/yyyy/",
+                                         FieldTrialList::DONT_ACTIVATE_TRIALS);
 
   FieldTrial* trial = FieldTrialList::Find("Some_name");
   ASSERT_NE(static_cast<FieldTrial*>(NULL), trial);
@@ -407,12 +408,14 @@ TEST_F(FieldTrialTest, Restore) {
 }
 
 TEST_F(FieldTrialTest, BogusRestore) {
-  EXPECT_FALSE(FieldTrialList::CreateTrialsFromString("MissingSlash"));
-  EXPECT_FALSE(FieldTrialList::CreateTrialsFromString("MissingGroupName/"));
   EXPECT_FALSE(FieldTrialList::CreateTrialsFromString(
-      "MissingFinalSlash/gname"));
+      "MissingSlash", FieldTrialList::DONT_ACTIVATE_TRIALS));
   EXPECT_FALSE(FieldTrialList::CreateTrialsFromString(
-      "noname, only group/"));
+      "MissingGroupName/", FieldTrialList::DONT_ACTIVATE_TRIALS));
+  EXPECT_FALSE(FieldTrialList::CreateTrialsFromString(
+      "MissingFinalSlash/gname", FieldTrialList::DONT_ACTIVATE_TRIALS));
+  EXPECT_FALSE(FieldTrialList::CreateTrialsFromString(
+      "noname, only group/", FieldTrialList::DONT_ACTIVATE_TRIALS));
 }
 
 TEST_F(FieldTrialTest, DuplicateRestore) {
@@ -426,19 +429,44 @@ TEST_F(FieldTrialTest, DuplicateRestore) {
   EXPECT_EQ("Some name/Winner/", save_string);
 
   // It is OK if we redundantly specify a winner.
-  EXPECT_TRUE(FieldTrialList::CreateTrialsFromString(save_string));
+  EXPECT_TRUE(FieldTrialList::CreateTrialsFromString(
+      save_string, FieldTrialList::DONT_ACTIVATE_TRIALS));
 
   // But it is an error to try to change to a different winner.
-  EXPECT_FALSE(FieldTrialList::CreateTrialsFromString("Some name/Loser/"));
+  EXPECT_FALSE(FieldTrialList::CreateTrialsFromString(
+      "Some name/Loser/", FieldTrialList::DONT_ACTIVATE_TRIALS));
+}
+
+TEST_F(FieldTrialTest, CreateTrialsFromStringActive) {
+  ASSERT_FALSE(FieldTrialList::TrialExists("Abc"));
+  ASSERT_FALSE(FieldTrialList::TrialExists("Xyz"));
+  ASSERT_TRUE(FieldTrialList::CreateTrialsFromString(
+      "Abc/def/Xyz/zyx/", FieldTrialList::ACTIVATE_TRIALS));
+
+  FieldTrial::ActiveGroups active_groups;
+  FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
+  ASSERT_EQ(2U, active_groups.size());
+  EXPECT_EQ("Abc", active_groups[0].trial_name);
+  EXPECT_EQ("def", active_groups[0].group_name);
+  EXPECT_EQ("Xyz", active_groups[1].trial_name);
+  EXPECT_EQ("zyx", active_groups[1].group_name);
 }
 
-TEST_F(FieldTrialTest, CreateTrialsFromStringAreActive) {
+TEST_F(FieldTrialTest, CreateTrialsFromStringNotActive) {
   ASSERT_FALSE(FieldTrialList::TrialExists("Abc"));
   ASSERT_FALSE(FieldTrialList::TrialExists("Xyz"));
-  ASSERT_TRUE(FieldTrialList::CreateTrialsFromString("Abc/def/Xyz/zyx/"));
+  ASSERT_TRUE(FieldTrialList::CreateTrialsFromString(
+      "Abc/def/Xyz/zyx/", FieldTrialList::DONT_ACTIVATE_TRIALS));
 
   FieldTrial::ActiveGroups active_groups;
   FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
+  ASSERT_TRUE(active_groups.empty());
+
+  // Check that the values still get returned and querying them activates them.
+  EXPECT_EQ("def", FieldTrialList::FindFullName("Abc"));
+  EXPECT_EQ("zyx", FieldTrialList::FindFullName("Xyz"));
+
+  FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
   ASSERT_EQ(2U, active_groups.size());
   EXPECT_EQ("Abc", active_groups[0].trial_name);
   EXPECT_EQ("def", active_groups[0].group_name);
@@ -446,11 +474,30 @@ TEST_F(FieldTrialTest, CreateTrialsFromStringAreActive) {
   EXPECT_EQ("zyx", active_groups[1].group_name);
 }
 
-TEST_F(FieldTrialTest, CreateTrialsFromStringObserver) {
+TEST_F(FieldTrialTest, CreateTrialsFromStringActiveObserver) {
   ASSERT_FALSE(FieldTrialList::TrialExists("Abc"));
 
   TestFieldTrialObserver observer;
-  ASSERT_TRUE(FieldTrialList::CreateTrialsFromString("Abc/def/"));
+  ASSERT_TRUE(FieldTrialList::CreateTrialsFromString(
+      "Abc/def/", FieldTrialList::ACTIVATE_TRIALS));
+
+  RunLoop().RunUntilIdle();
+  EXPECT_EQ("Abc", observer.trial_name());
+  EXPECT_EQ("def", observer.group_name());
+}
+
+TEST_F(FieldTrialTest, CreateTrialsFromStringNotActiveObserver) {
+  ASSERT_FALSE(FieldTrialList::TrialExists("Abc"));
+
+  TestFieldTrialObserver observer;
+  ASSERT_TRUE(FieldTrialList::CreateTrialsFromString(
+      "Abc/def/", FieldTrialList::DONT_ACTIVATE_TRIALS));
+  RunLoop().RunUntilIdle();
+  // Observer shouldn't be notified.
+  EXPECT_TRUE(observer.trial_name().empty());
+
+  // Check that the values still get returned and querying them activates them.
+  EXPECT_EQ("def", FieldTrialList::FindFullName("Abc"));
 
   RunLoop().RunUntilIdle();
   EXPECT_EQ("Abc", observer.trial_name());