Bug 913953 - Part h: Remove unused WaitForExitCode and GetAppOutput functions; r=ehsan
authorMs2ger <ms2ger@gmail.com>
Tue, 10 Sep 2013 09:03:33 +0200
changeset 159136 02d635ad7b7c70f214e7b0a669bf3ea9481180ea
parent 159135 22c979082c03f5dd0e3d1aaae91abf11f75cadb5
child 159137 2e9624b9aec15dc9fad811507e230d9fa645aba2
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs913953
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 913953 - Part h: Remove unused WaitForExitCode and GetAppOutput functions; r=ehsan
ipc/chromium/src/base/process_util.h
ipc/chromium/src/base/process_util_posix.cc
ipc/chromium/src/base/process_util_win.cc
--- a/ipc/chromium/src/base/process_util.h
+++ b/ipc/chromium/src/base/process_util.h
@@ -185,31 +185,16 @@ bool LaunchApp(const std::vector<std::st
 // returns if privileges were successfully adjusted.
 void SetCurrentProcessPrivileges(ChildPrivileges privs);
 
 // Executes the application specified by cl. This function delegates to one
 // of the above two platform-specific functions.
 bool LaunchApp(const CommandLine& cl,
                bool wait, bool start_hidden, ProcessHandle* process_handle);
 
-#if defined(OS_WIN)
-// Executes the application specified by |cmd_line| and copies the contents
-// printed to the standard output to |output|, which should be non NULL.
-// Blocks until the started process terminates.
-// Returns true if the application was run successfully, false otherwise.
-bool GetAppOutput(const std::wstring& cmd_line, std::string* output);
-#elif defined(OS_POSIX)
-// Executes the application specified by |cl| and wait for it to exit. Stores
-// the output (stdout) in |output|. Redirects stderr to /dev/null. Returns true
-// on success (application launched and exited cleanly, with exit code
-// indicating success). |output| is modified only when the function finished
-// successfully.
-bool GetAppOutput(const CommandLine& cl, std::string* output);
-#endif
-
 // Used to filter processes by process ID.
 class ProcessFilter {
  public:
   // Returns true to indicate set-inclusion and false otherwise.  This method
   // should not have side-effects and should be idempotent.
   virtual bool Includes(ProcessId pid, ProcessId parent_pid) const = 0;
   virtual ~ProcessFilter() { }
 };
@@ -224,22 +209,16 @@ bool KillProcess(ProcessHandle process, 
 // status indicates the process crashed. |child_exited| is set to true iff the
 // child process has terminated. (|child_exited| may be NULL.)
 //
 // On Windows, it is an error to call this if the process hasn't terminated
 // yet. On POSIX, |child_exited| is set correctly since we detect terminate in
 // a different manner on POSIX.
 bool DidProcessCrash(bool* child_exited, ProcessHandle handle);
 
-// Waits for process to exit. In POSIX systems, if the process hasn't been
-// signaled then puts the exit code in |exit_code|; otherwise it's considered
-// a failure. On Windows |exit_code| is always filled. Returns true on success,
-// and closes |handle| in any case.
-bool WaitForExitCode(ProcessHandle handle, int* exit_code);
-
 // This class provides a way to iterate through the list of processes
 // on the current machine that were started from the given executable
 // name.  To use, create an instance and then call NextProcessEntry()
 // until it returns false.
 class NamedProcessIterator {
  public:
   NamedProcessIterator(const std::wstring& executable_name,
                        const ProcessFilter* filter);
--- a/ipc/chromium/src/base/process_util_posix.cc
+++ b/ipc/chromium/src/base/process_util_posix.cc
@@ -275,33 +275,16 @@ bool DidProcessCrash(bool* child_exited,
   }
 
   if (WIFEXITED(status))
     return WEXITSTATUS(status) != 0;
 
   return false;
 }
 
-bool WaitForExitCode(ProcessHandle handle, int* exit_code) {
-  int status;
-  if (HANDLE_EINTR(waitpid(handle, &status, 0)) == -1) {
-    NOTREACHED();
-    return false;
-  }
-
-  if (WIFEXITED(status)) {
-    *exit_code = WEXITSTATUS(status);
-    return true;
-  }
-
-  // If it didn't exit cleanly, it must have been signaled.
-  DCHECK(WIFSIGNALED(status));
-  return false;
-}
-
 namespace {
 
 int64_t TimeValToMicroseconds(const struct timeval& tv) {
   return tv.tv_sec * kMicrosecondsPerSecond + tv.tv_usec;
 }
 
 }
 
@@ -339,91 +322,9 @@ int ProcessMetrics::GetCPUUsage() {
                              time_delta);
 
   last_system_time_ = system_time;
   last_time_ = time;
 
   return cpu;
 }
 
-bool GetAppOutput(const CommandLine& cl, std::string* output) {
-  int pipe_fd[2];
-  pid_t pid;
-
-  // Illegal to allocate memory after fork and before execvp
-  InjectiveMultimap fd_shuffle1, fd_shuffle2;
-  fd_shuffle1.reserve(3);
-  fd_shuffle2.reserve(3);
-  const std::vector<std::string>& argv = cl.argv();
-  scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
-
-  if (pipe(pipe_fd) < 0)
-    return false;
-
-  switch (pid = fork()) {
-    case -1:  // error
-      close(pipe_fd[0]);
-      close(pipe_fd[1]);
-      return false;
-    case 0:  // child
-      {
-        // 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.
-        int dev_null = open("/dev/null", O_WRONLY);
-        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));
-
-        // fd_shuffle1 is mutated by this call because it cannot malloc.
-        if (!ShuffleFileDescriptors(&fd_shuffle1))
-          _exit(127);
-
-        CloseSuperfluousFds(fd_shuffle2);
-
-        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());
-        _exit(127);
-      }
-    default:  // parent
-      {
-        // Close our writing end of pipe now. Otherwise later read would not
-        // be able to detect end of child's output (in theory we could still
-        // write to the pipe).
-        close(pipe_fd[1]);
-
-        int exit_code = EXIT_FAILURE;
-        bool success = WaitForExitCode(pid, &exit_code);
-        if (!success || exit_code != EXIT_SUCCESS) {
-          close(pipe_fd[0]);
-          return false;
-        }
-
-        char buffer[256];
-        std::string buf_output;
-
-        while (true) {
-          ssize_t bytes_read =
-              HANDLE_EINTR(read(pipe_fd[0], buffer, sizeof(buffer)));
-          if (bytes_read <= 0)
-            break;
-          buf_output.append(buffer, bytes_read);
-        }
-        output->swap(buf_output);
-        close(pipe_fd[0]);
-        return true;
-      }
-  }
-}
-
 }  // namespace base
--- a/ipc/chromium/src/base/process_util_win.cc
+++ b/ipc/chromium/src/base/process_util_win.cc
@@ -350,92 +350,16 @@ bool LaunchApp(const std::wstring& cmdli
 }
 
 bool LaunchApp(const CommandLine& cl,
                bool wait, bool start_hidden, ProcessHandle* process_handle) {
   return LaunchApp(cl.command_line_string(), wait,
                    start_hidden, process_handle);
 }
 
-bool GetAppOutput(const std::wstring& cmd_line, std::string* output) {
-  if (!output) {
-    NOTREACHED();
-    return false;
-  }
-
-  HANDLE out_read = NULL;
-  HANDLE out_write = NULL;
-
-  SECURITY_ATTRIBUTES sa_attr;
-  // Set the bInheritHandle flag so pipe handles are inherited.
-  sa_attr.nLength = sizeof(SECURITY_ATTRIBUTES);
-  sa_attr.bInheritHandle = TRUE;
-  sa_attr.lpSecurityDescriptor = NULL;
-
-  // Create the pipe for the child process's STDOUT.
-  if (!CreatePipe(&out_read, &out_write, &sa_attr, 0)) {
-    NOTREACHED() << "Failed to create pipe";
-    return false;
-  }
-
-  // Ensure we don't leak the handles.
-  ScopedHandle scoped_out_read(out_read);
-  ScopedHandle scoped_out_write(out_write);
-
-  // Ensure the read handle to the pipe for STDOUT is not inherited.
-  if (!SetHandleInformation(out_read, HANDLE_FLAG_INHERIT, 0)) {
-    NOTREACHED() << "Failed to disabled pipe inheritance";
-    return false;
-  }
-
-  // Now create the child process
-  PROCESS_INFORMATION proc_info = { 0 };
-  STARTUPINFO start_info = { 0 };
-
-  start_info.cb = sizeof(STARTUPINFO);
-  start_info.hStdOutput = out_write;
-  // Keep the normal stdin and stderr.
-  start_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
-  start_info.hStdError = GetStdHandle(STD_ERROR_HANDLE);
-  start_info.dwFlags |= STARTF_USESTDHANDLES;
-
-  // Create the child process.
-  if (!CreateProcess(NULL, const_cast<wchar_t*>(cmd_line.c_str()), NULL, NULL,
-                     TRUE,  // Handles are inherited.
-                     0, NULL, NULL, &start_info, &proc_info)) {
-    NOTREACHED() << "Failed to start process";
-    return false;
-  }
-
-  // We don't need the thread handle, close it now.
-  CloseHandle(proc_info.hThread);
-
-  // Close our writing end of pipe now. Otherwise later read would not be able
-  // to detect end of child's output.
-  scoped_out_write.Close();
-
-  // Read output from the child process's pipe for STDOUT
-  const int kBufferSize = 1024;
-  char buffer[kBufferSize];
-
-  for (;;) {
-    DWORD bytes_read = 0;
-    BOOL success = ReadFile(out_read, buffer, kBufferSize, &bytes_read, NULL);
-    if (!success || bytes_read == 0)
-      break;
-    output->append(buffer, bytes_read);
-  }
-
-  // Let's wait for the process to finish.
-  WaitForSingleObject(proc_info.hProcess, INFINITE);
-  CloseHandle(proc_info.hProcess);
-
-  return true;
-}
-
 bool KillProcess(ProcessHandle process, int exit_code, bool wait) {
   bool result = (TerminateProcess(process, exit_code) != FALSE);
   if (result && wait) {
     // The process may not end immediately due to pending I/O
     if (WAIT_OBJECT_0 != WaitForSingleObject(process, 60 * 1000))
       DLOG(ERROR) << "Error waiting for process exit: " << GetLastError();
   } else if (!result) {
     DLOG(ERROR) << "Unable to terminate process: " << GetLastError();
@@ -469,29 +393,16 @@ bool DidProcessCrash(bool* child_exited,
       exitcode == 0xC000013A ||     // Control-C/end session.
       exitcode == 0x40010004) {     // Debugger terminated process/end session.
     return false;
   }
 
   return true;
 }
 
-bool WaitForExitCode(ProcessHandle handle, int* exit_code) {
-  ScopedHandle closer(handle);  // Ensure that we always close the handle.
-  if (::WaitForSingleObject(handle, INFINITE) != WAIT_OBJECT_0) {
-    NOTREACHED();
-    return false;
-  }
-  DWORD temp_code;  // Don't clobber out-parameters in case of failure.
-  if (!::GetExitCodeProcess(handle, &temp_code))
-    return false;
-  *exit_code = temp_code;
-  return true;
-}
-
 void SetCurrentProcessPrivileges(ChildPrivileges privs) {
 
 }
 
 NamedProcessIterator::NamedProcessIterator(const std::wstring& executable_name,
                                            const ProcessFilter* filter)
     : started_iteration_(false),
       executable_name_(executable_name),