Fix adding duplicate trials to allocator (finally)

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 0ad13a22be77e00e85cde99aa41b72af63fa79c5..2e640afb037cae5c39ba6205d873e0be08f07e62 100644
--- a/base/metrics/field_trial_unittest.cc
+++ b/base/metrics/field_trial_unittest.cc
@@ -1187,11 +1187,44 @@ TEST(FieldTrialListTest, AddTrialsToAllocator) {
 
   FieldTrialList field_trial_list2(nullptr);
   std::unique_ptr<base::SharedMemory> shm(new SharedMemory(handle, true));
-  shm.get()->Map(4 << 10);  // Hardcoded, equal to kFieldTrialAllocationSize.
+  shm.get()->Map(64 << 10);  // Hardcoded, equal to kFieldTrialAllocationSize.

[Alexei Svitkine (slow), 2016/11/21 18:28:45]

This seems suboptimal.

I think you shouldn't make them equal and just use some size that's big enough
with a comment mentioning it should be big enough to hold the trials created in
the test. This way, it doesn't have to be updated when the other constant
changes.

(The other option would be to actually use the same constant and expose it
through an internal namespace - but that doesn't seem needed in this case.)

[lawrencewu, 2016/11/21 19:11:48]

Done.

   FieldTrialList::CreateTrialsFromSharedMemory(std::move(shm));
   std::string check_string;
   FieldTrialList::AllStatesToString(&check_string);
   EXPECT_EQ(save_string, check_string);
 }
 
+TEST(FieldTrialListTest, DoNotAddSimulatedFieldTrialsToAllocator) {
+  constexpr char kTrialName[] = "trial";
+  base::SharedMemoryHandle handle;
+  {
+    // Create a simulated trial and a real trial and call group() on them, which
+    // should only add the real trial to the field trial allocator.
+    FieldTrialList field_trial_list(nullptr);
+    FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded();
+
+    // This shouldn't add to the allocator.
+    FieldTrial* simulated_trial = FieldTrial::CreateSimulatedFieldTrial(
+        kTrialName, 100, "Simulated", 0.95);
+    simulated_trial->group();
+
+    // This should add to the allocator.
+    FieldTrial* real_trial =
+        FieldTrialList::CreateFieldTrial(kTrialName, "Real");
+    real_trial->group();
+
+    handle = base::SharedMemory::DuplicateHandle(
+        field_trial_list.field_trial_allocator_->shared_memory()->handle());
+  }
+
+  // Check that there's only one entry in the allocator.
+  FieldTrialList field_trial_list2(nullptr);
+  std::unique_ptr<base::SharedMemory> shm(new SharedMemory(handle, true));
+  shm.get()->Map(64 << 10);  // Hardcoded, equal to kFieldTrialAllocationSize.
+  FieldTrialList::CreateTrialsFromSharedMemory(std::move(shm));
+  std::string check_string;
+  FieldTrialList::AllStatesToString(&check_string);
+  ASSERT_TRUE(check_string.find("Simulated") == std::string::npos);

[Alexei Svitkine (slow), 2016/11/21 18:28:45]

EXPECT_NE?

[lawrencewu, 2016/11/21 19:11:48]

Done.

+}
+
 }  // namespace base