Share field trial allocator on zygote-using Linuxes

base/metrics/field_trial.cc

Index: base/metrics/field_trial.cc
diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index 1f18658abd72faf92b39ff982873a84c64ac7c91..cc9a035bc53779da6eb9598bdb48e70dc11cc345 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -21,6 +21,17 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 
+// On systems that use the zygote process to spawn child processes, we must
+// retrieve the correct fd using the mapping in GlobalDescriptors.
+#if defined(OS_POSIX) && !defined(OS_NACL) && !defined(OS_MACOSX) && \
+    !defined(OS_ANDROID)
+#define POSIX_WITH_ZYGOTE 1
+#endif
+
+#if defined(POSIX_WITH_ZYGOTE)
+#include "base/posix/global_descriptors.h"
+#endif
+
 namespace base {
 
 namespace {
@@ -232,11 +243,20 @@ HANDLE CreateReadOnlyHandle(FieldTrialList::FieldTrialAllocator* allocator) {
   DWORD access = SECTION_MAP_READ | SECTION_QUERY;
   HANDLE dst;
   if (!::DuplicateHandle(process, src, process, &dst, access, true, 0))
-    return nullptr;
+    return kInvalidPlatformFile;
   return dst;
 }
 #endif
 
+#if defined(POSIX_WITH_ZYGOTE)
+int CreateReadOnlyHandle(FieldTrialList::FieldTrialAllocator* allocator) {
+  SharedMemoryHandle new_handle;
+  allocator->shared_memory()->ShareReadOnlyToProcess(GetCurrentProcessHandle(),
+                                                     &new_handle);
+  return SharedMemory::GetFdFromSharedMemoryHandle(new_handle);
+}
+#endif
+
 }  // namespace
 
 // statics
@@ -736,19 +756,27 @@ bool FieldTrialList::CreateTrialsFromString(
 // static
 void FieldTrialList::CreateTrialsFromCommandLine(
     const CommandLine& cmd_line,
-    const char* field_trial_handle_switch) {
+    const char* field_trial_handle_switch,
+    int field_trial_descriptor_id) {
   global_->create_trials_from_command_line_called_ = true;
 
-#if defined(OS_WIN) && !defined(OS_NACL)
+#if defined(OS_WIN)
   if (cmd_line.HasSwitch(field_trial_handle_switch)) {
-    std::string arg = cmd_line.GetSwitchValueASCII(field_trial_handle_switch);
-    int field_trial_handle = std::stoi(arg);
-    HANDLE handle = reinterpret_cast<HANDLE>(field_trial_handle);
-    bool result = CreateTrialsFromWindowsHandle(handle);
+    std::string handle_switch =
+        cmd_line.GetSwitchValueASCII(field_trial_handle_switch);
+    bool result = CreateTrialsFromHandleSwitch(handle_switch);
     DCHECK(result);
   }
 #endif
 
+#if defined(POSIX_WITH_ZYGOTE)
+  bool result = CreateTrialsFromDescriptor(field_trial_descriptor_id);

[Alexei Svitkine (slow), 2016/11/29 19:32:47]

Nit: Just inline this into the if.

[lawrencewu, 2016/11/29 19:52:44]

Done.

+  // If we failed to create trials from the descriptor, fallback to the command
+  // line.
+  if (result)
+    return;
+#endif
+
   if (cmd_line.HasSwitch(switches::kForceFieldTrials)) {
     bool result = FieldTrialList::CreateTrialsFromString(
         cmd_line.GetSwitchValueASCII(switches::kForceFieldTrials),
@@ -757,6 +785,30 @@ void FieldTrialList::CreateTrialsFromCommandLine(
   }
 }
 
+#if defined(POSIX_WITH_ZYGOTE)
+// static
+bool FieldTrialList::CreateTrialsFromDescriptor(int field_trial_descriptor_id) {
+  if (!kUseSharedMemoryForFieldTrials)
+    return false;
+
+  if (field_trial_descriptor_id == -1) {

[Alexei Svitkine (slow), 2016/11/29 19:32:47]

Nit: No {}

[lawrencewu, 2016/11/29 19:52:44]

Done.

+    return false;
+  }
+
+  int fd =
+      GlobalDescriptors::GetInstance()->MaybeGet(field_trial_descriptor_id);
+  if (fd == -1)
+    return false;
+
+  global_->create_trials_from_command_line_called_ = true;

[Alexei Svitkine (slow), 2016/11/29 19:32:47]

This is not needed anymore since you're calling it from
CreateTrialsFromCommandLine.

[lawrencewu, 2016/11/29 19:52:44]

Removed.

+
+  SharedMemoryHandle shm_handle(fd, true);
+  bool result = FieldTrialList::CreateTrialsFromSharedMemoryHandle(shm_handle);
+  DCHECK(result);
+  return true;
+}
+#endif
+
 #if defined(OS_WIN)
 // static
 void FieldTrialList::AppendFieldTrialHandleIfNeeded(
@@ -771,6 +823,20 @@ void FieldTrialList::AppendFieldTrialHandleIfNeeded(
 }
 #endif
 
+#if defined(OS_POSIX) && !defined(OS_NACL)
+// static
+int FieldTrialList::GetFieldTrialHandle() {
+  if (!global_)

[Alexei Svitkine (slow), 2016/11/29 19:32:47]

Does this actually get hit?

[lawrencewu, 2016/11/29 19:52:44]

I added a log statement here and it doesn't get hit -- but I don't see the harm
in keeping this check.

[Alexei Svitkine (slow), 2016/11/29 20:19:27]

I think it would be better to remove this if it's not necessary.

[lawrencewu, 2016/11/29 21:40:18]

Actually, it looks like some tests do hit that and fail without it, so I added
the check back.

+    return kInvalidPlatformFile;
+  if (kUseSharedMemoryForFieldTrials) {
+    InstantiateFieldTrialAllocatorIfNeeded();
+    // We check for an invalid handle where this gets called.
+    return global_->readonly_allocator_handle_;
+  }
+  return kInvalidPlatformFile;
+}
+#endif
+
 // static
 void FieldTrialList::CopyFieldTrialStateToFlags(
     const char* field_trial_handle_switch,
@@ -781,26 +847,33 @@ void FieldTrialList::CopyFieldTrialStateToFlags(
   if (!global_)
     return;
 
-#if defined(OS_WIN)
+#if defined(OS_WIN) || defined(POSIX_WITH_ZYGOTE)
   // Use shared memory to pass the state if the feature is enabled, otherwise
   // fallback to passing it via the command line as a string.
   if (kUseSharedMemoryForFieldTrials) {
     InstantiateFieldTrialAllocatorIfNeeded();
     // If the readonly handle didn't get duplicated properly, then fallback to
     // original behavior.
-    if (!global_->readonly_allocator_handle_) {
+    if (global_->readonly_allocator_handle_ == kInvalidPlatformFile) {
       AddForceFieldTrialsFlag(cmd_line);
       return;
     }
 
-    // HANDLE is just typedef'd to void *. We basically cast the handle into an
-    // int (uintptr_t, to be exact), stringify the int, and pass it as a
-    // command-line flag. The child process will do the reverse conversions to
-    // retrieve the handle. See http://stackoverflow.com/a/153077
+// We need to pass a named anonymous handle to shared memory over the command

[Alexei Svitkine (slow), 2016/11/29 19:32:47]

Nit: Indent this comment.

[lawrencewu, 2016/11/29 19:52:44]

Done.

+// line on Windows, since the child doesn't know which of the handles it
+// inherited it should open. On POSIX, we don't need to do this -- we dup the fd
+// into a fixed fd kFieldTrialDescriptor, so we can just look it up there.
+#if defined(OS_WIN)
+    // PlatformFile is typedef'd to HANDLE which is typedef'd to void *. We
+    // basically cast the handle into an int (uintptr_t, to be exact), stringify
+    // the int, and pass it as a command-line flag. The child process will do
+    // the reverse conversions to retrieve the handle. See
+    // http://stackoverflow.com/a/153077
     auto uintptr_handle =
         reinterpret_cast<uintptr_t>(global_->readonly_allocator_handle_);
     std::string field_trial_handle = std::to_string(uintptr_handle);
     cmd_line->AppendSwitchASCII(field_trial_handle_switch, field_trial_handle);
+#endif
     global_->field_trial_allocator_->UpdateTrackingHistograms();

[Alexei Svitkine (slow), 2016/11/29 19:32:46]

Nit: Move this line above to line 861.

[lawrencewu, 2016/11/29 19:52:44]

Done.

     return;
   }
@@ -894,9 +967,19 @@ size_t FieldTrialList::GetFieldTrialCount() {
 
 #if defined(OS_WIN)
 // static
-bool FieldTrialList::CreateTrialsFromWindowsHandle(HANDLE handle) {
+bool FieldTrialList::CreateTrialsFromHandleSwitch(
+    const std::string& handle_switch) {
+  int field_trial_handle = std::stoi(handle_switch);
+  HANDLE handle = reinterpret_cast<HANDLE>(field_trial_handle);
   SharedMemoryHandle shm_handle(handle, GetCurrentProcId());
+  return FieldTrialList::CreateTrialsFromSharedMemoryHandle(shm_handle);
+}
+#endif
 
+#if !defined(OS_NACL)
+// static
+bool FieldTrialList::CreateTrialsFromSharedMemoryHandle(
+    SharedMemoryHandle shm_handle) {
   // shm gets deleted when it gets out of scope, but that's OK because we need
   // it only for the duration of this method.
   std::unique_ptr<SharedMemory> shm(new SharedMemory(shm_handle, true));
@@ -986,8 +1069,15 @@ void FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded() {
   if (global_->field_trial_allocator_ != nullptr)
     return;
 
+  SharedMemoryCreateOptions options;
+  options.size = kFieldTrialAllocationSize;
+  options.share_read_only = true;
+
   std::unique_ptr<SharedMemory> shm(new SharedMemory());
-  if (!shm->CreateAndMapAnonymous(kFieldTrialAllocationSize))
+  if (!shm->Create(options))
+    TerminateBecauseOutOfMemory(kFieldTrialAllocationSize);
+
+  if (!shm->Map(kFieldTrialAllocationSize))
     TerminateBecauseOutOfMemory(kFieldTrialAllocationSize);
 
   global_->field_trial_allocator_.reset(
@@ -999,7 +1089,7 @@ void FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded() {
     AddToAllocatorWhileLocked(registered.second);
   }
 
-#if defined(OS_WIN)
+#if defined(OS_WIN) || defined(POSIX_WITH_ZYGOTE)
   // Set |readonly_allocator_handle_| so we can pass it to be inherited and
   // via the command line.
   global_->readonly_allocator_handle_ =