Remove the hash fields from FieldTrials.

chrome/common/metrics/experiments_helper.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_COMMON_METRICS_EXPERIMENTS_HELPER_H_
 #define CHROME_COMMON_METRICS_EXPERIMENTS_HELPER_H_
 #pragma once
 
 #include "base/metrics/field_trial.h"
 
@@ skipping 10 matching lines @@
 //
 // Typical usage looks like this:
 //
 // // Set up your trial and groups as usual.
 // FieldTrial* trial = FieldTrialList::FactoryGetFieldTrial(
 //     "trial", 1000, "default", 2012, 12, 31, NULL);
 // const int kHighMemGroup = trial->AppendGroup("HighMem", 20);
 // const int kLowMemGroup = trial->AppendGroup("LowMem", 20);
 // // All groups are now created. We want to associate GoogleExperimentIDs with
 // // them, so do that now.
-// AssociateGoogleExperimentID(
-//     FieldTrial::MakeNameGroupId("trial", "default"), 123);
-// AssociateGoogleExperimentID(
-//     FieldTrial::MakeNameGroupId("trial", "HighMem"), 456);
-// AssociateGoogleExperimentID(
-//     FieldTrial::MakeNameGroupId("trial", "LowMem"), 789);
+// AssociateGoogleExperimentID(MakeNameGroupId("trial", "default"), 123);
+// AssociateGoogleExperimentID(MakeNameGroupId("trial", "HighMem"), 456);
+// AssociateGoogleExperimentID(MakeNameGroupId("trial", "LowMem"), 789);
 //
 // // Elsewhere, we are interested in retrieving the GoogleExperimentID
 // // assocaited with |trial|.
 // GoogleExperimentID id = GetGoogleExperimentID(
-//     FieldTrial::MakeNameGroupId(trial->name(), trial->group_name()));
+//     MakeNameGroupId(trial->name(), trial->group_name()));
 // // Do stuff with |id|...
 //
 // The AssociateGoogleExperimentID and GetGoogleExperimentID API methods are
 // thread safe.
 namespace experiments_helper {
 
 // An ID used by Google servers to identify a local browser experiment.
 typedef uint32 GoogleExperimentID;
 
+// The Unique ID of a trial, where the name and group identifiers are
+// hashes of the trial and group name strings.
+struct NameGroupId {
+  uint32 name;
+  uint32 group;
+};
+
+// We need to supply a Compare class for templates since NameGroupId is a
+// user-defined type.
+struct NameGroupIdCompare {
+  bool operator() (const experiments_helper::NameGroupId& lhs,

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

I don't think you need the namespace scope in here, since we are within it
now...

[SteveT, 2012/04/26 21:06:03]

Yup, thanks for the catch :)

Oh, and you're okay with me in-lining this utility struct, yeah?

+                   const experiments_helper::NameGroupId& rhs) const {
+    // The group and name fields are just SHA-1 Hashes, so we just need to treat
+    // them as IDs and do a less-than comparison. We test group first, since
+    // name is more likely to collide.
+    return lhs.group == rhs.group ? lhs.name < rhs.name :
+        lhs.group < rhs.group;
+  }
+};
+
 // Used to represent no associated Google experiment ID. Calls to the
 // GetGoogleExperimentID API below will return this empty value for FieldTrial
 // groups uninterested in associating themselves with Google experiments, or
 // those that have not yet been seen yet.
 extern const GoogleExperimentID kEmptyGoogleExperimentID;
 
+// Returns an array of Unique IDs for each Field Trial that has a chosen
+// group. Field Trials for which a group has not been chosen yet are NOT
+// returned in this list. In the real world, clients should use
+// GetFieldTrialNameGroupIds.
+void GetFieldTrialNameGroupIds(std::vector<NameGroupId>* name_group_ids);
+
+// Helper function to create a NameGroupId from |trial_name| and |group_name|.
+NameGroupId MakeNameGroupId(const std::string& trial_name,
+                            const std::string& group_name);
+
 // Set the GoogleExperimentID associated with a FieldTrial group. The group is
 // denoted by |group_identifier|, which can be created by passing the
 // FieldTrial's trial and group names to base::FieldTrial::MakeNameGroupId.
 // This does not need to be called for FieldTrials uninterested in Google
 // experiments.
-void AssociateGoogleExperimentID(
-    const base::FieldTrial::NameGroupId& group_identifier,
-    GoogleExperimentID id);
+void AssociateGoogleExperimentID(const NameGroupId& group_identifier,
+                                 GoogleExperimentID id);
 
 // Retrieve the GoogleExperimentID associated with a FieldTrial group. The group
 // is denoted by |group_identifier| (see comment above). This can be nicely
 // combined with FieldTrial::GetFieldTrialNameGroupIds to enumerate the
 // GoogleExperimentIDs for all active FieldTrial groups.
-GoogleExperimentID GetGoogleExperimentID(
-    const base::FieldTrial::NameGroupId& group_identifier);
+GoogleExperimentID GetGoogleExperimentID(const NameGroupId& group_identifier);
 
 // Get the current set of chosen FieldTrial groups (aka experiments) and send
 // them to the child process logging module so it can save it for crash dumps.
 void SetChildProcessLoggingExperimentList();
 
 }  // namespace experiments_helper
 
+// Expose a couple functions for testing. These functions will just wrap
+// functionality that is implemented above.
+namespace testing {
+
+void TestGetFieldTrialNameGroupIdsForSelectedGroups(
+    const base::FieldTrial::SelectedGroups& selected_groups,
+    std::vector<experiments_helper::NameGroupId>* name_group_ids);
+
+experiments_helper::NameGroupId TestMakeNameGroupId(

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

I don't think you need TestMakeNameGroupId since the method is available
above... Unless I missed something???

[SteveT, 2012/04/26 21:06:03]

We discussed changing the API to accept strings and using HashNameIds. I wanted
to put off doing this, but I think it makes good sense :)  It makes the API
simpler, and also resolves some of these issues with where MakeNameGroupId is
exposed at the same time.

+    const std::string& trial_name,
+    const std::string& group_name);
+
+uint32 TestHashName(const std::string& name);
+
+}
+
 #endif  // CHROME_COMMON_METRICS_EXPERIMENTS_HELPER_H_