Remove base::WaitForExitCode*

base/process/kill.h

Index: base/process/kill.h
diff --git a/base/process/kill.h b/base/process/kill.h
index 8c0a213daf975ffe6a1d80d583617ceedfb73113..173317b0005442ee9250515490f70e379eebbf63 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.
@@ -152,6 +136,19 @@ BASE_EXPORT void EnsureProcessTerminated(Process process);
 BASE_EXPORT void EnsureProcessGetsReaped(ProcessId pid);
 #endif
 
+#if defined(OS_POSIX)
+namespace internal {
+// Implements Process::WaitForExitWithTimeout for POSIX. It lives in this file
+// because the implementation is closely related to other functions declared

[danakj, 2015/02/26 22:03:25]

Why does this argument make sense for posix but not for win?

[rvargas (doing something else), 2015/02/27 01:54:18]

Because the windows implementation has nothing to do with other functions
implemented in kill_xx.cc.

Posix, on the other hand, relies on WaitForSingleNonChildProcess (although I
could move that too) and WaitpidWithTimeout (I could move that too),
!defined(OS_NACL_NONSFI) (could add that too), and it is somewhat similar to
other complex code like KillProcess() (and GetKnownDeadTerminationStatus?).

I don't want to split all that code so that it is less apparent that we have
multiple functions that do almost the same but not really.

[danakj, 2015/02/27 18:47:02]

Granted the posix code is larger, with some helper functions, but I don't think
that's a reason to leave it in the wrong place.

kill_win.cc has a KillProcess() with similar code to
Process::WaitForExitWithTimeout in process_win.cc also, so this seems
inconsistent still to me.
I think we should do the same thing with posix and win here. I'll be happiest
then. I lean slightly toward moving them both to process_foo because then we
don't have to add "Don't use this" type functions to header files, and maybe
comments could be added to point out similarities if that really makes sense?

+// here.
+// This is not meant for consumers outside of base::Process.

[danakj, 2015/02/26 22:03:25]

Can you still keep the whole comment that was on this function describing its
functionality before?

[rvargas (doing something else), 2015/02/27 01:54:18]

Some of that text is now irrelevant (the part about closing the handle). The
documentation is basically the same comment of Process::WaitForExitWithTimeout
so I thought it is better to point that out than to copy/paste a comment from
somewhere else, especially because the intention is to have a single user for
this function (not an API that is really meant to be used by anyone else). BTW,
the most up to date comment lives on process.h

What if I rename it to WaitForExitWithTimeoutImpl ? It is pretty standard not to
explain in detail what WxxImpl does. (done)

+bool WaitForExitCodeWithTimeout(ProcessHandle handle,
+                                int* exit_code,
+                                TimeDelta timeout);
+static const int kNoTimeout = -1;

[danakj, 2015/02/26 22:03:25]

maybe you can scope this constant name a bit more.. kWaitForExitCodeNoTimeout or
something?

[rvargas (doing something else), 2015/02/27 01:54:18]

Removed. 

+}  // namespace internal
+#endif  // defined(OS_POSIX)
+
 }  // namespace base
 
 #endif  // BASE_PROCESS_KILL_H_