Actually update FieldTrialEntry's activated field

base/metrics/field_trial.cc

Index: base/metrics/field_trial.cc
diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index 911c8e5ee9842759c3c820929e268098e6a6dbd6..8ecfd0163f2aae51ee87c423cfd3ae3713d8cb87 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -309,7 +309,8 @@ FieldTrial::FieldTrial(const std::string& trial_name,
       enable_field_trial_(true),
       forced_(false),
       group_reported_(false),
-      trial_registered_(false) {
+      trial_registered_(false),
+      ref_(SharedPersistentMemoryAllocator::kReferenceNull) {
   DCHECK_GT(total_probability, 0);
   DCHECK(!trial_name_.empty());
   DCHECK(!default_group_name_.empty());
@@ -340,6 +341,10 @@ void FieldTrial::FinalizeGroupChoice() {
   // finalized.
   DCHECK(!forced_);
   SetGroupChoice(default_group_name_, kDefaultGroupNumber);
+
+  // Add the field trial to shared memory.
+  if (kUseSharedMemoryForFieldTrials)
+    FieldTrialList::OnGroupFinalized(this);
 }
 
 bool FieldTrial::GetActiveGroup(ActiveGroup* active_group) const {
@@ -772,6 +777,14 @@ void FieldTrialList::RemoveObserver(Observer* observer) {
 }
 
 // static
+void FieldTrialList::OnGroupFinalized(FieldTrial* field_trial) {
+  if (!global_)
+    return;
+  AutoLock auto_lock(global_->lock_);
+  AddToAllocatorWhileLocked(field_trial);
+}
+
+// static
 void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
   if (!global_)
     return;
@@ -785,10 +798,8 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
     if (!field_trial->enable_field_trial_)
       return;
 
-    if (kUseSharedMemoryForFieldTrials) {
-      field_trial->AddToAllocatorWhileLocked(
-          global_->field_trial_allocator_.get());
-    }
+    if (kUseSharedMemoryForFieldTrials)
+      ActivateFieldTrialEntry(field_trial);
   }
 
   global_->observer_list_->Notify(
@@ -796,47 +807,84 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
       field_trial->trial_name(), field_trial->group_name_internal());
 }
 
+// static
+void FieldTrialList::ActivateFieldTrialEntry(FieldTrial* field_trial) {

[Alexei Svitkine (slow), 2016/10/26 16:21:08]

Nit: Please order the functions the same way as in the .h file.

[lawrencewu, 2016/10/26 16:35:24]

Done.

+  SharedPersistentMemoryAllocator* allocator =
+      global_->field_trial_allocator_.get();
+  SharedPersistentMemoryAllocator::Reference ref = field_trial->ref_;
+  if (ref == SharedPersistentMemoryAllocator::kReferenceNull) {
+    // It's fine to do this even if the allocator hasn't been instantiated
+    // yet -- it'll just return early.
+    AddToAllocatorWhileLocked(field_trial);
+  } else {
+    // It's also okay to do this even though the callee doesn't have a lock --
+    // the only thing that happens on a stale read here is a slight performance
+    // hit from the child re-synchronizing activation state.
+    FieldTrialEntry* entry =
+        allocator->GetAsObject<FieldTrialEntry>(ref, kFieldTrialType);
+    entry->activated = true;
+  }
+}
+
 #if !defined(OS_NACL)
 // static
 void FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded() {
-  AutoLock auto_lock(global_->lock_);
-  // Create the allocator if not already created and add all existing trials.
-  if (global_->field_trial_allocator_ != nullptr)
+  if (!global_)
     return;
+  {

[Alexei Svitkine (slow), 2016/10/26 16:21:08]

Nit: No need for this block since the auto_lock applies to everything below it
already.

[lawrencewu, 2016/10/26 16:35:24]

Fixed.

+    AutoLock auto_lock(global_->lock_);
+    // Create the allocator if not already created and add all existing trials.
+    if (global_->field_trial_allocator_ != nullptr)
+      return;
 
-  std::unique_ptr<SharedMemory> shm(new SharedMemory());
-  if (!shm->CreateAndMapAnonymous(kFieldTrialAllocationSize))
-    TerminateBecauseOutOfMemory(kFieldTrialAllocationSize);
+    std::unique_ptr<SharedMemory> shm(new SharedMemory());
+    if (!shm->CreateAndMapAnonymous(kFieldTrialAllocationSize))
+      TerminateBecauseOutOfMemory(kFieldTrialAllocationSize);
 
-  // TODO(lawrencewu): call UpdateTrackingHistograms() when all field trials
-  // have been registered (perhaps in the destructor?)
-  global_->field_trial_allocator_.reset(new SharedPersistentMemoryAllocator(
-      std::move(shm), 0, kAllocatorName, false));
-  global_->field_trial_allocator_->CreateTrackingHistograms(kAllocatorName);
+    // TODO(lawrencewu): call UpdateTrackingHistograms() when all field trials
+    // have been registered (perhaps in the destructor?)
+    global_->field_trial_allocator_.reset(new SharedPersistentMemoryAllocator(
+        std::move(shm), 0, kAllocatorName, false));
+    global_->field_trial_allocator_->CreateTrackingHistograms(kAllocatorName);
 
-  // Add all existing field trials.
-  for (const auto& registered : global_->registered_) {
-    registered.second->AddToAllocatorWhileLocked(
-        global_->field_trial_allocator_.get());
-  }
+    // Add all existing field trials.
+    for (const auto& registered : global_->registered_) {
+      AddToAllocatorWhileLocked(registered.second);
+    }
 
 #if defined(OS_WIN)
-  // Set |readonly_allocator_handle_| so we can pass it to be inherited and via
-  // the command line.
-  global_->readonly_allocator_handle_ =
-      CreateReadOnlyHandle(global_->field_trial_allocator_.get());
+    // Set |readonly_allocator_handle_| so we can pass it to be inherited and
+    // via the command line.
+    global_->readonly_allocator_handle_ =
+        CreateReadOnlyHandle(global_->field_trial_allocator_.get());
 #endif
+  }
 }
 #endif
 
-void FieldTrial::AddToAllocatorWhileLocked(
-    SharedPersistentMemoryAllocator* allocator) {
+// static
+size_t FieldTrialList::GetFieldTrialCount() {
+  if (!global_)
+    return 0;
+  AutoLock auto_lock(global_->lock_);
+  return global_->registered_.size();
+}
+
+// static
+void FieldTrialList::AddToAllocatorWhileLocked(FieldTrial* field_trial) {
+  SharedPersistentMemoryAllocator* allocator =
+      global_->field_trial_allocator_.get();
+
   // Don't do anything if the allocator hasn't been instantiated yet.
   if (allocator == nullptr)
     return;
 
-  State trial_state;
-  if (!GetState(&trial_state))
+  // Or if we've already added it.
+  if (field_trial->ref_ != SharedPersistentMemoryAllocator::kReferenceNull)
+    return;
+
+  FieldTrial::State trial_state;
+  if (!field_trial->GetState(&trial_state))
     return;
 
   size_t trial_name_size = trial_state.trial_name.size() + 1;
@@ -869,14 +917,7 @@ void FieldTrial::AddToAllocatorWhileLocked(
   group_name[trial_state.group_name.size()] = '\0';
 
   allocator->MakeIterable(ref);
-}
-
-// static
-size_t FieldTrialList::GetFieldTrialCount() {
-  if (!global_)
-    return 0;
-  AutoLock auto_lock(global_->lock_);
-  return global_->registered_.size();
+  field_trial->ref_ = ref;
 }
 
 // static