Tell nacl_helper to use non SFI mode in HandleForkRequest

components/nacl/browser/nacl_process_host.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/browser/nacl_process_host.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 574 matching lines @@
       env->SetVar(kPath, new_path);
     }
 #endif  // _DLL
   }
 #endif
 
   scoped_ptr<CommandLine> cmd_line(new CommandLine(exe_path));
   CopyNaClCommandLineArguments(cmd_line.get());
 
   cmd_line->AppendSwitchASCII(switches::kProcessType,
+                              uses_nonsfi_mode_ ?
+                              switches::kNaClNonSfiLoaderProcess :

[Mark Seaborn, 2014/03/28 15:38:42]

Have you checked all the places that use kNaClLoaderProcess to see if they also
need to check for the Non-SFI variant?

I found one:
AdjustLinuxOOMScore() in chrome/app/chrome_main_delegate.cc

[hamaji, 2014/03/30 03:31:35]

Sorry, I forgot to check this. It seems the one you mentioned is the only one we
need to modify. I couldn't understand nacl_broker_listener.cc well, but it seems
this is compiled only on windows.

                               switches::kNaClLoaderProcess);
   cmd_line->AppendSwitchASCII(switches::kProcessChannelID, channel_id);
   if (NaClBrowser::GetDelegate()->DialogsAreSuppressed())
     cmd_line->AppendSwitch(switches::kNoErrorDialogs);
 
   // On Windows we might need to start the broker process to launch a new loader
 #if defined(OS_WIN)
   if (RunningOnWOW64()) {
     if (!NaClBrokerService::GetInstance()->LaunchLoader(
             weak_factory_.GetWeakPtr(), channel_id)) {
@@ skipping 165 matching lines @@
   params.validation_cache_enabled = nacl_browser->ValidationCacheIsEnabled();
   params.validation_cache_key = nacl_browser->GetValidationCacheKey();
   params.version = NaClBrowser::GetDelegate()->GetVersionString();
   params.enable_exception_handling = enable_exception_handling_;
   params.enable_debug_stub = enable_debug_stub_ &&
       NaClBrowser::GetDelegate()->URLMatchesDebugPatterns(manifest_url_);
   // Enable PPAPI proxy channel creation only for renderer processes.
   params.enable_ipc_proxy = enable_ppapi_proxy();
   params.uses_irt = uses_irt_;
   params.enable_dyncode_syscalls = enable_dyncode_syscalls_;
-  params.uses_nonsfi_mode = uses_nonsfi_mode_;
 
   const ChildProcessData& data = process_->GetData();
   if (!ShareHandleToSelLdr(data.handle,
                            internal_->socket_for_sel_ldr, true,
                            &params.handles)) {
     return false;
   }
 
   if (params.uses_irt) {
     base::PlatformFile irt_file = nacl_browser->IrtFile();
@@ skipping 29 matching lines @@
 #if defined(OS_POSIX)
   if (params.enable_debug_stub) {
     net::SocketDescriptor server_bound_socket = GetDebugStubSocketHandle();
     if (server_bound_socket != net::kInvalidSocket) {
       params.debug_stub_server_bound_socket =
           FileDescriptor(server_bound_socket, true);
     }
   }
 #endif
 
-  if (params.uses_nonsfi_mode) {
+  if (uses_nonsfi_mode_) {
 #if defined(OS_LINUX)
     const bool kNonSFIModeSupported = true;
 #else
     const bool kNonSFIModeSupported = false;
 #endif
     if (!kNonSFIModeSupported ||

[Mark Seaborn, 2014/03/28 15:38:42]

This sanity check now happens after we've launched the process with
uses_nonsfi_mode=true.  It would be better to move it to happen earlier, just in
case.  (Also it would be good to comment it as a sanity check.)

[hamaji, 2014/03/30 03:31:35]

Done. I've moved this to Launch(). It seems there are some other sanity checks
and the intention of the check seems to be clear as I've added
SendErrorToRenderer, so I didn't add any comments.

         !CommandLine::ForCurrentProcess()->HasSwitch(
             switches::kEnableNaClNonSfiMode)) {
       return false;
     }
   }
 
   process_->Send(new NaClProcessMsg_Start(params));
 
   internal_->socket_for_sel_ldr = NACL_INVALID_HANDLE;
   return true;
@@ skipping 234 matching lines @@
         process_handle.Take(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif
 
 }  // namespace nacl