NaCl: Clean up file descriptor setup in nacl_helper on linux

There is no need to be using dup2() to set up an FD with a fixed
number; this risks overwriting an FD.  The point of
base::GlobalDescriptors is that it provides a level of indirection
that allows any FD number to be used.

Remove kNaClBrowserDescriptor.  Remove the browserdesc argument that
is not used for anything other than an assertion and so isn't needed.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=2096
TEST=NaCl tests in browser_tests

Review URL: https://codereview.chromium.org/11819021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176106 0039d316-1c4b-4281-b951-d872f2087c98

chrome/app/nacl_fork_delegate_linux.cc

@@ -34,13 +34,12 @@ const char kNaClHelperReservedAtZero[] =
     "--reserved_at_zero=0xXXXXXXXXXXXXXXXX";
 const char kNaClHelperRDebug[] = "--r_debug=0xXXXXXXXXXXXXXXXX";
 
-void NaClForkDelegate::Init(const int browserdesc, const int sandboxdesc) {
+void NaClForkDelegate::Init(const int sandboxdesc) {
   VLOG(1) << "NaClForkDelegate::Init()";
   int fds[2];
 
-  // Confirm a couple hard-wired assumptions.
-  // The NaCl constants are from chrome/nacl/nacl_linux_helper.h
-  DCHECK(kNaClBrowserDescriptor == browserdesc);
+  // Confirm a hard-wired assumption.
+  // The NaCl constant is from chrome/nacl/nacl_linux_helper.h
   DCHECK(kNaClSandboxDescriptor == sandboxdesc);
 
   CHECK(socketpair(PF_UNIX, SOCK_SEQPACKET, 0, fds) == 0);

chrome/app/nacl_fork_delegate_linux.h

@@ -22,7 +22,7 @@ class NaClForkDelegate : public content::ZygoteForkDelegate {
   NaClForkDelegate();
   virtual ~NaClForkDelegate();
 
-  virtual void Init(int browserdesc, int sandboxdesc) OVERRIDE;
+  virtual void Init(int sandboxdesc) OVERRIDE;
   virtual void InitialUMA(std::string* uma_name,
                           int* uma_sample,
                           int* uma_boundary_value) OVERRIDE;

chrome/common/nacl_helper_linux.h

@@ -18,17 +18,14 @@
 // For communications between NaCl loader and browser.
 // See also content/common/zygote_main_linux.cc and
 // http://code.google.com/p/chromium/wiki/LinuxZygote
-#define kNaClBrowserDescriptor 3
+
 // For communications between NaCl loader and zygote.
-// We put the kNaClZygoteDescriptor on 3 together with the
-// kNaClBrowserDescriptor. They are never used at the same
-// time, and this prevents /dev/urandom from using the fd.
 #define kNaClZygoteDescriptor 3
 // For communications between the NaCl loader process and
 // the SUID sandbox.
 #define kNaClSandboxDescriptor 5
-// NOTE: kNaClBrowserDescriptor and kNaClSandboxDescriptor must match
-// content/browser/zygote_main_linux.cc kBrowserDescriptor and
+// NOTE: kNaClSandboxDescriptor must match
+// content/browser/zygote_main_linux.cc
 // kMagicSandboxIPCDescriptor.
 
 // A fork request from the Zygote to the helper includes an array

chrome/nacl/nacl_helper_linux.cc

@@ -43,14 +43,8 @@ void BecomeNaClLoader(const std::vector<int>& child_fds,
   // don't need zygote FD any more
   if (HANDLE_EINTR(close(kNaClZygoteDescriptor)) != 0)
     LOG(ERROR) << "close(kNaClZygoteDescriptor) failed.";
-  // Set up browser descriptor on fd 3 and IPC as expected by Chrome.
   base::GlobalDescriptors::GetInstance()->Set(kPrimaryIPCChannel,
-      kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor);
-  int zfd = dup2(child_fds[kNaClBrowserFDIndex], kNaClBrowserDescriptor);
-  if (zfd != kNaClBrowserDescriptor) {
-    LOG(ERROR) << "Could not initialize kNaClBrowserDescriptor";
-    _exit(-1);
-  }
+                                              child_fds[kNaClBrowserFDIndex]);
 
   MessageLoopForIO main_message_loop;
   NaClListener listener;

content/public/common/zygote_fork_delegate_linux.h

@@ -25,7 +25,7 @@ class ZygoteForkDelegate {
 
   // Initialization happens in the zygote after it has been
   // started by ZygoteMain.
-  virtual void Init(int browserdesc, int sandboxdesc) = 0;
+  virtual void Init(int sandboxdesc) = 0;
 
   // After Init, supply a UMA_HISTOGRAM_ENUMERATION the delegate
   // would like to supply on the first fork.

content/zygote/zygote_main_linux.cc

@@ -463,8 +463,7 @@ bool ZygoteMain(const MainFunctionParams& params,
 
   if (forkdelegate != NULL) {
     VLOG(1) << "ZygoteMain: initializing fork delegate";
-    forkdelegate->Init(Zygote::kBrowserDescriptor,
-                       Zygote::kMagicSandboxIPCDescriptor);
+    forkdelegate->Init(Zygote::kMagicSandboxIPCDescriptor);
   } else {
     VLOG(1) << "ZygoteMain: fork delegate is NULL";
   }