Use SharedPersistentMemoryAllocator to share field trial state

base/metrics/field_trial_unittest.cc

Index: base/metrics/field_trial_unittest.cc
diff --git a/base/metrics/field_trial_unittest.cc b/base/metrics/field_trial_unittest.cc
index bb45bd717e02f3e527efd775bf60ba821c034801..04d43338928ae00ef009394b423f26a7d6c2b7b0 100644
--- a/base/metrics/field_trial_unittest.cc
+++ b/base/metrics/field_trial_unittest.cc
@@ -1143,12 +1143,35 @@ TEST(FieldTrialListTest, TestCopyFieldTrialStateToFlags) {
   base::FilePath test_file_path = base::FilePath(FILE_PATH_LITERAL("Program"));
   base::CommandLine cmd_line = base::CommandLine(test_file_path);
 
-  std::unique_ptr<base::SharedMemory> field_trial_state =
-      base::FieldTrialList::CopyFieldTrialStateToFlags("field-trial-handle",
-                                                       &cmd_line);
+  base::FieldTrialList::CopyFieldTrialStateToFlags("field-trial-handle",
+                                                   &cmd_line);
 
-  EXPECT_TRUE(field_trial_state.get() == nullptr);
   EXPECT_TRUE(cmd_line.HasSwitch(switches::kForceFieldTrials));
 }
 
+TEST(FieldTrialListTest, InstantiateAllocator) {

[Alexei Svitkine (slow), 2016/10/20 17:50:29]

This test would pass if the function didn't add any trials either, right?

How about having a test that actually ensures that all the trials are added?

One way you could do it is to test both the serialization and deserialization.
For example, create a field trial list with some trials, serialize it to the
allocator, then create a new field trial list and deserialize from the allocator
and check that the contents match (e.g. by comparing the field trial states
string).

[lawrencewu, 2016/10/21 16:41:58]

Done.

+  // Test that instantiating the allocator twice doesn't create a new allocator
+  // or re-add any field trials.
+  base::FieldTrialList field_trial_list(
+      base::MakeUnique<base::MockEntropyProvider>());

[Alexei Svitkine (slow), 2016/10/20 17:50:29]

You can just pass in nullptr.

[lawrencewu, 2016/10/21 16:41:58]

Done.

+  base::FieldTrialList::CreateFieldTrial("Trial1", "Group1");
+
+  EXPECT_TRUE(base::FieldTrialList::GetFieldTrialAllocatorForTesting() ==
+              nullptr);

[Alexei Svitkine (slow), 2016/10/20 17:50:29]

EXPECT_EQ or just EXPECT_FALSE

[lawrencewu, 2016/10/21 16:41:58]

Done.

+  base::FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded();
+  void* memory =
+      base::FieldTrialList::GetFieldTrialAllocatorForTesting()->shared_memory();
+  size_t used =
+      base::FieldTrialList::GetFieldTrialAllocatorForTesting()->used();
+
+  // Ensure that the function is idempotent.
+  base::FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded();

[Alexei Svitkine (slow), 2016/10/20 17:50:29]

After this test runs, it sounds like it will leave a field trial allocator that
was created. That doesn't seem great - because that may affect functioning of
other tests. We should clear it so that there's no side effects from running the
test.

Actually, thinking about it more, how about making the allocator be a member of
the FieldTrialList object? This way, it's lifetime is tied to the FieldTrialList
object and so any tests that already create/delete the object will correctly
reset the allocator too.

To make this work with my suggestion of testing the serialization and
deserialization, I suggest making a helper function that takes the allocator
object as a parameter, so that the serialize/deserialize can use that function
for testing.

[lawrencewu, 2016/10/21 16:41:58]

Done.

+  void* new_memory =
+      base::FieldTrialList::GetFieldTrialAllocatorForTesting()->shared_memory();
+  size_t new_used =
+      base::FieldTrialList::GetFieldTrialAllocatorForTesting()->used();
+  EXPECT_TRUE(memory == new_memory);

[Alexei Svitkine (slow), 2016/10/20 17:50:29]

EXPECT_EQ

[lawrencewu, 2016/10/21 16:41:58]

Done.

+  EXPECT_TRUE(used == new_used);
+}
+
 }  // namespace base