Fix data race when setting FieldTrial::group_reported_.

Per the bug, the issue is that |group_reported_| gets set by a call to
group() while another thread may be iterating over trials and reading
these values via GetActiveFieldTrialGroups().

Since the value is read by another thread and we don't set it atomically
or under a lock, other threads may see a stale value for this variable.

This change refactors the code such that setting |group_reported_| is
also done while holding the lock.

BUG=164349
TEST=Data race failures in this code disappear

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

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

2 files changed, 17 insertions, 13 deletions

diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index aa3427a..14e16ea 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -144,11 +144,7 @@ int FieldTrial::AppendGroup(const std::string& name,
 
 int FieldTrial::group() {
   FinalizeGroupChoice();
-  if (!group_reported_) {
-    if (enable_field_trial_)
-      FieldTrialList::NotifyFieldTrialGroupSelection(trial_name_, group_name_);
-    group_reported_ = true;
-  }
+  FieldTrialList::NotifyFieldTrialGroupSelection(this);
   return group_;
 }
 
@@ -438,15 +434,24 @@ void FieldTrialList::RemoveObserver(Observer* observer) {
 }
 
 // static
-void FieldTrialList::NotifyFieldTrialGroupSelection(
-    const std::string& trial_name,
-    const std::string& group_name) {
+void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
   if (!global_)
     return;
+
+  {
+    AutoLock auto_lock(global_->lock_);
+    if (field_trial->group_reported_)
+      return;
+    field_trial->group_reported_ = true;
+  }
+
+  if (!field_trial->enable_field_trial_)
+    return;
+
   global_->observer_list_->Notify(
       &FieldTrialList::Observer::OnFieldTrialGroupFinalized,
-      trial_name,
-      group_name);
+      field_trial->trial_name(),
+      field_trial->group_name_internal());
 }
 
 // static
diff --git a/base/metrics/field_trial.h b/base/metrics/field_trial.h
index 556ee40..007c1da 100644
--- a/base/metrics/field_trial.h
+++ b/base/metrics/field_trial.h
@@ -394,9 +394,8 @@ class BASE_EXPORT FieldTrialList {
   // Remove an observer.
   static void RemoveObserver(Observer* observer);
 
-  // Notify all observers that a group is finalized for the named Trial.
-  static void NotifyFieldTrialGroupSelection(const std::string& trial_name,
-                                             const std::string& group_name);
+  // Notify all observers that a group has been finalized for |field_trial|.
+  static void NotifyFieldTrialGroupSelection(FieldTrial* field_trial);
 
   // Return the number of active field trials.
   static size_t GetFieldTrialCount();