Use SharedPersistentMemoryAllocator to share field trial state

base/metrics/field_trial.h

Index: base/metrics/field_trial.h
diff --git a/base/metrics/field_trial.h b/base/metrics/field_trial.h
index 99e705a1fa82dd80a34701cbad08e857c589d9f7..7aa900fb23f5aca7d0237b74cb33612b24e2994d 100644
--- a/base/metrics/field_trial.h
+++ b/base/metrics/field_trial.h
@@ -69,7 +69,9 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/shared_memory.h"
+#include "base/metrics/persistent_memory_allocator.h"
 #include "base/observer_list_threadsafe.h"
+#include "base/process/launch.h"
 #include "base/strings/string_piece.h"
 #include "base/synchronization/lock.h"
 #include "base/time/time.h"
@@ -479,14 +481,22 @@ class BASE_EXPORT FieldTrialList {
       const base::CommandLine& cmd_line,
       const char* field_trial_handle_switch);
 
+#if defined(OS_WIN)
+  static void AppendFieldTrialHandleIfNeeded(

[Alexei Svitkine (slow), 2016/10/21 18:20:26]

Add a comment, please.

[lawrencewu, 2016/10/21 20:23:13]

Done.

+      base::HandlesToInheritVector* handles);
+#endif
+
   // Adds a switch to the command line containing the field trial state as a
   // string (if not using shared memory to share field trial state), or the
   // shared memory handle + length.
   // Needs the |field_trial_handle_switch| argument to be passed in since base/
   // can't depend on content/.
-  static std::unique_ptr<base::SharedMemory> CopyFieldTrialStateToFlags(
-      const char* field_trial_handle_switch,
-      base::CommandLine* cmd_line);
+  static void CopyFieldTrialStateToFlags(const char* field_trial_handle_switch,
+                                         base::CommandLine* cmd_line);
+
+  // Returns a pointer to the global field trial allocator object for testing.
+  static base::SharedPersistentMemoryAllocator*
+  GetFieldTrialAllocatorForTesting();
 
   // Create a FieldTrial with the given |name| and using 100% probability for
   // the FieldTrial, force FieldTrial to have the same group string as
@@ -512,6 +522,28 @@ class BASE_EXPORT FieldTrialList {
   static size_t GetFieldTrialCount();
 
  private:
+  // Allow tests to access our innards for testing purposes.
+  FRIEND_TEST_ALL_PREFIXES(FieldTrialListTest, InstantiateAllocator);
+  FRIEND_TEST_ALL_PREFIXES(FieldTrialListTest, AddTrialsToAllocator);
+
+  // Expects a mapped piece of shared memory |shm| that was created from the
+  // browser process's field_trial_allocator and shared via the command line.
+  // This function recreates the allocator, iterates through all the field
+  // trials in it, and creates them via CreateFieldTrial().
+  static void CreateTrialsFromSharedMemory(
+      std::unique_ptr<base::SharedMemory> shm);
+
+  // Instantiate the field trial allocator, add all existing field trials to it,
+  // and duplicates its handle to a read-only handle, which gets stored in
+  // |readonly_allocator_handle|.
+  static void InstantiateFieldTrialAllocatorIfNeeded();
+
+  // Called in NotifyFieldTrialGroupSelection() and afterinstantiating the

[Alexei Svitkine (slow), 2016/10/21 18:20:26]

Nit: I suggest removing the first sentence - the comment should mention what the
function does and where it's called from is already obvious from the rest of the
code.

[lawrencewu, 2016/10/21 20:23:13]

Done.

+  // allocator. Updates field_trial_allocator by adding the |field_trial|. Since
+  // the check for the allocator is not thread-safe, the caller must wrap this
+  // call in a lock.
+  static void UpdateFieldTrialAllocatorWhileLocked(FieldTrial* field_trial);
+
   // A map from FieldTrial names to the actual instances.
   typedef std::map<std::string, FieldTrial*> RegistrationMap;
 
@@ -549,6 +581,17 @@ class BASE_EXPORT FieldTrialList {
   // List of observers to be notified when a group is selected for a FieldTrial.
   scoped_refptr<ObserverListThreadSafe<Observer> > observer_list_;
 
+  // Allocator used to instantiate field trial in child processes. In the
+  // future, we may want to move this to a more generic place if we want to
+  // start passing more data other than field trials.
+  std::unique_ptr<base::SharedPersistentMemoryAllocator> field_trial_allocator;

[Alexei Svitkine (slow), 2016/10/21 18:20:26]

Add _ at the end

[lawrencewu, 2016/10/21 20:23:13]

Done.

+
+#if defined(OS_WIN)
+  // Readonly copy of the handle to the allocator, stored here since we need it
+  // twice.

[Alexei Svitkine (slow), 2016/10/21 18:20:26]

"stored here since we need it twice" is very vague and doesn't tell me what it's
talking about.

Be more explicit, e.g.: "Needs to be a member variable because it's needed from
both CopyFieldTrialStateToFlags() and AppendFieldTrialHandleIfNeeded()."

[lawrencewu, 2016/10/21 20:23:13]

Done.

+  HANDLE readonly_allocator_handle;

[Alexei Svitkine (slow), 2016/10/21 18:20:26]

Add _ at the end

[lawrencewu, 2016/10/21 20:23:13]

Done.

+#endif
+
   DISALLOW_COPY_AND_ASSIGN(FieldTrialList);
 };