Fix adding duplicate trials to allocator (finally)

base/metrics/field_trial_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/metrics/field_trial.h"
 
 #include <stddef.h>
 
 #include "base/base_switches.h"
 #include "base/build_time.h"
@@ skipping 1169 matching lines @@
     FieldTrialList field_trial_list(nullptr);
     FieldTrialList::CreateFieldTrial("Trial1", "Group1");
     FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded();
     FieldTrialList::AllStatesToString(&save_string);
     handle = base::SharedMemory::DuplicateHandle(
         field_trial_list.field_trial_allocator_->shared_memory()->handle());
   }
 
   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