Split NaCl SFI and non-SFI helpers into separate processes

components/nacl/zygote/nacl_fork_delegate_linux.cc

Index: components/nacl/zygote/nacl_fork_delegate_linux.cc
diff --git a/components/nacl/zygote/nacl_fork_delegate_linux.cc b/components/nacl/zygote/nacl_fork_delegate_linux.cc
index 0e5f8e7eccc23c1a44a5a718c34164aa1271385e..a4eed5629627392881575da08de691a5ccdaf66b 100644
--- a/components/nacl/zygote/nacl_fork_delegate_linux.cc
+++ b/components/nacl/zygote/nacl_fork_delegate_linux.cc
@@ -108,9 +108,9 @@ bool SendIPCRequestAndReadReply(int ipc_channel,
 
 }  // namespace.
 
-NaClForkDelegate::NaClForkDelegate()
-    : status_(kNaClHelperUnused),
-      fd_(-1) {}
+NaClForkDelegate::NaClForkDelegate(bool nonsfi_mode)
+    : nonsfi_mode_(nonsfi_mode), status_(kNaClHelperUnused), fd_(-1) {
+}
 
 void NaClForkDelegate::Init(const int sandboxdesc,
                             const bool enable_layer1_sandbox) {
@@ -262,7 +262,8 @@ void NaClForkDelegate::Init(const int sandboxdesc,
 void NaClForkDelegate::InitialUMA(std::string* uma_name,
                                   int* uma_sample,
                                   int* uma_boundary_value) {
-  *uma_name = "NaCl.Client.Helper.InitState";
+  *uma_name = nonsfi_mode_ ? "NaCl.Client.HelperNonSFI.InitState"
+                           : "NaCl.Client.Helper.InitState";
   *uma_sample = status_;
   *uma_boundary_value = kNaClHelperStatusBoundary;
 }
@@ -279,10 +280,11 @@ bool NaClForkDelegate::CanHelp(const std::string& process_type,
                                std::string* uma_name,
                                int* uma_sample,
                                int* uma_boundary_value) {
-  if (process_type != switches::kNaClLoaderProcess &&
-      process_type != switches::kNaClLoaderNonSfiProcess)
+  if (process_type != (nonsfi_mode_ ? switches::kNaClLoaderNonSfiProcess

[jln (very slow on Chromium), 2014/05/09 20:51:20]

This is hard to read.

Maybe have an intermediate can_help_with_process_type variable?

I wonder if you shouldn't just save that information in the constructor,
depending on nonsfi_mode.

[mdempsky, 2014/05/09 21:34:11]

Done.  (I named it helpable_process_type.)
I think this way is a bit cleaner (no redundant state to keep track of), but I
don't feel strongly.  Long term, I expect CanHelp() to go away when we get rid
of fork delegates and instead have multiple full zygotes.

+                                    : switches::kNaClLoaderProcess))
     return false;
-  *uma_name = "NaCl.Client.Helper.StateOnFork";
+  *uma_name = nonsfi_mode_ ? "NaCl.Client.HelperNonSFI.StateOnFork"
+                           : "NaCl.Client.Helper.StateOnFork";
   *uma_sample = status_;
   *uma_boundary_value = kNaClHelperStatusBoundary;
   return true;
@@ -303,11 +305,7 @@ pid_t NaClForkDelegate::Fork(const std::string& process_type,
   // First, send a remote fork request.
   Pickle write_pickle;
   write_pickle.WriteInt(nacl::kNaClForkRequest);
-  // TODO(hamaji): When we split the helper binary for non-SFI mode
-  // from nacl_helper, stop sending this information.
-  const bool uses_nonsfi_mode =
-    process_type == switches::kNaClLoaderNonSfiProcess;
-  write_pickle.WriteBool(uses_nonsfi_mode);
+  write_pickle.WriteBool(nonsfi_mode_);
   write_pickle.WriteString(channel_id);
 
   char reply_buf[kNaClMaxIPCMessageLength];