Revert "POSIX: don't allocate memory after forking."

Appears to break tlslite on the Mac.

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

11 files changed, 118 insertions, 536 deletions

diff --git a/base/base.gyp b/base/base.gyp
index 30d746d..2eec8e1 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -61,7 +61,7 @@
         'crypto/signature_verifier_unittest.cc',
         'data_pack_unittest.cc',
         'debug_util_unittest.cc',
-        'dir_reader_posix_unittest.cc',
+        'file_watcher_unittest.cc',
         'event_trace_consumer_win_unittest.cc',
         'event_trace_controller_win_unittest.cc',
         'event_trace_provider_win_unittest.cc',
@@ -70,7 +70,6 @@
         'file_path_unittest.cc',
         'file_util_unittest.cc',
         'file_version_info_unittest.cc',
-        'file_watcher_unittest.cc',
         'gfx/rect_unittest.cc',
         'gmock_unittest.cc',
         'histogram_unittest.cc',
@@ -182,7 +181,6 @@
             '../third_party/icu/icu.gyp:icudata',
           ],
           'sources!': [
-            'dir_reader_posix_unittest.cc',
             'file_descriptor_shuffle_unittest.cc',
           ],
         }, {  # OS != "win"
diff --git a/base/base.gypi b/base/base.gypi
index 1fbb537..c9d2e08 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -61,9 +61,6 @@
           'debug_util_mac.cc',
           'debug_util_posix.cc',
           'debug_util_win.cc',
-          'dir_reader_fallback.h',
-          'dir_reader_linux.h',
-          'dir_reader_posix.h',
           'event_trace_consumer_win.h',
           'event_trace_controller_win.cc',
           'event_trace_controller_win.h',
diff --git a/base/dir_reader_fallback.h b/base/dir_reader_fallback.h
deleted file mode 100644
index c8f02e6..0000000
--- a/base/dir_reader_fallback.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef BASE_DIR_READER_FALLBACK_H_
-#define BASE_DIR_READER_FALLBACK_H_
-
-namespace base {
-
-class DirReaderFallback {
- public:
-  // Open a directory. If |IsValid| is true, then |Next| can be called to start
-  // the iteration at the beginning of the directory.
-  explicit DirReaderFallback(const char* directory_path) { }
-  // After construction, IsValid returns true iff the directory was
-  // successfully opened.
-  bool IsValid() const { return false; }
-  // Move to the next entry returning false if the iteration is complete.
-  bool Next() { return false; }
-  // Return the name of the current directory entry.
-  const char* name() { return 0;}
-  // Return the file descriptor which is being used.
-  int fd() const { return -1; }
-  // Returns true if this is a no-op fallback class (for testing).
-  static bool IsFallback() { return true; }
-};
-
-}  // namespace base
-
-#endif  // BASE_DIR_READER_FALLBACK_H_
diff --git a/base/dir_reader_linux.h b/base/dir_reader_linux.h
deleted file mode 100644
index 7fd534f..0000000
--- a/base/dir_reader_linux.h
+++ /dev/null
@@ -1,97 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef BASE_DIR_READER_LINUX_H_
-#define BASE_DIR_READER_LINUX_H_
-
-#include <errno.h>
-#include <fcntl.h>
-#include <stdint.h>
-#include <sys/syscall.h>
-#include <unistd.h>
-
-#include "base/logging.h"
-#include "base/eintr_wrapper.h"
-
-// See the comments in dir_reader_posix.h about this.
-
-namespace base {
-
-struct linux_dirent {
-  uint64_t        d_ino;
-  int64_t         d_off;
-  unsigned short  d_reclen;
-  unsigned char   d_type;
-  char            d_name[0];
-};
-
-class DirReaderLinux {
- public:
-  explicit DirReaderLinux(const char* directory_path)
-      : fd_(open(directory_path, O_RDONLY | O_DIRECTORY)),
-        offset_(0),
-        size_(0) {
-  }
-
-  ~DirReaderLinux() {
-    if (fd_ >= 0) {
-      if (HANDLE_EINTR(close(fd_)))
-        RAW_LOG(ERROR, "Failed to close directory handle");
-    }
-  }
-
-  bool IsValid() const {
-    return fd_ >= 0;
-  }
-
-  // Move to the next entry returning false if the iteration is complete.
-  bool Next() {
-    if (size_) {
-      linux_dirent* dirent = reinterpret_cast<linux_dirent*>(&buf_[offset_]);
-      offset_ += dirent->d_reclen;
-    }
-
-    if (offset_ != size_)
-      return true;
-
-    const int r = syscall(__NR_getdents64, fd_, buf_, sizeof(buf_));
-    if (r == 0)
-      return false;
-    if (r == -1) {
-      DPLOG(FATAL) << "getdents64 returned an error: " << errno;
-      return false;
-    }
-    size_ = r;
-    offset_ = 0;
-    return true;
-  }
-
-  const char* name() const {
-    if (!size_)
-      return NULL;
-
-    const linux_dirent* dirent =
-        reinterpret_cast<const linux_dirent*>(&buf_[offset_]);
-    return dirent->d_name;
-  }
-
-  int fd() const {
-    return fd_;
-  }
-
-  static bool IsFallback() {
-    return false;
-  }
-
- private:
-  const int fd_;
-  unsigned char buf_[512];
-  size_t offset_, size_;
-
-  DISALLOW_COPY_AND_ASSIGN(DirReaderLinux);
-};
-
-}  // namespace base
-
-#endif // BASE_DIR_READER_LINUX_H_
diff --git a/base/dir_reader_posix.h b/base/dir_reader_posix.h
deleted file mode 100644
index e9ad11e..0000000
--- a/base/dir_reader_posix.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef BASE_DIR_READER_POSIX_H_
-#define BASE_DIR_READER_POSIX_H_
-
-#include "build/build_config.h"
-
-// This header provides a class, DirReaderPosix, which allows one to open and
-// read from directories without allocating memory. For the interface, see
-// the generic fallback in dir_reader_fallback.h.
-
-#if defined(OS_LINUX)
-#include "base/dir_reader_linux.h"
-#else
-#include "base/dir_reader_fallback.h"
-#endif
-
-namespace base {
-
-#if defined(OS_LINUX)
-typedef DirReaderLinux DirReaderPosix;
-#else
-typedef DirReaderFallback DirReaderPosix;
-#endif
-
-}  // namespace base
-
-#endif // BASE_DIR_READER_POSIX_H_
diff --git a/base/dir_reader_posix_unittest.cc b/base/dir_reader_posix_unittest.cc
deleted file mode 100644
index 04f7243..0000000
--- a/base/dir_reader_posix_unittest.cc
+++ /dev/null
@@ -1,91 +0,0 @@
-// Copyright (c) 2010 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 "base/dir_reader_posix.h"
-
-#include <fcntl.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "base/logging.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using base::DirReaderPosix;
-
-namespace {
-typedef testing::Test DirReaderPosixUnittest;
-
-TEST(DirReaderPosixUnittest, Read) {
-  static const unsigned kNumFiles = 100;
-
-  if (DirReaderPosix::IsFallback())
-    return;
-
-  char kDirTemplate[] = "/tmp/org.chromium.dir-reader-posix-XXXXXX";
-  const char* dir = mkdtemp(kDirTemplate);
-  CHECK(dir);
-
-  const int prev_wd = open(".", O_RDONLY | O_DIRECTORY);
-  CHECK_GE(prev_wd, 0);
-
-  PCHECK(chdir(dir) == 0);
-
-  for (unsigned i = 0; i < kNumFiles; i++) {
-    char buf[16];
-    snprintf(buf, sizeof(buf), "%d", i);
-    const int fd = open(buf, O_CREAT | O_RDONLY | O_EXCL, 0600);
-    PCHECK(fd >= 0);
-    PCHECK(close(fd) == 0);
-  }
-
-  std::set<unsigned> seen;
-
-  DirReaderPosix reader(dir);
-  EXPECT_TRUE(reader.IsValid());
-
-  if (!reader.IsValid())
-    return;
-
-  bool seen_dot = false, seen_dotdot = false;
-
-  for (; reader.Next(); ) {
-    if (strcmp(reader.name(), ".") == 0) {
-      seen_dot = true;
-      continue;
-    }
-    if (strcmp(reader.name(), "..") == 0) {
-      seen_dotdot = true;
-      continue;
-    }
-
-    SCOPED_TRACE(testing::Message() << "reader.name(): " << reader.name());
-
-    char *endptr;
-    const unsigned long value = strtoul(reader.name(), &endptr, 10);
-
-    EXPECT_FALSE(*endptr);
-    EXPECT_LT(value, kNumFiles);
-    EXPECT_EQ(0u, seen.count(value));
-    seen.insert(value);
-  }
-
-  for (unsigned i = 0; i < kNumFiles; i++) {
-    char buf[16];
-    snprintf(buf, sizeof(buf), "%d", i);
-    PCHECK(unlink(buf) == 0);
-  }
-
-  PCHECK(rmdir(dir) == 0);
-
-  PCHECK(fchdir(prev_wd) == 0);
-  PCHECK(close(prev_wd) == 0);
-
-  EXPECT_TRUE(seen_dot);
-  EXPECT_TRUE(seen_dotdot);
-  EXPECT_EQ(kNumFiles, seen.size());
-}
-
-}  // anonymous namespace
diff --git a/base/file_descriptor_shuffle.cc b/base/file_descriptor_shuffle.cc
index 2bb156b..e722a29 100644
--- a/base/file_descriptor_shuffle.cc
+++ b/base/file_descriptor_shuffle.cc
@@ -12,36 +12,28 @@
 
 namespace base {
 
-bool PerformInjectiveMultimapDestructive(
-    InjectiveMultimap* m, InjectionDelegate* delegate) {
-  static const size_t kMaxExtraFDs = 16;
-  int extra_fds[kMaxExtraFDs];
-  unsigned next_extra_fd = 0;
-
-  // DANGER: this function may not allocate.
+bool PerformInjectiveMultimap(const InjectiveMultimap& m_in,
+                              InjectionDelegate* delegate) {
+  InjectiveMultimap m(m_in);
+  std::vector<int> extra_fds;
 
-  for (InjectiveMultimap::iterator i = m->begin(); i != m->end(); ++i) {
+  for (InjectiveMultimap::iterator i = m.begin(); i != m.end(); ++i) {
     int temp_fd = -1;
 
     // We DCHECK the injectiveness of the mapping.
-    for (InjectiveMultimap::iterator j = i + 1; j != m->end(); ++j) {
+    for (InjectiveMultimap::iterator j = i + 1; j != m.end(); ++j) {
       DCHECK(i->dest != j->dest) << "Both fd " << i->source
           << " and " << j->source << " map to " << i->dest;
     }
 
     const bool is_identity = i->source == i->dest;
 
-    for (InjectiveMultimap::iterator j = i + 1; j != m->end(); ++j) {
+    for (InjectiveMultimap::iterator j = i + 1; j != m.end(); ++j) {
       if (!is_identity && i->dest == j->source) {
         if (temp_fd == -1) {
           if (!delegate->Duplicate(&temp_fd, i->dest))
             return false;
-          if (next_extra_fd < kMaxExtraFDs) {
-            extra_fds[next_extra_fd++] = temp_fd;
-          } else {
-            RAW_LOG(ERROR, "PerformInjectiveMultimapDestructive overflowed "
-                           "extra_fds. Leaking file descriptors!");
-          }
+          extra_fds.push_back(temp_fd);
         }
 
         j->source = temp_fd;
@@ -66,18 +58,14 @@ bool PerformInjectiveMultimapDestructive(
       delegate->Close(i->source);
   }
 
-  for (unsigned i = 0; i < next_extra_fd; i++)
-    delegate->Close(extra_fds[i]);
+  for (std::vector<int>::const_iterator
+       i = extra_fds.begin(); i != extra_fds.end(); ++i) {
+    delegate->Close(*i);
+  }
 
   return true;
 }
 
-bool PerformInjectiveMultimap(const InjectiveMultimap& m_in,
-                              InjectionDelegate* delegate) {
-  InjectiveMultimap m(m_in);
-  return PerformInjectiveMultimapDestructive(&m, delegate);
-}
-
 bool FileDescriptorTableInjection::Duplicate(int* result, int fd) {
   *result = HANDLE_EINTR(dup(fd));
   return *result >= 0;
diff --git a/base/file_descriptor_shuffle.h b/base/file_descriptor_shuffle.h
index e1c93cd..e0cb88b 100644
--- a/base/file_descriptor_shuffle.h
+++ b/base/file_descriptor_shuffle.h
@@ -69,13 +69,9 @@ typedef std::vector<InjectionArc> InjectiveMultimap;
 bool PerformInjectiveMultimap(const InjectiveMultimap& map,
                               InjectionDelegate* delegate);
 
-bool PerformInjectiveMultimapDestructive(InjectiveMultimap* map,
-                                         InjectionDelegate* delegate);
-
-// This function will not call malloc but will mutate |map|
-static inline bool ShuffleFileDescriptors(InjectiveMultimap* map) {
+static inline bool ShuffleFileDescriptors(const InjectiveMultimap& map) {
   FileDescriptorTableInjection delegate;
-  return PerformInjectiveMultimapDestructive(map, &delegate);
+  return PerformInjectiveMultimap(map, &delegate);
 }
 
 }  // namespace base
diff --git a/base/process_util.h b/base/process_util.h
index 0c8c474..cf4bd01 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -111,6 +111,12 @@ bool AdjustOOMScore(ProcessId process, int score);
 #endif
 
 #if defined(OS_POSIX)
+// Sets all file descriptors to close on exec except for stdin, stdout
+// and stderr.
+// TODO(agl): remove this function
+// WARNING: do not use. It's inherently race-prone in the face of
+// multi-threading.
+void SetAllFDsToCloseOnExec();
 // Close all file descriptors, expect those which are a destination in the
 // given multimap. Only call this function in a child process where you know
 // that there aren't any other threads.
diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index f9f0f29..fdac7a2 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -18,7 +18,6 @@
 
 #include "base/compiler_specific.h"
 #include "base/debug_util.h"
-#include "base/dir_reader_posix.h"
 #include "base/eintr_wrapper.h"
 #include "base/logging.h"
 #include "base/platform_thread.h"
@@ -29,13 +28,8 @@
 #include "base/time.h"
 #include "base/waitable_event.h"
 
-
 #if defined(OS_MACOSX)
-#include <crt_externs.h>
-#define environ (*_NSGetEnviron())
 #include "base/mach_ipc_mac.h"
-#else
-extern char** environ;
 #endif
 
 const int kMicrosecondsPerSecond = 1000000;
@@ -179,26 +173,24 @@ class ScopedDIRClose {
 };
 typedef scoped_ptr_malloc<DIR, ScopedDIRClose> ScopedDIR;
 
+void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
 #if defined(OS_LINUX)
   static const rlim_t kSystemDefaultMaxFds = 8192;
-  static const char kFDDir[] = "/proc/self/fd";
+  static const char fd_dir[] = "/proc/self/fd";
 #elif defined(OS_MACOSX)
   static const rlim_t kSystemDefaultMaxFds = 256;
-  static const char kFDDir[] = "/dev/fd";
+  static const char fd_dir[] = "/dev/fd";
 #elif defined(OS_SOLARIS)
   static const rlim_t kSystemDefaultMaxFds = 8192;
-  static const char kFDDir[] = "/dev/fd";
+  static const char fd_dir[] = "/dev/fd";
 #elif defined(OS_FREEBSD)
   static const rlim_t kSystemDefaultMaxFds = 8192;
-  static const char kFDDir[] = "/dev/fd";
+  static const char fd_dir[] = "/dev/fd";
 #elif defined(OS_OPENBSD)
   static const rlim_t kSystemDefaultMaxFds = 256;
-  static const char kFDDir[] = "/dev/fd";
+  static const char fd_dir[] = "/dev/fd";
 #endif
-
-void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
-  // DANGER: no calls to malloc are allowed from now on:
-  // http://crbug.com/36678
+  std::set<int> saved_fds;
 
   // Get the maximum number of FDs possible.
   struct rlimit nofile;
@@ -214,20 +206,25 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
   if (max_fds > INT_MAX)
     max_fds = INT_MAX;
 
-  DirReaderPosix fd_dir(kFDDir);
+  // Don't close stdin, stdout and stderr
+  saved_fds.insert(STDIN_FILENO);
+  saved_fds.insert(STDOUT_FILENO);
+  saved_fds.insert(STDERR_FILENO);
+
+  for (base::InjectiveMultimap::const_iterator
+       i = saved_mapping.begin(); i != saved_mapping.end(); ++i) {
+    saved_fds.insert(i->dest);
+  }
+
+  ScopedDIR dir_closer(opendir(fd_dir));
+  DIR *dir = dir_closer.get();
+  if (NULL == dir) {
+    DLOG(ERROR) << "Unable to open " << fd_dir;
 
-  if (!fd_dir.IsValid()) {
     // Fallback case: Try every possible fd.
     for (rlim_t i = 0; i < max_fds; ++i) {
       const int fd = static_cast<int>(i);
-      if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
-        continue;
-      InjectiveMultimap::const_iterator i;
-      for (i = saved_mapping.begin(); i != saved_mapping.end(); i++) {
-        if (fd == i->dest)
-          break;
-      }
-      if (i != saved_mapping.end())
+      if (saved_fds.find(fd) != saved_fds.end())
         continue;
 
       // Since we're just trying to close anything we can find,
@@ -236,27 +233,20 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
     }
     return;
   }
+  int dir_fd = dirfd(dir);
 
-  const int dir_fd = fd_dir.fd();
-
-  for ( ; fd_dir.Next(); ) {
+  struct dirent *ent;
+  while ((ent = readdir(dir))) {
     // Skip . and .. entries.
-    if (fd_dir.name()[0] == '.')
+    if (ent->d_name[0] == '.')
       continue;
 
     char *endptr;
     errno = 0;
-    const long int fd = strtol(fd_dir.name(), &endptr, 10);
-    if (fd_dir.name()[0] == 0 || *endptr || fd < 0 || errno)
-      continue;
-    if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
+    const long int fd = strtol(ent->d_name, &endptr, 10);
+    if (ent->d_name[0] == 0 || *endptr || fd < 0 || errno)
       continue;
-    InjectiveMultimap::const_iterator i;
-    for (i = saved_mapping.begin(); i != saved_mapping.end(); i++) {
-      if (fd == i->dest)
-        break;
-    }
-    if (i != saved_mapping.end())
+    if (saved_fds.find(fd) != saved_fds.end())
       continue;
     if (fd == dir_fd)
       continue;
@@ -272,6 +262,41 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
   }
 }
 
+// Sets all file descriptors to close on exec except for stdin, stdout
+// and stderr.
+// TODO(agl): Remove this function. It's fundamentally broken for multithreaded
+// apps.
+void SetAllFDsToCloseOnExec() {
+#if defined(OS_LINUX)
+  const char fd_dir[] = "/proc/self/fd";
+#elif defined(OS_MACOSX) || defined(OS_FREEBSD) || defined(OS_OPENBSD) || \
+    defined(OS_SOLARIS)
+  const char fd_dir[] = "/dev/fd";
+#endif
+  ScopedDIR dir_closer(opendir(fd_dir));
+  DIR *dir = dir_closer.get();
+  if (NULL == dir) {
+    DLOG(ERROR) << "Unable to open " << fd_dir;
+    return;
+  }
+
+  struct dirent *ent;
+  while ((ent = readdir(dir))) {
+    // Skip . and .. entries.
+    if (ent->d_name[0] == '.')
+      continue;
+    int i = atoi(ent->d_name);
+    // We don't close stdin, stdout or stderr.
+    if (i <= STDERR_FILENO)
+      continue;
+
+    int flags = fcntl(i, F_GETFD);
+    if ((flags == -1) || (fcntl(i, F_SETFD, flags | FD_CLOEXEC) == -1)) {
+      DLOG(ERROR) << "fcntl failure.";
+    }
+  }
+}
+
 #if defined(OS_MACOSX)
 static std::string MachErrorCode(kern_return_t err) {
   return StringPrintf("0x%x %s", err, mach_error_string(err));
@@ -333,144 +358,21 @@ static pid_t fork_and_get_task(task_t* child_task) {
 }
 
 bool LaunchApp(const std::vector<std::string>& argv,
-               const environment_vector& env_changes,
+               const environment_vector& environ,
                const file_handle_mapping_vector& fds_to_remap,
                bool wait, ProcessHandle* process_handle) {
   return LaunchAppAndGetTask(
-      argv, env_changes, fds_to_remap, wait, NULL, process_handle);
+      argv, environ, fds_to_remap, wait, NULL, process_handle);
 }
 #endif  // defined(OS_MACOSX)
 
-// AlterEnvironment returns a modified environment vector, constructed from the
-// current environment and the list of changes given in |changes|. Each key in
-// the environment is matched against the first element of the pairs. In the
-// event of a match, the value is replaced by the second of the pair, unless
-// the second is empty, in which case the key-value is removed.
-//
-// The returned array is allocated using new[] and must be freed by the caller.
-static char** AlterEnvironment(const environment_vector& changes) {
-  unsigned count = 0;
-  unsigned size = 0;
-
-  // First assume that all of the current environment will be included.
-  for (unsigned i = 0; environ[i]; i++) {
-    const char *const pair = environ[i];
-    count++;
-    size += strlen(pair) + 1 /* terminating NUL */;
-  }
-
-  for (environment_vector::const_iterator
-       j = changes.begin(); j != changes.end(); j++) {
-    bool found = false;
-    const char *pair;
-
-    for (unsigned i = 0; environ[i]; i++) {
-      pair = environ[i];
-      const char *const equals = strchr(pair, '=');
-      if (!equals)
-        continue;
-      const unsigned keylen = equals - pair;
-      if (keylen == j->first.size() &&
-          memcmp(pair, j->first.data(), keylen) == 0) {
-        found = true;
-        break;
-      }
-    }
-
-    // if found, we'll either be deleting or replacing this element.
-    if (found) {
-      count--;
-      size -= strlen(pair) + 1;
-      if (j->second.size())
-        found = false;
-    }
-
-    // if !found, then we have a new element to add.
-    if (!found) {
-      count++;
-      size += j->first.size() + 1 /* '=' */ + j->second.size() + 1 /* NUL */;
-    }
-  }
-
-  uint8_t *buffer = new uint8_t[sizeof(char*) * count + size];
-  char **const ret = reinterpret_cast<char**>(buffer);
-  unsigned k = 0;
-  char *scratch = reinterpret_cast<char*>(buffer + sizeof(char*) * count);
-
-  for (unsigned i = 0; environ[i]; i++) {
-    const char *const pair = environ[i];
-    const char *const equals = strchr(pair, '=');
-    if (!equals) {
-      const unsigned len = strlen(pair);
-      ret[k++] = scratch;
-      memcpy(scratch, pair, len + 1);
-      scratch += len + 1;
-      continue;
-    }
-    const unsigned keylen = equals - pair;
-    bool handled = false;
-    for (environment_vector::const_iterator
-         j = changes.begin(); j != changes.end(); j++) {
-      if (j->first.size() == keylen &&
-          memcmp(j->first.data(), pair, keylen) == 0) {
-        if (!j->second.empty()) {
-          ret[k++] = scratch;
-          memcpy(scratch, pair, keylen + 1);
-          scratch += keylen + 1;
-          memcpy(scratch, j->second.c_str(), j->second.size() + 1);
-          scratch += j->second.size() + 1;
-        }
-        handled = true;
-        break;
-      }
-    }
-
-    if (!handled) {
-      const unsigned len = strlen(pair);
-      ret[k++] = scratch;
-      memcpy(scratch, pair, len + 1);
-      scratch += len + 1;
-    }
-  }
-
-  // Now handle new elements
-  for (environment_vector::const_iterator
-       j = changes.begin(); j != changes.end(); j++) {
-    bool found = false;
-    for (unsigned i = 0; environ[i]; i++) {
-      const char *const pair = environ[i];
-      const char *const equals = strchr(pair, '=');
-      if (!equals)
-        continue;
-      const unsigned keylen = equals - pair;
-      if (keylen == j->first.size() &&
-          memcmp(pair, j->first.data(), keylen) == 0) {
-        found = true;
-        break;
-      }
-    }
-
-    if (!found) {
-      ret[k++] = scratch;
-      memcpy(scratch, j->first.data(), j->first.size());
-      scratch += j->first.size();
-      *scratch++ = '=';
-      memcpy(scratch, j->second.c_str(), j->second.size() + 1);
-      scratch += j->second.size() + 1;
-     }
-  }
-
-  ret[k] = NULL;
-  return ret;
-}
-
 #if defined(OS_MACOSX)
 bool LaunchAppAndGetTask(
 #else
 bool LaunchApp(
 #endif
     const std::vector<std::string>& argv,
-    const environment_vector& env_changes,
+    const environment_vector& environ,
     const file_handle_mapping_vector& fds_to_remap,
     bool wait,
 #if defined(OS_MACOSX)
@@ -478,12 +380,6 @@ bool LaunchApp(
 #endif
     ProcessHandle* process_handle) {
   pid_t pid;
-  InjectiveMultimap fd_shuffle1, fd_shuffle2;
-  fd_shuffle1.reserve(fds_to_remap.size());
-  fd_shuffle2.reserve(fds_to_remap.size());
-  scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
-  scoped_array<char*> new_environ(AlterEnvironment(env_changes));
-
 #if defined(OS_MACOSX)
   if (task_handle == NULL) {
     pid = fork();
@@ -507,44 +403,45 @@ bool LaunchApp(
     RestoreDefaultExceptionHandler();
 #endif
 
-#if 0
-    // When debugging it can be helpful to check that we really aren't making
-    // any hidden calls to malloc.
-    void *malloc_thunk =
-        reinterpret_cast<void*>(reinterpret_cast<intptr_t>(malloc) & ~4095);
-    mprotect(malloc_thunk, 4096, PROT_READ | PROT_WRITE | PROT_EXEC);
-    memset(reinterpret_cast<void*>(malloc), 0xff, 8);
-#endif
-
-    // DANGER: no calls to malloc are allowed from now on:
-    // http://crbug.com/36678
-
+    InjectiveMultimap fd_shuffle;
     for (file_handle_mapping_vector::const_iterator
         it = fds_to_remap.begin(); it != fds_to_remap.end(); ++it) {
-      fd_shuffle1.push_back(InjectionArc(it->first, it->second, false));
-      fd_shuffle2.push_back(InjectionArc(it->first, it->second, false));
+      fd_shuffle.push_back(InjectionArc(it->first, it->second, false));
     }
 
-    environ = new_environ.get();
+    for (environment_vector::const_iterator it = environ.begin();
+         it != environ.end(); ++it) {
+      if (it->first.empty())
+        continue;
+
+      if (it->second.empty()) {
+        unsetenv(it->first.c_str());
+      } else {
+        setenv(it->first.c_str(), it->second.c_str(), 1);
+      }
+    }
 
     // Obscure fork() rule: in the child, if you don't end up doing exec*(),
     // you call _exit() instead of exit(). This is because _exit() does not
     // call any previously-registered (in the parent) exit handlers, which
     // might do things like block waiting for threads that don't even exist
     // in the child.
-
-    // fd_shuffle1 is mutated by this call because it cannot malloc.
-    if (!ShuffleFileDescriptors(&fd_shuffle1))
+    if (!ShuffleFileDescriptors(fd_shuffle))
       _exit(127);
 
-    CloseSuperfluousFds(fd_shuffle2);
+    // If we are using the SUID sandbox, it sets a magic environment variable
+    // ("SBX_D"), so we remove that variable from the environment here on the
+    // off chance that it's already set.
+    unsetenv("SBX_D");
+
+    CloseSuperfluousFds(fd_shuffle);
 
+    scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
     for (size_t i = 0; i < argv.size(); i++)
       argv_cstr[i] = const_cast<char*>(argv[i].c_str());
     argv_cstr[argv.size()] = NULL;
     execvp(argv_cstr[0], argv_cstr.get());
-    RAW_LOG(ERROR, "LaunchApp: failed to execvp:");
-    RAW_LOG(ERROR, argv_cstr[0]);
+    PLOG(ERROR) << "LaunchApp: execvp(" << argv_cstr[0] << ") failed";
     _exit(127);
   } else {
     // Parent process
@@ -737,12 +634,6 @@ static bool GetAppOutputInternal(const CommandLine& cl, char* const envp[],
                                  bool do_search_path) {
   int pipe_fd[2];
   pid_t pid;
-  InjectiveMultimap fd_shuffle1, fd_shuffle2;
-  const std::vector<std::string>& argv = cl.argv();
-  scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
-
-  fd_shuffle1.reserve(3);
-  fd_shuffle2.reserve(3);
 
   // Either |do_search_path| should be false or |envp| should be null, but not
   // both.
@@ -761,8 +652,6 @@ static bool GetAppOutputInternal(const CommandLine& cl, char* const envp[],
 #if defined(OS_MACOSX)
         RestoreDefaultExceptionHandler();
 #endif
-        // DANGER: no calls to malloc are allowed from now on:
-        // http://crbug.com/36678
 
         // Obscure fork() rule: in the child, if you don't end up doing exec*(),
         // you call _exit() instead of exit(). This is because _exit() does not
@@ -773,20 +662,18 @@ static bool GetAppOutputInternal(const CommandLine& cl, char* const envp[],
         if (dev_null < 0)
           _exit(127);
 
-        fd_shuffle1.push_back(InjectionArc(pipe_fd[1], STDOUT_FILENO, true));
-        fd_shuffle1.push_back(InjectionArc(dev_null, STDERR_FILENO, true));
-        fd_shuffle1.push_back(InjectionArc(dev_null, STDIN_FILENO, true));
-        // Adding another element here? Remeber to increase the argument to
-        // reserve(), above.
-
-        std::copy(fd_shuffle1.begin(), fd_shuffle1.end(),
-                  std::back_inserter(fd_shuffle2));
+        InjectiveMultimap fd_shuffle;
+        fd_shuffle.push_back(InjectionArc(pipe_fd[1], STDOUT_FILENO, true));
+        fd_shuffle.push_back(InjectionArc(dev_null, STDERR_FILENO, true));
+        fd_shuffle.push_back(InjectionArc(dev_null, STDIN_FILENO, true));
 
-        if (!ShuffleFileDescriptors(&fd_shuffle1))
+        if (!ShuffleFileDescriptors(fd_shuffle))
           _exit(127);
 
-        CloseSuperfluousFds(fd_shuffle2);
+        CloseSuperfluousFds(fd_shuffle);
 
+        const std::vector<std::string> argv = cl.argv();
+        scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
         for (size_t i = 0; i < argv.size(); i++)
           argv_cstr[i] = const_cast<char*>(argv[i].c_str());
         argv_cstr[argv.size()] = NULL;
diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc
index d22a60a..3170b6a 100644
--- a/base/process_util_unittest.cc
+++ b/base/process_util_unittest.cc
@@ -294,48 +294,6 @@ TEST_F(ProcessUtilTest, FDRemapping) {
   DPCHECK(ret == 0);
 }
 
-static std::string TestLaunchApp(const base::environment_vector& env_changes) {
-  std::vector<std::string> args;
-  base::file_handle_mapping_vector fds_to_remap;
-  ProcessHandle handle;
-
-  args.push_back("bash");
-  args.push_back("-c");
-  args.push_back("echo $BASE_TEST");
-
-  int fds[2];
-  PCHECK(pipe(fds) == 0);
-
-  fds_to_remap.push_back(std::make_pair(fds[1], 1));
-  EXPECT_TRUE(LaunchApp(args, env_changes, fds_to_remap,
-                        true /* wait for exit */, &handle));
-  PCHECK(close(fds[1]) == 0);
-
-  char buf[32];
-  const ssize_t n = HANDLE_EINTR(read(fds[0], buf, sizeof(buf)));
-  PCHECK(n > 0);
-  return std::string(buf, n);
-}
-
-TEST_F(ProcessUtilTest, LaunchApp) {
-  base::environment_vector env_changes;
-
-  env_changes.push_back(std::make_pair(std::string("BASE_TEST"),
-                                       std::string("bar")));
-  EXPECT_EQ("bar\n", TestLaunchApp(env_changes));
-  env_changes.clear();
-
-  setenv("BASE_TEST", "testing", 1 /* override */);
-  EXPECT_EQ("testing\n", TestLaunchApp(env_changes));
-
-  env_changes.push_back(std::make_pair(std::string("BASE_TEST"),
-                                       std::string("")));
-  EXPECT_EQ("\n", TestLaunchApp(env_changes));
-
-  env_changes[0].second = "foo";
-  EXPECT_EQ("foo\n", TestLaunchApp(env_changes));
-}
-
 TEST_F(ProcessUtilTest, GetAppOutput) {
   std::string output;
   EXPECT_TRUE(GetAppOutput(CommandLine(FilePath("true")), &output));