Use SharedPersistentMemoryAllocator to share field trial state

content/browser/child_process_launcher.cc

Index: content/browser/child_process_launcher.cc
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc
index 82d6744b8dc98e829a2b314c55d8fccd781887f8..ed4dfa0f1d65e07ba8c1e79557bfa24cb1051494 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -12,6 +12,7 @@
 #include "base/files/file_util.h"
 #include "base/i18n/icu_util.h"
 #include "base/logging.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/process/launch.h"
 #include "base/process/process.h"
@@ -121,7 +122,6 @@ void LaunchOnLauncherThread(const NotifyCallback& callback,
 #if defined(OS_ANDROID)
                             base::ScopedFD ipcfd,
 #endif
-                            const base::SharedMemory* field_trial_state,
                             mojo::edk::ScopedPlatformHandle client_handle,
                             base::CommandLine* cmd_line) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
@@ -153,8 +153,12 @@ void LaunchOnLauncherThread(const NotifyCallback& callback,
   } else {
     base::HandlesToInheritVector handles;
     handles.push_back(client_handle.get().handle);
-    if (field_trial_state)
-      handles.push_back(field_trial_state->handle().GetHandle());
+    if (base::FieldTrialList::field_trial_allocator) {

[Alexei Svitkine (slow), 2016/10/12 20:41:00]

Instead of this construct, how about making a function on FieldTrialList that
takes |handles| and appends it if necessary. Then, it's a simple call here and
you don't need to expose the allocator object outside the .cc file.

[lawrencewu, 2016/10/14 04:54:10]

Done. Created a AppendFieldTrialHandleIfNeeded() function and made the allocator
private.

+      handles.push_back(
+          base::FieldTrialList::field_trial_allocator->shared_memory()

[bcwhite, 2016/10/13 14:28:02]

The shared memory MUST be passed read-only.  Renderers are untrusted and we
can't allow for a compromised one to be able to alter these settings.  Ensure
that is the case and document it.

[lawrencewu, 2016/10/14 04:54:10]

If I understand the shared memory model correctly, this means that if the shared
memory is NOT read-only (which, I believe, is the case), then I must duplicate
the handle with read-only permissions and pass that handle down, right?

Haven't done any work on this issue yet, just want to clarify.

[bcwhite, 2016/10/14 13:56:56]

Sounds reasonable but I don't know.  You'll have to ask somebody with more
low-level windows experience.

+              ->handle()
+              .GetHandle());
+    }
     cmd_line->AppendSwitchASCII(
         mojo::edk::PlatformChannelPair::kMojoPlatformChannelHandleSwitch,
         base::UintToString(base::win::HandleToUint32(handles[0])));
@@ -400,7 +404,6 @@ ChildProcessLauncher::ChildProcessLauncher(
     base::CommandLine* cmd_line,
     int child_process_id,
     Client* client,
-    const base::SharedMemory* field_trial_state,
     const std::string& mojo_child_token,
     const mojo::edk::ProcessErrorCallback& process_error_callback,
     bool terminate_on_shutdown)
@@ -421,7 +424,7 @@ ChildProcessLauncher::ChildProcessLauncher(
       weak_factory_(this) {
   DCHECK(CalledOnValidThread());
   CHECK(BrowserThread::GetCurrentThreadIdentifier(&client_thread_id_));
-  Launch(delegate, cmd_line, child_process_id, field_trial_state);
+  Launch(delegate, cmd_line, child_process_id);
 }
 
 ChildProcessLauncher::~ChildProcessLauncher() {
@@ -437,8 +440,7 @@ ChildProcessLauncher::~ChildProcessLauncher() {
 
 void ChildProcessLauncher::Launch(SandboxedProcessLauncherDelegate* delegate,
                                   base::CommandLine* cmd_line,
-                                  int child_process_id,
-                                  const base::SharedMemory* field_trial_state) {
+                                  int child_process_id) {
   DCHECK(CalledOnValidThread());
 
 #if defined(OS_ANDROID)
@@ -487,7 +489,7 @@ void ChildProcessLauncher::Launch(SandboxedProcessLauncherDelegate* delegate,
 #if defined(OS_ANDROID)
                  base::Passed(&ipcfd),
 #endif
-                 field_trial_state, base::Passed(&client_handle), cmd_line));
+                 base::Passed(&client_handle), cmd_line));
 }
 
 void ChildProcessLauncher::UpdateTerminationStatus(bool known_dead) {