Added a Unique ID for a Field Trial containing it's hashed name and the selected group ID.

base/metrics/field_trial.cc

 // Copyright (c) 2011 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.
 
 #include "base/metrics/field_trial.h"
 
 #include "base/build_time.h"
+#include "base/hash_tables.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "base/sha1.h"
 #include "base/stringprintf.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 
 namespace base {
 
 // static
@@ skipping 113 matching lines @@
   }
   return group_;
 }
 
 std::string FieldTrial::group_name() {
   group();  // call group() to make sure group assignment was done.
   DCHECK(!group_name_.empty());
   return group_name_;
 }
 
+bool FieldTrial::GetUID(UID* uid) {
+  if (group_ == kNotFinalized)
+    return false;
+  *uid = std::make_pair(BASE_HASH_NAMESPACE::hash_value(name_), group_);

[jar (doing other things), 2012/01/24 19:19:45]

IMO, you should consider generating the hash once and for all when the field
trial is created, and leaving it in a slot for the field trial.

[I was also mistakenly worried about races here... but since you're recording
group, and not group name, there is no problem. If you transition to
group_name_, and hashing it, it will be critical to put the hash in a slot so
that you can atomically access it without a race concern vs group_, which is set
asynchronously compared to the group_name_.]

[MAD, 2012/01/24 20:30:24]

Yes, good point, I agree...
Done...

+  return true;
+}
+
 // static
 std::string FieldTrial::MakeName(const std::string& name_prefix,
                                  const std::string& trial_name) {
   std::string big_string(name_prefix);
   big_string.append(1, kHistogramFieldTrialSeparator);
   return big_string.append(FieldTrialList::FindFullName(trial_name));
 }
 
 // static
 void FieldTrial::EnableBenchmarking() {
@@ skipping 114 matching lines @@
     DCHECK_EQ(name.find(kPersistentStringSeparator), std::string::npos);
     DCHECK_EQ(group_name.find(kPersistentStringSeparator), std::string::npos);
     output->append(name);
     output->append(1, kPersistentStringSeparator);
     output->append(group_name);
     output->append(1, kPersistentStringSeparator);
   }
 }
 
 // static
+void FieldTrialList::GetFieldTrialUIDs(std::vector<FieldTrial::UID>* uids) {
+  if (!global_)
+    return;
+  DCHECK(uids->empty());
+  AutoLock auto_lock(global_->lock_);

[jar (doing other things), 2012/01/24 19:19:45]

This is probably ok.  I'm always careful to do as little as possible under a
lock.  Generating the hash does take a bit of time... so the temptation would be
to read the list of FieldTrial instances into a vector, release the lock, and
then process those instances.

Alternatively, if you moved to having a slot for the hashes, then the getter
would be more clearly insignificant.

[MAD, 2012/01/24 20:30:24]

Done...

+
+  for (RegistrationList::iterator it = global_->registered_.begin();
+       it != global_->registered_.end(); ++it) {
+    FieldTrial::UID uid;
+    if (it->second->GetUID(&uid))
+      uids->push_back(uid);
+  }
+}
+
+// static
 bool FieldTrialList::CreateTrialsInChildProcess(
     const std::string& parent_trials) {
   DCHECK(global_);
   if (parent_trials.empty() || !global_)
     return true;
 
   size_t next_item = 0;
   while (next_item < parent_trials.length()) {
     size_t name_end = parent_trials.find(kPersistentStringSeparator, next_item);
     if (name_end == parent_trials.npos || next_item == name_end)
@@ skipping 93 matching lines @@
 }
 
 FieldTrial* FieldTrialList::PreLockedFind(const std::string& name) {
   RegistrationList::iterator it = registered_.find(name);
   if (registered_.end() == it)
     return NULL;
   return it->second;
 }
 
 }  // namespace base