Use SharedPersistentMemoryAllocator to share field trial state

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 4f3e0c76fcd58c606f3b98f90718f8dc63538d5f..447e31dc883c15fe70a432c38700bc6cf77b6534 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
+#include "base/process/memory.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -35,9 +36,37 @@ const char kActivationMarker = '*';
 // for now while the implementation is fleshed out (e.g. data format, single
 // shared memory segment). See https://codereview.chromium.org/2365273004/ and
 // crbug.com/653874
-#if defined(OS_WIN)
 const bool kUseSharedMemoryForFieldTrials = false;
-#endif
+
+// Constants for the field trial allocator.
+const char kAllocatorName[] = "FieldTrialAllocator";
+const uint32_t kFieldTrialType = 0xABA17E13 + 1;  // SHA1(FieldTrialEntry) v1
+
+// We create one FieldTrialEntry struct per field trial in shared memory (via
+// field_trial_allocator_). It contains whether or not it's activated, and the
+// offset from the start of the allocated memory to the group name. We don't
+// need the offset into the trial name because that's always a fixed position
+// from the start of the struct. Two strings will be appended to the end of this
+// structure in allocated memory, so the result will look like this:
+// ---------------------------------
+// | fte | trial_name | group_name |
+// ---------------------------------
+struct FieldTrialEntry {
+  bool activated;

[Alexei Svitkine (slow), 2016/10/24 14:40:27]

I don't think your current code updates this when the trial gets activated -
which is the whole point of keeping it here.

To do that, you probably need to keep a pointer to FTE from each FieldTrial
object - or else have to look it up each time.

I'm OK with you putting a TODO about it here and addressing it in a follow-up CL
- given that we're not turning this on yet.

[lawrencewu, 2016/10/24 15:35:13]

I added a comment and will tackle this in the next CL.

+  uint32_t group_name_offset;
+
+  const char* GetTrialName() const {
+    const char* src =
+        reinterpret_cast<char*>(const_cast<FieldTrialEntry*>(this));
+    return src + sizeof(FieldTrialEntry);
+  }
+
+  const char* GetGroupName() const {
+    const char* src =
+        reinterpret_cast<char*>(const_cast<FieldTrialEntry*>(this));
+    return src + this->group_name_offset;
+  }
+};
 
 // Created a time value based on |year|, |month| and |day_of_month| parameters.
 Time CreateTimeFromParams(int year, int month, int day_of_month) {
@@ -124,11 +153,33 @@ bool ParseFieldTrialsString(const std::string& trials_string,
   return true;
 }
 
+void AddForceFieldTrialsFlag(base::CommandLine* cmd_line) {
+  std::string field_trial_states;
+  FieldTrialList::AllStatesToString(&field_trial_states);
+  if (!field_trial_states.empty()) {
+    cmd_line->AppendSwitchASCII(switches::kForceFieldTrials,
+                                field_trial_states);
+  }
+}
+
+#if defined(OS_WIN)
+HANDLE CreateReadOnlyHandle(base::SharedPersistentMemoryAllocator* allocator) {
+  HANDLE src = allocator->shared_memory()->handle().GetHandle();
+  ProcessHandle process = GetCurrentProcess();
+  DWORD access = SECTION_MAP_READ | SECTION_QUERY;
+  HANDLE dst;
+  if (!::DuplicateHandle(process, src, process, &dst, access, true, 0))
+    return NULL;

[Alexei Svitkine (slow), 2016/10/24 14:40:27]

Don't use NULL in new code.

Use nullptr - unless there's a different convention for HANDLE values. Also, be
consistent with what you have in the header file for readonly_allocator_handle_
- where you currently use 0.

[lawrencewu, 2016/10/24 15:35:13]

Used nullptr and initialized it in the header file to nullptr too.

+  return dst;
+}
+#endif
+
 }  // namespace
 
 // statics
 const int FieldTrial::kNotFinalized = -1;
 const int FieldTrial::kDefaultGroupNumber = 0;
+const int kFieldTrialAllocationSize = 4 << 10;  // 4 KiB = one page
 bool FieldTrial::enable_benchmarking_ = false;
 
 int FieldTrialList::kNoExpirationYear = 0;
@@ -586,14 +637,13 @@ void FieldTrialList::CreateTrialsFromCommandLine(
         base::SharedMemoryHandle(handle, base::GetCurrentProcId());
 
     // Gets deleted when it gets out of scope, but that's OK because we need it
-    // only for the duration of this call currently anyway.
-    base::SharedMemory shared_memory(shm_handle, false);
-    shared_memory.Map(field_trial_length);
+    // only for the duration of this method.
+    std::unique_ptr<base::SharedMemory> shm(
+        new base::SharedMemory(shm_handle, true));
+    if (!shm.get()->Map(field_trial_length))
+      base::TerminateBecauseOutOfMemory(field_trial_length);
 
-    char* field_trial_state = static_cast<char*>(shared_memory.memory());
-    bool result = FieldTrialList::CreateTrialsFromString(
-        std::string(field_trial_state), std::set<std::string>());
-    DCHECK(result);
+    FieldTrialList::CreateTrialsFromSharedMemory(std::move(shm));
     return;
   }
 #endif
@@ -606,37 +656,71 @@ void FieldTrialList::CreateTrialsFromCommandLine(
   }
 }
 
+#if defined(OS_WIN)
+// static
+void FieldTrialList::AppendFieldTrialHandleIfNeeded(
+    base::HandlesToInheritVector* handles) {
+  handles->push_back(global_->readonly_allocator_handle_);

[Alexei Svitkine (slow), 2016/10/24 14:40:27]

You only want to do this if it's no null, right?

[lawrencewu, 2016/10/24 15:35:13]

Yep, done.

+}
+#endif
+
+// static
+void FieldTrialList::CreateTrialsFromSharedMemory(
+    std::unique_ptr<base::SharedMemory> shm) {
+  const base::SharedPersistentMemoryAllocator shalloc(std::move(shm), 0,
+                                                      kAllocatorName, true);
+  base::PersistentMemoryAllocator::Iterator iter(&shalloc);
+
+  base::SharedPersistentMemoryAllocator::Reference ref;
+  while ((ref = iter.GetNextOfType(kFieldTrialType)) !=
+         base::SharedPersistentMemoryAllocator::kReferenceNull) {
+    const FieldTrialEntry* entry =
+        shalloc.GetAsObject<const FieldTrialEntry>(ref, kFieldTrialType);
+    FieldTrial* trial =
+        CreateFieldTrial(entry->GetTrialName(), entry->GetGroupName());
+
+    if (entry->activated) {
+      // Call |group()| to mark the trial as "used" and notify observers, if
+      // any. This is useful to ensure that field trials created in child
+      // processes are properly reported in crash reports.
+      trial->group();
+    }
+  }
+}
+
 // static
-std::unique_ptr<base::SharedMemory> FieldTrialList::CopyFieldTrialStateToFlags(
+void FieldTrialList::CopyFieldTrialStateToFlags(
     const char* field_trial_handle_switch,
     base::CommandLine* cmd_line) {
-  std::string field_trial_states;
-  base::FieldTrialList::AllStatesToString(&field_trial_states);
-  if (!field_trial_states.empty()) {
-// Use shared memory to pass the state if the feature is enabled, otherwise
-// fallback to passing it via the command line as a string.
 #if defined(OS_WIN)
-    if (kUseSharedMemoryForFieldTrials) {
-      std::unique_ptr<base::SharedMemory> shm(new base::SharedMemory());
-      size_t length = field_trial_states.size() + 1;
-      shm->CreateAndMapAnonymous(length);
-      memcpy(shm->memory(), field_trial_states.c_str(), length);
-
-      // HANDLE is just typedef'd to void *
-      auto uintptr_handle =
-          reinterpret_cast<std::uintptr_t>(shm->handle().GetHandle());
-      std::string field_trial_handle =
-          std::to_string(uintptr_handle) + "," + std::to_string(length);
-
-      cmd_line->AppendSwitchASCII(field_trial_handle_switch,
-                                  field_trial_handle);
-      return shm;
+  // Use shared memory to pass the state if the feature is enabled, otherwise
+  // fallback to passing it via the command line as a string.
+  if (kUseSharedMemoryForFieldTrials) {
+    FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded();
+    // If the readonly handle didn't get duplicated properly, then fallback to
+    // original behavior.
+    if (!global_->readonly_allocator_handle_) {
+      AddForceFieldTrialsFlag(cmd_line);
+      return;
     }
-#endif
-    cmd_line->AppendSwitchASCII(switches::kForceFieldTrials,
-                                field_trial_states);
+
+    // HANDLE is just typedef'd to void *. We basically cast the handle into an
+    // int (uintptr_t, to be exact), stringify the int, and pass it as a
+    // command-line flag. The child process will do the reverse conversions to
+    // retrieve the handle. See http://stackoverflow.com/a/153077
+    auto uintptr_handle =
+        reinterpret_cast<uintptr_t>(global_->readonly_allocator_handle_);
+    size_t field_trial_length =
+        global_->field_trial_allocator_->shared_memory()->mapped_size();
+    std::string field_trial_handle = std::to_string(uintptr_handle) + "," +
+                                     std::to_string(field_trial_length);
+
+    cmd_line->AppendSwitchASCII(field_trial_handle_switch, field_trial_handle);
+    return;
   }
-  return std::unique_ptr<base::SharedMemory>(nullptr);
+#endif
+
+  AddForceFieldTrialsFlag(cmd_line);
 }
 
 // static
@@ -689,10 +773,14 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
     if (field_trial->group_reported_)
       return;
     field_trial->group_reported_ = true;
-  }
 
-  if (!field_trial->enable_field_trial_)
-    return;
+    if (!field_trial->enable_field_trial_)
+      return;
+
+    if (kUseSharedMemoryForFieldTrials)
+      field_trial->AddToAllocatorWhileLocked(
+          global_->field_trial_allocator_.get());
+  }
 
   global_->observer_list_->Notify(
       FROM_HERE, &FieldTrialList::Observer::OnFieldTrialGroupFinalized,
@@ -700,6 +788,73 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
 }
 
 // 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)
+    return;
+
+  std::unique_ptr<base::SharedMemory> shm(new base::SharedMemory());
+  if (!shm->CreateAndMapAnonymous(kFieldTrialAllocationSize))
+    base::TerminateBecauseOutOfMemory(kFieldTrialAllocationSize);
+
+  // TODO(lawrencewu): call UpdateTrackingHistograms() when all field trials
+  // have been registered (perhaps in the destructor?)
+  global_->field_trial_allocator_.reset(
+      new base::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());
+  }
+
+#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());
+#endif
+}
+
+void FieldTrial::AddToAllocatorWhileLocked(
+    base::SharedPersistentMemoryAllocator* allocator) {
+  // Don't do anything if the allocator hasn't been instantiated yet.
+  if (allocator == nullptr)
+    return;
+
+  // We allocate just enough memory to fit the FTE struct, trial name, and group
+  // name. If there's no more memory available, quit early.
+  State trial_state;
+  GetState(&trial_state);

[Alexei Svitkine (slow), 2016/10/24 14:40:27]

This function has a return value - if it's false, |trial_state| doesn't get
filled in.

You want to do an early return if it's false, probably with a comment.

[lawrencewu, 2016/10/24 15:35:13]

Done.

+
+  size_t trial_name_size = trial_state.trial_name.size() + 1;
+  size_t group_name_size = trial_state.group_name.size() + 1;
+
+  uint32_t trial_name_offset = sizeof(FieldTrialEntry);
+  uint32_t group_name_offset = trial_name_offset + trial_name_size;
+  size_t size = sizeof(FieldTrialEntry) + trial_name_size + group_name_size;
+
+  base::SharedPersistentMemoryAllocator::Reference ref =
+      allocator->Allocate(size, kFieldTrialType);
+  if (ref == base::SharedPersistentMemoryAllocator::kReferenceNull)
+    return;
+
+  FieldTrialEntry* entry =
+      allocator->GetAsObject<FieldTrialEntry>(ref, kFieldTrialType);
+  entry->activated = trial_state.activated;
+  entry->group_name_offset = group_name_offset;
+  memcpy(reinterpret_cast<char*>(entry) + trial_name_offset,
+         trial_state.trial_name.data(), trial_name_size);
+  memcpy(reinterpret_cast<char*>(entry) + group_name_offset,
+         trial_state.group_name.data(), group_name_size);
+
+  allocator->MakeIterable(ref);
+}
+
+// static
 size_t FieldTrialList::GetFieldTrialCount() {
   if (!global_)
     return 0;