Remove base::WaitForSingleProcess

BUG=417532

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

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

apps/load_and_launch_browsertest.cc

@@ -62,8 +62,10 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
   ASSERT_TRUE(process.IsValid());
 
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-  ASSERT_TRUE(base::WaitForSingleProcess(process.Handle(),
-                                         TestTimeouts::action_timeout()));
+  int exit_code;
+  ASSERT_TRUE(process.WaitForExitWithTimeout(TestTimeouts::action_timeout(),
+                                             &exit_code));
+  ASSERT_EQ(0, exit_code);
 }
 
 // TODO(jackhou): Enable this test once it works on OSX. It currently does not
@@ -102,8 +104,10 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
   ASSERT_TRUE(process.IsValid());
 
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-  ASSERT_TRUE(base::WaitForSingleProcess(process.Handle(),
-                                         TestTimeouts::action_timeout()));
+  int exit_code;
+  ASSERT_TRUE(process.WaitForExitWithTimeout(TestTimeouts::action_timeout(),
+                                             &exit_code));
+  ASSERT_EQ(0, exit_code);
 }
 
 namespace {

base/debug/stack_trace_unittest.cc

@@ -148,8 +148,9 @@ MULTIPROCESS_TEST_MAIN(MismatchedMallocChildProcess) {
 TEST_F(StackTraceTest, AsyncSignalUnsafeSignalHandlerHang) {
   Process child = SpawnChild("MismatchedMallocChildProcess");
   ASSERT_TRUE(child.IsValid());
-  ASSERT_TRUE(WaitForSingleProcess(child.Handle(),
-                                   TestTimeouts::action_timeout()));
+  int exit_code;
+  ASSERT_TRUE(child.WaitForExitWithTimeout(TestTimeouts::action_timeout(),
+                                           &exit_code));
 }
 #endif  // !defined(OS_IOS)
 

base/process/kill.h

@@ -119,12 +119,6 @@ BASE_EXPORT bool WaitForProcessesToExit(
     base::TimeDelta wait,
     const ProcessFilter* filter);
 
-// Wait for a single process to exit. Return true if it exited cleanly within
-// the given time limit. On Linux |handle| must be a child process, however
-// on Mac and Windows it can be any process.
-BASE_EXPORT bool WaitForSingleProcess(ProcessHandle handle,
-                                      base::TimeDelta wait);
-
 // Waits a certain amount of time (can be 0) for all the processes with a given
 // executable name to exit, then kills off any of them that are still around.
 // If filter is non-null, then only processes selected by the filter are waited

base/process/kill_posix.cc

@@ -84,6 +84,97 @@ bool WaitpidWithTimeout(ProcessHandle handle,
 
   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,
@@ -230,7 +321,19 @@ bool WaitForExitCode(ProcessHandle handle, int* exit_code) {
 
 bool WaitForExitCodeWithTimeout(ProcessHandle handle,
                                 int* exit_code,
-                                base::TimeDelta timeout) {
+                                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;
@@ -246,138 +349,28 @@ bool WaitForExitCodeWithTimeout(ProcessHandle handle,
 }
 
 bool WaitForProcessesToExit(const FilePath::StringType& executable_name,
-                            base::TimeDelta wait,
+                            TimeDelta wait,
                             const ProcessFilter* filter) {
   bool result = false;
 
   // TODO(port): This is inefficient, but works if there are multiple procs.
   // TODO(port): use waitpid to avoid leaving zombies around
 
-  base::TimeTicks end_time = base::TimeTicks::Now() + wait;
+  TimeTicks end_time = TimeTicks::Now() + wait;
   do {
     NamedProcessIterator iter(executable_name, filter);
     if (!iter.NextProcessEntry()) {
       result = true;
       break;
     }
-    base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100));
-  } while ((end_time - base::TimeTicks::Now()) > base::TimeDelta());
+    PlatformThread::Sleep(TimeDelta::FromMilliseconds(100));
+  } while ((end_time - TimeTicks::Now()) > TimeDelta());
 
   return result;
 }
 
-#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,
-                                         base::TimeDelta wait) {
-  DCHECK_GT(handle, 0);
-  DCHECK(wait.InMilliseconds() == base::kNoTimeout || wait > base::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() == base::kNoTimeout;
-  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 WaitForSingleProcess(ProcessHandle handle, base::TimeDelta wait) {
-  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, wait);
-#else
-    // Currently on Linux we can't handle non child processes.
-    NOTIMPLEMENTED();
-#endif  // OS_MACOSX
-  }
-
-  int status;
-  if (!WaitpidWithTimeout(handle, &status, wait))
-    return false;
-  return WIFEXITED(status);
-}
-
 bool CleanupProcesses(const FilePath::StringType& executable_name,
-                      base::TimeDelta wait,
+                      TimeDelta wait,
                       int exit_code,
                       const ProcessFilter* filter) {
   bool exited_cleanly = WaitForProcessesToExit(executable_name, wait, filter);

base/process/kill_win.cc

@@ -189,7 +189,7 @@ bool WaitForExitCode(ProcessHandle handle, int* exit_code) {
 
 bool WaitForExitCodeWithTimeout(ProcessHandle handle,
                                 int* exit_code,
-                                base::TimeDelta timeout) {
+                                TimeDelta timeout) {
   if (::WaitForSingleObject(
       handle, static_cast<DWORD>(timeout.InMilliseconds())) != WAIT_OBJECT_0)
     return false;
@@ -202,7 +202,7 @@ bool WaitForExitCodeWithTimeout(ProcessHandle handle,
 }
 
 bool WaitForProcessesToExit(const FilePath::StringType& executable_name,
-                            base::TimeDelta wait,
+                            TimeDelta wait,
                             const ProcessFilter* filter) {
   bool result = true;
   DWORD start_time = GetTickCount();
@@ -224,13 +224,8 @@ bool WaitForProcessesToExit(const FilePath::StringType& executable_name,
   return result;
 }
 
-bool WaitForSingleProcess(ProcessHandle handle, base::TimeDelta wait) {
-  int exit_code;
-  return WaitForExitCodeWithTimeout(handle, &exit_code, wait) && exit_code == 0;
-}
-
 bool CleanupProcesses(const FilePath::StringType& executable_name,
-                      base::TimeDelta wait,
+                      TimeDelta wait,
                       int exit_code,
                       const ProcessFilter* filter) {
   if (WaitForProcessesToExit(executable_name, wait, filter))
@@ -249,9 +244,9 @@ void EnsureProcessTerminated(Process process) {
 
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&TimerExpiredTask::TimedOut,
-                 base::Owned(new TimerExpiredTask(process.Pass()))),
-      base::TimeDelta::FromMilliseconds(kWaitInterval));
+      Bind(&TimerExpiredTask::TimedOut,
+           Owned(new TimerExpiredTask(process.Pass()))),
+      TimeDelta::FromMilliseconds(kWaitInterval));
 }
 
 }  // namespace base

base/process/process.h

@@ -98,6 +98,8 @@ class BASE_EXPORT Process {
 
   // Waits for the process to exit. Returns true on success.
   // On POSIX, if the process has been signaled then |exit_code| is set to -1.
+  // On Linux this must be a child process, however on Mac and Windows it can be
+  // any process.
   bool WaitForExit(int* exit_code);
 
   // Same as WaitForExit() but only waits for up to |timeout|.

base/process/process_unittest.cc

@@ -124,7 +124,8 @@ TEST_F(ProcessTest, Terminate) {
   exit_code = kDummyExitCode;
   int kExpectedExitCode = 250;
   process.Terminate(kExpectedExitCode);
-  WaitForSingleProcess(process.Handle(), TestTimeouts::action_max_timeout());
+  process.WaitForExitWithTimeout(TestTimeouts::action_max_timeout(),
+                                 &exit_code);
 
   EXPECT_NE(TERMINATION_STATUS_STILL_RUNNING,
             GetTerminationStatus(process.Handle(), &exit_code));

base/process/process_util_unittest.cc

@@ -151,8 +151,9 @@ MULTIPROCESS_TEST_MAIN(SimpleChildProcess) {
 TEST_F(ProcessUtilTest, SpawnChild) {
   base::Process process = SpawnChild("SimpleChildProcess");
   ASSERT_TRUE(process.IsValid());
-  EXPECT_TRUE(base::WaitForSingleProcess(process.Handle(),
-                                         TestTimeouts::action_max_timeout()));
+  int exit_code;
+  EXPECT_TRUE(process.WaitForExitWithTimeout(
+                  TestTimeouts::action_max_timeout(), &exit_code));
 }
 
 MULTIPROCESS_TEST_MAIN(SlowChildProcess) {
@@ -167,8 +168,9 @@ TEST_F(ProcessUtilTest, KillSlowChild) {
   base::Process process = SpawnChild("SlowChildProcess");
   ASSERT_TRUE(process.IsValid());
   SignalChildren(signal_file.c_str());
-  EXPECT_TRUE(base::WaitForSingleProcess(process.Handle(),
-                                         TestTimeouts::action_max_timeout()));
+  int exit_code;
+  EXPECT_TRUE(process.WaitForExitWithTimeout(
+                  TestTimeouts::action_max_timeout(), &exit_code));
   remove(signal_file.c_str());
 }
 
@@ -550,12 +552,12 @@ int ProcessUtilTest::CountOpenFDsInChild() {
 
 #if defined(THREAD_SANITIZER)
   // Compiler-based ThreadSanitizer makes this test slow.
-  CHECK(base::WaitForSingleProcess(process.Handle(),
-                                   base::TimeDelta::FromSeconds(3)));
+  base::TimeDelta timeout = base::TimeDelta::FromSeconds(3);
 #else
-  CHECK(base::WaitForSingleProcess(process.Handle(),
-                                   base::TimeDelta::FromSeconds(1)));
+  base::TimeDelta timeout = base::TimeDelta::FromSeconds(1);
 #endif
+  int exit_code;
+  CHECK(process.WaitForExitWithTimeout(timeout, &exit_code));
   ret = IGNORE_EINTR(close(fds[0]));
   DPCHECK(ret == 0);
 
@@ -891,8 +893,9 @@ TEST_F(ProcessUtilTest, DelayedTermination) {
   base::Process child_process = SpawnChild("process_util_test_never_die");
   ASSERT_TRUE(child_process.IsValid());
   base::EnsureProcessTerminated(child_process.Duplicate());
-  base::WaitForSingleProcess(child_process.Handle(),
-                             base::TimeDelta::FromSeconds(5));
+  int exit_code;
+  child_process.WaitForExitWithTimeout(base::TimeDelta::FromSeconds(5),
+                                       &exit_code);
 
   // Check that process was really killed.
   EXPECT_TRUE(IsProcessDead(child_process.Handle()));

chrome/browser/apps/app_shim/app_shim_interactive_uitest_mac.mm

@@ -430,8 +430,10 @@ IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, MAYBE_HostedAppLaunch) {
     GetFirstHostedAppWindow()->window()->Close();
     // Wait for the window to be closed.
     listener.WaitUntilRemoved();
-    ASSERT_TRUE(
-        base::WaitForSingleProcess(shim_pid, TestTimeouts::action_timeout()));
+    base::Process shim_process(shim_pid);
+    int exit_code;
+    ASSERT_TRUE(shim_process.WaitForExitWithTimeout(
+                    TestTimeouts::action_timeout(), &exit_code));
 
     EXPECT_FALSE(GetFirstHostedAppWindow());
     EXPECT_FALSE(HasAppShimHost(profile(), app->id()));
@@ -502,8 +504,10 @@ IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, MAYBE_Launch) {
     pid_t shim_pid;
     EXPECT_EQ(noErr, GetProcessPID(&shim_psn, &shim_pid));
     GetFirstAppWindow()->GetBaseWindow()->Close();
-    ASSERT_TRUE(
-        base::WaitForSingleProcess(shim_pid, TestTimeouts::action_timeout()));
+    base::Process shim_process(shim_pid);
+    int exit_code;
+    ASSERT_TRUE(shim_process.WaitForExitWithTimeout(
+                    TestTimeouts::action_timeout(), &exit_code));
 
     EXPECT_FALSE(GetFirstAppWindow());
     EXPECT_FALSE(HasAppShimHost(profile(), app->id()));

chrome/browser/process_singleton_browsertest.cc

@@ -101,8 +101,8 @@ class ChromeStarter : public base::RefCountedThreadSafe<ChromeStarter> {
     // We can wait on the handle here, we should get stuck on one and only
     // one process. The test below will take care of killing that process
     // to unstuck us once it confirms there is only one.
-    process_terminated_ = base::WaitForSingleProcess(process_.Handle(),
-                                                     timeout_);
+    int exit_code;
+    process_terminated_ = process_.WaitForExitWithTimeout(timeout_, &exit_code);
     // Let the test know we are done.
     done_event_.Signal();
   }
@@ -293,8 +293,8 @@ IN_PROC_BROWSER_TEST_F(ProcessSingletonTest, MAYBE_StartupRaceCondition) {
           starters_done_events, pending_starters.size());
       size_t starter_index = pending_starters[done_index];
       // If the starter is done but has not marked itself as terminated,
-      // it is because it timed out of its WaitForSingleProcess(). Only the
-      // last one standing should be left waiting... So we failed...
+      // it is because it timed out of its WaitForExitCodeWithTimeout(). Only
+      // the last one standing should be left waiting... So we failed...
       EXPECT_TRUE(chrome_starters_[starter_index]->process_terminated_ ||
                   failed) << "There is more than one main process.";
       if (!chrome_starters_[starter_index]->process_terminated_) {

chrome/browser/service_process/service_process_control_browsertest.cc

@@ -80,9 +80,10 @@ class ServiceProcessControlBrowserTest
     ForceServiceProcessShutdown("", 0);
 #endif  // OS_MACOSX
     if (service_process_.IsValid()) {
-      EXPECT_TRUE(base::WaitForSingleProcess(
-          service_process_.Handle(),
-          TestTimeouts::action_max_timeout()));
+      int exit_code;
+      EXPECT_TRUE(service_process_.WaitForExitWithTimeout(
+          TestTimeouts::action_max_timeout(), &exit_code));
+      EXPECT_EQ(0, exit_code);
       service_process_.Close();
     }
   }

chrome/utility/importer/firefox_importer_unittest_utils_mac.cc

@@ -151,9 +151,9 @@ FFUnitTestDecryptorProxy::~FFUnitTestDecryptorProxy() {
   channel_->Close();
 
   if (child_process_.IsValid()) {
-    base::WaitForSingleProcess(child_process_.Handle(),
-                               base::TimeDelta::FromSeconds(5));
-    child_process_.Close();
+    int exit_code;
+    child_process_.WaitForExitWithTimeout(base::TimeDelta::FromSeconds(5),
+                                          &exit_code);
   }
 }
 

ipc/ipc_test_base.cc

@@ -142,8 +142,9 @@ bool IPCTestBase::StartClient() {
 bool IPCTestBase::WaitForClientShutdown() {
   DCHECK(client_process_.IsValid());
 
-  bool rv = base::WaitForSingleProcess(client_process_.Handle(),
-                                       base::TimeDelta::FromSeconds(5));
+  int exit_code;
+  bool rv = client_process_.WaitForExitWithTimeout(
+      base::TimeDelta::FromSeconds(5), &exit_code);
   client_process_.Close();
   return rv;
 }

net/test/spawned_test_server/local_test_server.cc

@@ -125,16 +125,15 @@ bool LocalTestServer::Stop() {
     return true;
 
   // First check if the process has already terminated.
-  bool ret = base::WaitForSingleProcess(process_.Handle(), base::TimeDelta());
-  if (!ret) {
+  int exit_code;
+  bool ret = process_.WaitForExitWithTimeout(base::TimeDelta(), &exit_code);
+  if (!ret)
     ret = base::KillProcess(process_.Handle(), 1, true);
-  }
 
-  if (ret) {
+  if (ret)
     process_.Close();
-  } else {
+  else
     VLOG(1) << "Kill failed?";
-  }
 
   return ret;
 }