Add field trial dump and retrieval methods from shared memory

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 7fedb375ae43ecebdafc941fcdcd257ff6faf57a..92bf419c94796362ac4b91966e894438993f9604 100644
--- a/base/metrics/field_trial.h
+++ b/base/metrics/field_trial.h
@@ -73,6 +73,7 @@
 #include "base/memory/shared_memory.h"
 #include "base/metrics/persistent_memory_allocator.h"
 #include "base/observer_list_threadsafe.h"
+#include "base/pickle.h"
 #include "base/process/launch.h"
 #include "base/strings/string_piece.h"
 #include "base/synchronization/lock.h"
@@ -133,6 +134,43 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
     ~State();
   };
 
+  // We create one FieldTrialEntry per field trial in shared memory, via
+  // AddToAllocatorWhileLocked. The FieldTrialEntry is followed by a
+  // base::Pickle object that we unpickle and read from. Any changes to this
+  // structure requires a bump in kFieldTrialType id defined above.

[manzagop (departed), 2016/12/09 21:22:24]

nit: update comment as kFieldTrialType is defined in .cc, not above.

[lawrencewu, 2016/12/09 22:20:49]

Done.

+  struct FieldTrialEntry {
+    // Expected size for 32/64-bit check.
+    static constexpr size_t kExpectedInstanceSize = 8;
+
+    // Whether or not this field trial is activated. This is really just a
+    // boolean but marked as a uint32_t for portability reasons.
+    uint32_t activated;
+
+    // Size of the pickled structure, NOT the total size of this entry.
+    uint32_t pickle_size;
+
+    // Calling this is only valid when the entry is initialized. That is, it
+    // resides in shared memory and has a pickle containing the trial name and
+    // group name following it.
+    bool GetTrialAndGroupName(StringPiece* trial_name,
+                              StringPiece* group_name) const;
+
+    // Calling this is only valid when the entry is initialized as well. Reads
+    // the parameters following the trial and group name and stores them as
+    // key-value mappings in |params|.
+    bool GetParams(std::map<std::string, std::string>* params) const;
+
+   private:
+    // Returns an iterator over the data containing names and params.
+    PickleIterator GetPickleIterator() const;
+
+    // Takes the iterator and writes out the first two items into |trial_name|
+    // and |group_name|.
+    bool ReadStringPair(PickleIterator* iter,
+                        StringPiece* trial_name,
+                        StringPiece* group_name) const;
+  };

[Alexei Svitkine (slow), 2016/12/09 20:08:43]

Nit: Add DISALLOW_COPY_AND_ASSIGN(FieldTrialEntry)?

[bcwhite, 2016/12/09 20:13:29]

Allocator objects are always POD so can always be copied and assigned.  You may
get an error if you add ctor definitions because the compiler may no longer
consider it a POD.

[manzagop (departed), 2016/12/09 21:22:24]

Drive by: if relying on POD / trivial copy, use a is_pod / is_trivial or some
such static assert?

[bcwhite, 2016/12/09 21:38:42]

Already exists.  See PersistentMemoryAllocator::GetAsObject<>()

+
   typedef std::vector<ActiveGroup> ActiveGroups;
 
   // A return value to indicate that a given instance has not yet had a group
@@ -577,6 +615,19 @@ class BASE_EXPORT FieldTrialList {
   // Clears all the params in the allocator.
   static void ClearParamsFromSharedMemoryForTesting();
 
+  // We want to be able to dump field trial state to an allocator so that it

[Alexei Svitkine (slow), 2016/12/09 20:08:43]

Nit: Can you rephrase this comment and the one below without using "we want"
language. i.e. describe what it does.

[lawrencewu, 2016/12/09 20:31:11]

Done (as well as below).

+  // can be analyzed after a crash.
+  static void DumpAllFieldTrialsToPersistentAllocator(
+      PersistentMemoryAllocator* allocator);
+
+  // We want to be able to retrieve field trial state from an allocator so that
+  // it can be analyzed after a crash. The pointers in the returned vector are
+  // into the persistent memory segment and so are only valid as long as the
+  // allocator is valid.
+  static std::vector<const FieldTrial::FieldTrialEntry*>
+  GetAllFieldTrialsFromPersistentAllocator(
+      const PersistentMemoryAllocator* allocator);

[Alexei Svitkine (slow), 2016/12/09 20:08:43]

Can this be passed by const& instead of const*?

[lawrencewu, 2016/12/09 20:31:11]

Done.

+
  private:
   // Allow tests to access our innards for testing purposes.
   FRIEND_TEST_ALL_PREFIXES(FieldTrialListTest, InstantiateAllocator);
@@ -620,7 +671,8 @@ class BASE_EXPORT FieldTrialList {
 
   // Adds the field trial to the allocator. Caller must hold a lock before
   // calling this.
-  static void AddToAllocatorWhileLocked(FieldTrial* field_trial);
+  static void AddToAllocatorWhileLocked(PersistentMemoryAllocator* allocator,
+                                        FieldTrial* field_trial);
 
   // Activate the corresponding field trial entry struct in shared memory.
   static void ActivateFieldTrialEntryWhileLocked(FieldTrial* field_trial);