Add a whitelist check for nacl-nonsfi mode

components/nacl/browser/nacl_process_host.cc

Index: components/nacl/browser/nacl_process_host.cc
diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc
index ff88399e67227e1e12c0d1f083acd3ed6c543fe4..947f7bdf6ff84c6662f477d280a0ea98f6686cfc 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -27,6 +27,7 @@
 #include "base/win/windows_version.h"
 #include "build/build_config.h"
 #include "components/nacl/browser/nacl_browser.h"
+#include "components/nacl/browser/nacl_browser_delegate.h"
 #include "components/nacl/browser/nacl_host_message_filter.h"
 #include "components/nacl/common/nacl_cmd_line.h"
 #include "components/nacl/common/nacl_host_messages.h"
@@ -430,15 +431,21 @@ void NaClProcessHost::Launch(
   }
 
   if (uses_nonsfi_mode_) {
-#if defined(OS_LINUX)
-    const bool kNonSFIModeSupported = true;
+    const bool nonsfi_switch_enabled =
+        cmd->HasSwitch(switches::kEnableNaClNonSfiMode);
+#if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARM_FAMILY)

[Mark Seaborn, 2014/05/08 01:06:02]

This should be ARCH_CPU_ARMEL, otherwise it would get enabled for ARM64 too.

[elijahtaylor1, 2014/05/08 20:00:24]

Thanks, I didn't know there was this distinction

+    const bool nonsfi_mode_supported = true;
+#elif defined(OS_LINUX) || defined(OS_CHROMEOS)

[Mark Seaborn, 2014/05/08 01:06:02]

You only need OS_LINUX.  OS_CHROMEOS implies OS_LINUX.

[elijahtaylor1, 2014/05/08 20:00:24]

Done.

+    const bool nonsfi_mode_supported = nonsfi_switch_enabled;
 #else
-    const bool kNonSFIModeSupported = false;
+    const bool nonsfi_mode_supported = false;
 #endif
-    if (!kNonSFIModeSupported ||
-        !cmd->HasSwitch(switches::kEnableNaClNonSfiMode)) {
-      SendErrorToRenderer("NaCl non-SFI mode works only on Linux with"
-                          " --enable-nacl-nonsfi-mode specified");
+    bool is_enabled = nonsfi_switch_enabled ||

[Mark Seaborn, 2014/05/08 01:06:02]

I find this logic hard to understand because nonsfi_switch_enabled is checked
multiple times.

How about this:

  bool nonsfi_mode_enabled = false;
#if defined(OS_LINUX)
  nonsfi_mode_enabled ||= cmd->HasSwitch(switches::kEnableNaClNonSfiMode);
# if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
  nonsfi_mode_enabled ||=
NaClBrowser::GetDelegate()->IsNonSfiModeAllowed(manifest_url_);
# endif
#endif
  if (!nonsfi_mode_enabled) {
    ...
  }

[elijahtaylor1, 2014/05/08 20:00:24]

Yeah, I wasn't in love with how this turned out.  I like that your version just
uses one flag that's additive.

+        NaClBrowser::GetDelegate()->IsNonSfiModeAllowed(manifest_url_);
+
+    if (!nonsfi_mode_supported || !is_enabled) {
+      SendErrorToRenderer("NaCl non-SFI mode is not available for this platform"
+                          " and NaCl module.");
       delete this;
       return;
     }