Remove the hash fields from FieldTrials.

chrome/common/metrics/experiments_helper_unittest.cc

Index: chrome/common/metrics/experiments_helper_unittest.cc
diff --git a/chrome/common/metrics/experiments_helper_unittest.cc b/chrome/common/metrics/experiments_helper_unittest.cc
index 546cce86c3ebe5abd84c0a97743933e850876b08..1d5aeb686759119150d5b8998d66781c1cff316c 100644
--- a/chrome/common/metrics/experiments_helper_unittest.cc
+++ b/chrome/common/metrics/experiments_helper_unittest.cc
@@ -18,8 +18,8 @@ namespace {
 // that this will do the group assignment in |trial| if not already done.
 experiments_helper::GoogleExperimentID GetIDForTrial(base::FieldTrial* trial) {
   return experiments_helper::GetGoogleExperimentID(
-      base::FieldTrial::MakeNameGroupId(trial->name(),
-                                        trial->group_name()));
+      experiments_helper::MakeNameGroupId(trial->name(),
+                                          trial->group_name()));
 }
 
 }  // namespace
@@ -49,6 +49,60 @@ class ExperimentsHelperTest : public ::testing::Test {
   scoped_ptr<content::TestBrowserThread> ui_thread_;
 };
 
+TEST_F(ExperimentsHelperTest, HashName) {
+  // Make sure hashing is stable on all platforms.
+  struct {
+    const char* name;
+    uint32 hash_value;
+  } known_hashes[] = {
+    {"a", 937752454u},
+    {"1", 723085877u},
+    {"Trial Name", 2713117220u},
+    {"Group Name", 3201815843u},
+    {"My Favorite Experiment", 3722155194u},
+    {"My Awesome Group Name", 4109503236u},
+    {"abcdefghijklmonpqrstuvwxyz", 787728696u},
+    {"0123456789ABCDEF", 348858318U}
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(known_hashes); ++i) {
+    EXPECT_EQ(known_hashes[i].hash_value,
+              experiments_helper::TestingHashName(known_hashes[i].name));
+  }
+}
+
+TEST_F(ExperimentsHelperTest, GetFieldTrialSelectedGroups) {
+  std::string trial_one("trial one");
+  std::string group_one("group one");
+  std::string trial_two("trial two");
+  std::string group_two("group two");
+
+  experiments_helper::SelectedGroups selected_groups;
+  base::FieldTrial::SelectedGroup selected_group;
+  selected_group.trial = trial_one;
+  selected_group.group = group_one;
+  selected_groups.push_back(selected_group);
+
+  selected_group.trial = trial_two;
+  selected_group.group = group_two;
+  selected_groups.push_back(selected_group);
+
+  // Create our expected hashes.
+  uint32 trial_one_id = experiments_helper::TestingHashName(trial_one);
+  uint32 group_one_id = experiments_helper::TestingHashName(group_one);
+  uint32 trial_two_id = experiments_helper::TestingHashName(trial_two);
+  uint32 group_two_id = experiments_helper::TestingHashName(group_two);
+
+  std::vector<experiments_helper::NameGroupId> name_group_ids;
+  GetFieldTrialNameGroupIdsForSelectedGroups(selected_groups, &name_group_ids);
+  EXPECT_EQ(2U, name_group_ids.size());
+  for (size_t i = 0; i < name_group_ids.size(); ++i) {
+    EXPECT_TRUE(trial_one_id != name_group_ids[i].name ||

[MAD, 2012/04/25 20:03:22]

This doesn't give an error if the returned vector has the same set of values
twice...

[SteveT, 2012/04/26 19:03:31]

Right. Going to do this with a set, which requires me to expose my
NameGroupIdCompare helper class in experiments_helper.h. Is it okay that I
inlined the implementation there or should I move it to experiments_helper.cc?

[MAD, 2012/04/26 19:40:12]

You could also create a C style array on the stack, assert that the
name_group_ids has the same size, and then walk the C style, looking for the
value in the vector, as opposed to the other way around...

+                group_one_id == name_group_ids[i].group);
+    EXPECT_TRUE(trial_two_id != name_group_ids[i].name ||
+                group_two_id == name_group_ids[i].group);
+  }
+}
+
 // Test that if the trial is immediately disabled, GetGoogleExperimentID just
 // returns the empty ID.
 TEST_F(ExperimentsHelperTest, DisableImmediately) {
@@ -76,9 +130,10 @@ TEST_F(ExperimentsHelperTest, DisableAfterInitialization) {
                                                  next_year_, 12, 12, NULL));
   trial->AppendGroup(non_default_name, 100);
   experiments_helper::AssociateGoogleExperimentID(
-      base::FieldTrial::MakeNameGroupId(trial->name(), default_name), 123);
+      experiments_helper::MakeNameGroupId(trial->name(), default_name), 123);
   experiments_helper::AssociateGoogleExperimentID(
-      base::FieldTrial::MakeNameGroupId(trial->name(), non_default_name), 456);
+      experiments_helper::MakeNameGroupId(trial->name(), non_default_name),
+      456);
   ASSERT_EQ(non_default_name, trial->group_name());
   ASSERT_EQ(456U, GetIDForTrial(trial.get()));
   trial->Disable();
@@ -97,10 +152,10 @@ TEST_F(ExperimentsHelperTest, AssociateGoogleExperimentID) {
 
   // Set GoogleExperimentIDs so we can verify that they were chosen correctly.
   experiments_helper::AssociateGoogleExperimentID(
-      base::FieldTrial::MakeNameGroupId(trial_true->name(), default_name1),
+      experiments_helper::MakeNameGroupId(trial_true->name(), default_name1),
       123);
   experiments_helper::AssociateGoogleExperimentID(
-      base::FieldTrial::MakeNameGroupId(trial_true->name(), winner),
+      experiments_helper::MakeNameGroupId(trial_true->name(), winner),
       456);
 
   EXPECT_EQ(winner_group, trial_true->group());
@@ -115,10 +170,10 @@ TEST_F(ExperimentsHelperTest, AssociateGoogleExperimentID) {
   int loser_group = trial_false->AppendGroup(loser, 0);
 
   experiments_helper::AssociateGoogleExperimentID(
-      base::FieldTrial::MakeNameGroupId(trial_false->name(), default_name2),
+      experiments_helper::MakeNameGroupId(trial_false->name(), default_name2),
       123);
   experiments_helper::AssociateGoogleExperimentID(
-      base::FieldTrial::MakeNameGroupId(trial_false->name(), loser),
+      experiments_helper::MakeNameGroupId(trial_false->name(), loser),
       456);
 
   EXPECT_NE(loser_group, trial_false->group());