Split NaCl SFI and non-SFI helpers into separate processes

components/nacl/zygote/nacl_fork_delegate_linux.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 "components/nacl/zygote/nacl_fork_delegate_linux.h"
 
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/resource.h>
 #include <sys/socket.h>
@@ skipping 90 matching lines @@
   if (msg_len <= 0) {
     LOG(ERROR) << "SendIPCRequestAndReadReply: RecvMsg failed";
     return false;
   }
   *reply_size = msg_len;
   return true;
 }
 
 }  // 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) {
   VLOG(1) << "NaClForkDelegate::Init()";
   int fds[2];
 
   scoped_ptr<sandbox::SetuidSandboxClient> setuid_sandbox_client(
       sandbox::SetuidSandboxClient::Create());
 
   // For communications between the NaCl loader process and
@@ skipping 131 matching lines @@
   // TODO(bradchen): Make this LOG(ERROR) when the NaCl helper
   // becomes the default.
   fd_ = -1;
   if (IGNORE_EINTR(close(fds[0])) != 0)
     LOG(ERROR) << "close(fds[0]) failed";
 }
 
 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;
 }
 
 NaClForkDelegate::~NaClForkDelegate() {
   // side effect of close: delegate process will terminate
   if (status_ == kNaClHelperSuccess) {
     if (IGNORE_EINTR(close(fd_)) != 0)
       LOG(ERROR) << "close(fd_) failed";
   }
 }
 
 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;
 }
 
 pid_t NaClForkDelegate::Fork(const std::string& process_type,
                              const std::vector<int>& fds,
                              const std::string& channel_id) {
   VLOG(1) << "NaClForkDelegate::Fork";
 
   DCHECK(fds.size() == kNumPassedFDs);
 
   if (status_ != kNaClHelperSuccess) {
     LOG(ERROR) << "Cannot launch NaCl process: nacl_helper failed to start";
     return -1;
   }
 
   // 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];
   ssize_t reply_size = 0;
   bool got_reply =
       SendIPCRequestAndReadReply(fd_, fds, write_pickle,
                                  reply_buf, sizeof(reply_buf), &reply_size);
   if (!got_reply) {
     LOG(ERROR) << "Could not perform remote fork.";
     return -1;
@@ skipping 47 matching lines @@
   int remote_exit_code;
   if (!iter.ReadInt(&remote_exit_code)) {
     LOG(ERROR) << "GetTerminationStatus: pickle failed";
     return false;
   }
 
   *status = static_cast<base::TerminationStatus>(termination_status);
   *exit_code = remote_exit_code;
   return true;
 }