Remove base::WaitForExitCode*

BUG=417532

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

Cr-Commit-Position: refs/heads/master@{#318939}

6 files changed, 213 insertions, 251 deletions

diff --git a/base/process/kill.h b/base/process/kill.h
index 8c0a213..fb6423f 100644
--- a/base/process/kill.h
+++ b/base/process/kill.h
@@ -94,22 +94,6 @@ BASE_EXPORT TerminationStatus GetKnownDeadTerminationStatus(
     ProcessHandle handle, int* exit_code);
 #endif  // defined(OS_POSIX)
 
-// Waits for process to exit. On 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.
-BASE_EXPORT bool WaitForExitCode(ProcessHandle handle, int* exit_code);
-
-// Waits for process to exit. If it did exit within |timeout_milliseconds|,
-// then puts the exit code in |exit_code|, and returns true.
-// In POSIX systems, if the process has been signaled then |exit_code| is set
-// to -1. Returns false on failure (the caller is then responsible for closing
-// |handle|).
-// The caller is always responsible for closing the |handle|.
-BASE_EXPORT bool WaitForExitCodeWithTimeout(ProcessHandle handle,
-                                            int* exit_code,
-                                            base::TimeDelta timeout);
-
 // Wait for all the processes based on the named executable to exit.  If filter
 // is non-null, then only processes selected by the filter are waited on.
 // Returns after all processes have exited or wait_milliseconds have expired.
diff --git a/base/process/kill_posix.cc b/base/process/kill_posix.cc
index 77705ee..298486b 100644
--- a/base/process/kill_posix.cc
+++ b/base/process/kill_posix.cc
@@ -22,161 +22,6 @@ namespace base {
 
 namespace {
 
-#if !defined(OS_NACL_NONSFI)
-bool WaitpidWithTimeout(ProcessHandle handle,
-                        int* status,
-                        base::TimeDelta wait) {
-  // This POSIX version of this function only guarantees that we wait no less
-  // than |wait| for the process to exit.  The child process may
-  // exit sometime before the timeout has ended but we may still block for up
-  // to 256 milliseconds after the fact.
-  //
-  // waitpid() has no direct support on POSIX for specifying a timeout, you can
-  // either ask it to block indefinitely or return immediately (WNOHANG).
-  // When a child process terminates a SIGCHLD signal is sent to the parent.
-  // Catching this signal would involve installing a signal handler which may
-  // affect other parts of the application and would be difficult to debug.
-  //
-  // Our strategy is to call waitpid() once up front to check if the process
-  // has already exited, otherwise to loop for |wait|, sleeping for
-  // at most 256 milliseconds each time using usleep() and then calling
-  // waitpid().  The amount of time we sleep starts out at 1 milliseconds, and
-  // we double it every 4 sleep cycles.
-  //
-  // usleep() is speced to exit if a signal is received for which a handler
-  // has been installed.  This means that when a SIGCHLD is sent, it will exit
-  // depending on behavior external to this function.
-  //
-  // This function is used primarily for unit tests, if we want to use it in
-  // the application itself it would probably be best to examine other routes.
-
-  if (wait.InMilliseconds() == base::kNoTimeout) {
-    return HANDLE_EINTR(waitpid(handle, status, 0)) > 0;
-  }
-
-  pid_t ret_pid = HANDLE_EINTR(waitpid(handle, status, WNOHANG));
-  static const int64 kMaxSleepInMicroseconds = 1 << 18;  // ~256 milliseconds.
-  int64 max_sleep_time_usecs = 1 << 10;  // ~1 milliseconds.
-  int64 double_sleep_time = 0;
-
-  // If the process hasn't exited yet, then sleep and try again.
-  TimeTicks wakeup_time = TimeTicks::Now() + wait;
-  while (ret_pid == 0) {
-    TimeTicks now = TimeTicks::Now();
-    if (now > wakeup_time)
-      break;
-    // Guaranteed to be non-negative!
-    int64 sleep_time_usecs = (wakeup_time - now).InMicroseconds();
-    // Sleep for a bit while we wait for the process to finish.
-    if (sleep_time_usecs > max_sleep_time_usecs)
-      sleep_time_usecs = max_sleep_time_usecs;
-
-    // usleep() will return 0 and set errno to EINTR on receipt of a signal
-    // such as SIGCHLD.
-    usleep(sleep_time_usecs);
-    ret_pid = HANDLE_EINTR(waitpid(handle, status, WNOHANG));
-
-    if ((max_sleep_time_usecs < kMaxSleepInMicroseconds) &&
-        (double_sleep_time++ % 4 == 0)) {
-      max_sleep_time_usecs *= 2;
-    }
-  }
-
-  return ret_pid > 0;
-}
-
-#if defined(OS_MACOSX)
-// Using kqueue on Mac so that we can wait on non-child processes.
-// We can't use kqueues on child processes because we need to reap
-// our own children using wait.
-static bool WaitForSingleNonChildProcess(ProcessHandle handle,
-                                         TimeDelta wait) {
-  DCHECK_GT(handle, 0);
-  DCHECK(wait.InMilliseconds() == kNoTimeout || wait > TimeDelta());
-
-  ScopedFD kq(kqueue());
-  if (!kq.is_valid()) {
-    DPLOG(ERROR) << "kqueue";
-    return false;
-  }
-
-  struct kevent change = {0};
-  EV_SET(&change, handle, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL);
-  int result = HANDLE_EINTR(kevent(kq.get(), &change, 1, NULL, 0, NULL));
-  if (result == -1) {
-    if (errno == ESRCH) {
-      // If the process wasn't found, it must be dead.
-      return true;
-    }
-
-    DPLOG(ERROR) << "kevent (setup " << handle << ")";
-    return false;
-  }
-
-  // Keep track of the elapsed time to be able to restart kevent if it's
-  // interrupted.
-  bool wait_forever = wait.InMilliseconds() == kNoTimeout;
-  TimeDelta remaining_delta;
-  TimeTicks deadline;
-  if (!wait_forever) {
-    remaining_delta = wait;
-    deadline = TimeTicks::Now() + remaining_delta;
-  }
-
-  result = -1;
-  struct kevent event = {0};
-
-  while (wait_forever || remaining_delta > TimeDelta()) {
-    struct timespec remaining_timespec;
-    struct timespec* remaining_timespec_ptr;
-    if (wait_forever) {
-      remaining_timespec_ptr = NULL;
-    } else {
-      remaining_timespec = remaining_delta.ToTimeSpec();
-      remaining_timespec_ptr = &remaining_timespec;
-    }
-
-    result = kevent(kq.get(), NULL, 0, &event, 1, remaining_timespec_ptr);
-
-    if (result == -1 && errno == EINTR) {
-      if (!wait_forever) {
-        remaining_delta = deadline - TimeTicks::Now();
-      }
-      result = 0;
-    } else {
-      break;
-    }
-  }
-
-  if (result < 0) {
-    DPLOG(ERROR) << "kevent (wait " << handle << ")";
-    return false;
-  } else if (result > 1) {
-    DLOG(ERROR) << "kevent (wait " << handle << "): unexpected result "
-                << result;
-    return false;
-  } else if (result == 0) {
-    // Timed out.
-    return false;
-  }
-
-  DCHECK_EQ(result, 1);
-
-  if (event.filter != EVFILT_PROC ||
-      (event.fflags & NOTE_EXIT) == 0 ||
-      event.ident != static_cast<uintptr_t>(handle)) {
-    DLOG(ERROR) << "kevent (wait " << handle
-                << "): unexpected event: filter=" << event.filter
-                << ", fflags=" << event.fflags
-                << ", ident=" << event.ident;
-    return false;
-  }
-
-  return true;
-}
-#endif  // OS_MACOSX
-#endif  // !defined(OS_NACL_NONSFI)
-
 TerminationStatus GetTerminationStatusImpl(ProcessHandle handle,
                                            bool can_block,
                                            int* exit_code) {
@@ -302,52 +147,6 @@ TerminationStatus GetKnownDeadTerminationStatus(ProcessHandle handle,
 }
 
 #if !defined(OS_NACL_NONSFI)
-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;
-}
-
-bool WaitForExitCodeWithTimeout(ProcessHandle handle,
-                                int* exit_code,
-                                TimeDelta timeout) {
-  ProcessHandle parent_pid = GetParentProcessId(handle);
-  ProcessHandle our_pid = GetCurrentProcessHandle();
-  if (parent_pid != our_pid) {
-#if defined(OS_MACOSX)
-    // On Mac we can wait on non child processes.
-    return WaitForSingleNonChildProcess(handle, timeout);
-#else
-    // Currently on Linux we can't handle non child processes.
-    NOTIMPLEMENTED();
-#endif  // OS_MACOSX
-  }
-
-  int status;
-  if (!WaitpidWithTimeout(handle, &status, timeout))
-    return false;
-  if (WIFSIGNALED(status)) {
-    *exit_code = -1;
-    return true;
-  }
-  if (WIFEXITED(status)) {
-    *exit_code = WEXITSTATUS(status);
-    return true;
-  }
-  return false;
-}
-
 bool WaitForProcessesToExit(const FilePath::StringType& executable_name,
                             TimeDelta wait,
                             const ProcessFilter* filter) {
diff --git a/base/process/kill_win.cc b/base/process/kill_win.cc
index 387c9e2..4f0f2ef 100644
--- a/base/process/kill_win.cc
+++ b/base/process/kill_win.cc
@@ -175,26 +175,6 @@ TerminationStatus GetTerminationStatus(ProcessHandle handle, int* exit_code) {
   }
 }
 
-bool WaitForExitCode(ProcessHandle handle, int* exit_code) {
-  // TODO(rvargas) crbug.com/417532: Remove this function.
-  Process process(handle);
-  return process.WaitForExit(exit_code);
-}
-
-bool WaitForExitCodeWithTimeout(ProcessHandle handle,
-                                int* exit_code,
-                                TimeDelta timeout) {
-  if (::WaitForSingleObject(
-      handle, static_cast<DWORD>(timeout.InMilliseconds())) != WAIT_OBJECT_0)
-    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;
-}
-
 bool WaitForProcessesToExit(const FilePath::StringType& executable_name,
                             TimeDelta wait,
                             const ProcessFilter* filter) {
diff --git a/base/process/launch_posix.cc b/base/process/launch_posix.cc
index 203b7c8..f9963fa 100644
--- a/base/process/launch_posix.cc
+++ b/base/process/launch_posix.cc
@@ -33,7 +33,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/posix/eintr_wrapper.h"
-#include "base/process/kill.h"
+#include "base/process/process.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/waitable_event.h"
@@ -684,7 +684,8 @@ static GetAppOutputInternalResult GetAppOutputInternal(
 
         // Always wait for exit code (even if we know we'll declare
         // GOT_MAX_OUTPUT).
-        bool success = WaitForExitCode(pid, exit_code);
+        Process process(pid);
+        bool success = process.WaitForExit(exit_code);
 
         // If we stopped because we read as much as we wanted, we return
         // GOT_MAX_OUTPUT (because the child may exit due to |SIGPIPE|).
diff --git a/base/process/process_posix.cc b/base/process/process_posix.cc
index bc2f3f8..a36bf77 100644
--- a/base/process/process_posix.cc
+++ b/base/process/process_posix.cc
@@ -5,12 +5,206 @@
 #include "base/process/process.h"
 
 #include <sys/resource.h>
-#include <sys/time.h>
-#include <sys/types.h>
+#include <sys/wait.h>
 
+#include "base/files/scoped_file.h"
 #include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
 #include "base/process/kill.h"
 
+#if defined(OS_MACOSX)
+#include <sys/event.h>
+#endif
+
+namespace {
+
+#if !defined(OS_NACL_NONSFI)
+
+bool WaitpidWithTimeout(base::ProcessHandle handle,
+                        int* status,
+                        base::TimeDelta wait) {
+  // This POSIX version of this function only guarantees that we wait no less
+  // than |wait| for the process to exit.  The child process may
+  // exit sometime before the timeout has ended but we may still block for up
+  // to 256 milliseconds after the fact.
+  //
+  // waitpid() has no direct support on POSIX for specifying a timeout, you can
+  // either ask it to block indefinitely or return immediately (WNOHANG).
+  // When a child process terminates a SIGCHLD signal is sent to the parent.
+  // Catching this signal would involve installing a signal handler which may
+  // affect other parts of the application and would be difficult to debug.
+  //
+  // Our strategy is to call waitpid() once up front to check if the process
+  // has already exited, otherwise to loop for |wait|, sleeping for
+  // at most 256 milliseconds each time using usleep() and then calling
+  // waitpid().  The amount of time we sleep starts out at 1 milliseconds, and
+  // we double it every 4 sleep cycles.
+  //
+  // usleep() is speced to exit if a signal is received for which a handler
+  // has been installed.  This means that when a SIGCHLD is sent, it will exit
+  // depending on behavior external to this function.
+  //
+  // This function is used primarily for unit tests, if we want to use it in
+  // the application itself it would probably be best to examine other routes.
+
+  if (wait == base::TimeDelta::Max()) {
+    return HANDLE_EINTR(waitpid(handle, status, 0)) > 0;
+  }
+
+  pid_t ret_pid = HANDLE_EINTR(waitpid(handle, status, WNOHANG));
+  static const int64 kMaxSleepInMicroseconds = 1 << 18;  // ~256 milliseconds.
+  int64 max_sleep_time_usecs = 1 << 10;  // ~1 milliseconds.
+  int64 double_sleep_time = 0;
+
+  // If the process hasn't exited yet, then sleep and try again.
+  base::TimeTicks wakeup_time = base::TimeTicks::Now() + wait;
+  while (ret_pid == 0) {
+    base::TimeTicks now = base::TimeTicks::Now();
+    if (now > wakeup_time)
+      break;
+    // Guaranteed to be non-negative!
+    int64 sleep_time_usecs = (wakeup_time - now).InMicroseconds();
+    // Sleep for a bit while we wait for the process to finish.
+    if (sleep_time_usecs > max_sleep_time_usecs)
+      sleep_time_usecs = max_sleep_time_usecs;
+
+    // usleep() will return 0 and set errno to EINTR on receipt of a signal
+    // such as SIGCHLD.
+    usleep(sleep_time_usecs);
+    ret_pid = HANDLE_EINTR(waitpid(handle, status, WNOHANG));
+
+    if ((max_sleep_time_usecs < kMaxSleepInMicroseconds) &&
+        (double_sleep_time++ % 4 == 0)) {
+      max_sleep_time_usecs *= 2;
+    }
+  }
+
+  return ret_pid > 0;
+}
+
+#if defined(OS_MACOSX)
+// Using kqueue on Mac so that we can wait on non-child processes.
+// We can't use kqueues on child processes because we need to reap
+// our own children using wait.
+static bool WaitForSingleNonChildProcess(base::ProcessHandle handle,
+                                         base::TimeDelta wait) {
+  DCHECK_GT(handle, 0);
+  DCHECK_GT(wait, base::TimeDelta());
+
+  base::ScopedFD kq(kqueue());
+  if (!kq.is_valid()) {
+    DPLOG(ERROR) << "kqueue";
+    return false;
+  }
+
+  struct kevent change = {0};
+  EV_SET(&change, handle, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL);
+  int result = HANDLE_EINTR(kevent(kq.get(), &change, 1, NULL, 0, NULL));
+  if (result == -1) {
+    if (errno == ESRCH) {
+      // If the process wasn't found, it must be dead.
+      return true;
+    }
+
+    DPLOG(ERROR) << "kevent (setup " << handle << ")";
+    return false;
+  }
+
+  // Keep track of the elapsed time to be able to restart kevent if it's
+  // interrupted.
+  bool wait_forever = (wait == base::TimeDelta::Max());
+  base::TimeDelta remaining_delta;
+  base::TimeTicks deadline;
+  if (!wait_forever) {
+    remaining_delta = wait;
+    deadline = base::TimeTicks::Now() + remaining_delta;
+  }
+
+  result = -1;
+  struct kevent event = {0};
+
+  while (wait_forever || remaining_delta > base::TimeDelta()) {
+    struct timespec remaining_timespec;
+    struct timespec* remaining_timespec_ptr;
+    if (wait_forever) {
+      remaining_timespec_ptr = NULL;
+    } else {
+      remaining_timespec = remaining_delta.ToTimeSpec();
+      remaining_timespec_ptr = &remaining_timespec;
+    }
+
+    result = kevent(kq.get(), NULL, 0, &event, 1, remaining_timespec_ptr);
+
+    if (result == -1 && errno == EINTR) {
+      if (!wait_forever) {
+        remaining_delta = deadline - base::TimeTicks::Now();
+      }
+      result = 0;
+    } else {
+      break;
+    }
+  }
+
+  if (result < 0) {
+    DPLOG(ERROR) << "kevent (wait " << handle << ")";
+    return false;
+  } else if (result > 1) {
+    DLOG(ERROR) << "kevent (wait " << handle << "): unexpected result "
+                << result;
+    return false;
+  } else if (result == 0) {
+    // Timed out.
+    return false;
+  }
+
+  DCHECK_EQ(result, 1);
+
+  if (event.filter != EVFILT_PROC ||
+      (event.fflags & NOTE_EXIT) == 0 ||
+      event.ident != static_cast<uintptr_t>(handle)) {
+    DLOG(ERROR) << "kevent (wait " << handle
+                << "): unexpected event: filter=" << event.filter
+                << ", fflags=" << event.fflags
+                << ", ident=" << event.ident;
+    return false;
+  }
+
+  return true;
+}
+#endif  // OS_MACOSX
+
+bool WaitForExitWithTimeoutImpl(base::ProcessHandle handle,
+                                int* exit_code,
+                                base::TimeDelta timeout) {
+  base::ProcessHandle parent_pid = base::GetParentProcessId(handle);
+  base::ProcessHandle our_pid = base::GetCurrentProcessHandle();
+  if (parent_pid != our_pid) {
+#if defined(OS_MACOSX)
+    // On Mac we can wait on non child processes.
+    return WaitForSingleNonChildProcess(handle, timeout);
+#else
+    // Currently on Linux we can't handle non child processes.
+    NOTIMPLEMENTED();
+#endif  // OS_MACOSX
+  }
+
+  int status;
+  if (!WaitpidWithTimeout(handle, &status, timeout))
+    return false;
+  if (WIFSIGNALED(status)) {
+    *exit_code = -1;
+    return true;
+  }
+  if (WIFEXITED(status)) {
+    *exit_code = WEXITSTATUS(status);
+    return true;
+  }
+  return false;
+}
+#endif  // !defined(OS_NACL_NONSFI)
+
+}  // namespace
+
 namespace base {
 
 Process::Process(ProcessHandle handle) : process_(handle) {
@@ -102,15 +296,11 @@ void Process::Terminate(int result_code) {
 }
 
 bool Process::WaitForExit(int* exit_code) {
-  // TODO(rvargas) crbug.com/417532: Remove this constant.
-  const int kNoTimeout = -1;
-  return WaitForExitWithTimeout(TimeDelta::FromMilliseconds(kNoTimeout),
-                                exit_code);
+  return WaitForExitWithTimeout(TimeDelta::Max(), exit_code);
 }
 
 bool Process::WaitForExitWithTimeout(TimeDelta timeout, int* exit_code) {
-  // TODO(rvargas) crbug.com/417532: Move the implementation here.
-  return base::WaitForExitCodeWithTimeout(Handle(), exit_code, timeout);
+  return WaitForExitWithTimeoutImpl(Handle(), exit_code, timeout);
 }
 
 #if !defined(OS_LINUX)
diff --git a/base/process/process_win.cc b/base/process/process_win.cc
index 8e5360b..b62fdb4f 100644
--- a/base/process/process_win.cc
+++ b/base/process/process_win.cc
@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/field_trial.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/process/kill.h"
 #include "base/win/windows_version.h"
 
@@ -141,10 +142,17 @@ bool Process::WaitForExit(int* exit_code) {
 }
 
 bool Process::WaitForExitWithTimeout(TimeDelta timeout, int* exit_code) {
-  // TODO(rvargas) crbug.com/417532: Move the implementation here.
-  if (timeout > TimeDelta::FromMilliseconds(INFINITE))
-    timeout = TimeDelta::FromMilliseconds(INFINITE);
-  return base::WaitForExitCodeWithTimeout(Handle(), exit_code, timeout);
+  // Limit timeout to INFINITE.
+  DWORD timeout_ms = saturated_cast<DWORD>(timeout.InMilliseconds());
+  if (::WaitForSingleObject(Handle(), timeout_ms) != WAIT_OBJECT_0)
+    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;
 }
 
 bool Process::IsProcessBackgrounded() const {