MultiProcessTest: Update SpawnChild* to return a Process.

We should not be dealing with raw handles.

BUG=417532

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

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

7 files changed, 60 insertions, 66 deletions

diff --git a/base/debug/stack_trace_unittest.cc b/base/debug/stack_trace_unittest.cc
index eb0bd9a..b07fcdb 100644
--- a/base/debug/stack_trace_unittest.cc
+++ b/base/debug/stack_trace_unittest.cc
@@ -146,9 +146,10 @@ MULTIPROCESS_TEST_MAIN(MismatchedMallocChildProcess) {
 // and e.g. mismatched new[]/delete would cause a hang because
 // of re-entering malloc.
 TEST_F(StackTraceTest, AsyncSignalUnsafeSignalHandlerHang) {
-  ProcessHandle child = SpawnChild("MismatchedMallocChildProcess");
-  ASSERT_NE(kNullProcessHandle, child);
-  ASSERT_TRUE(WaitForSingleProcess(child, TestTimeouts::action_timeout()));
+  Process child = SpawnChild("MismatchedMallocChildProcess");
+  ASSERT_TRUE(child.IsValid());
+  ASSERT_TRUE(WaitForSingleProcess(child.Handle(),
+                                   TestTimeouts::action_timeout()));
 }
 #endif  // !defined(OS_IOS)
 
diff --git a/base/memory/shared_memory_unittest.cc b/base/memory/shared_memory_unittest.cc
index 265179a..6e5a4d8 100644
--- a/base/memory/shared_memory_unittest.cc
+++ b/base/memory/shared_memory_unittest.cc
@@ -699,15 +699,15 @@ const char* const SharedMemoryProcessTest::s_test_name_ = "MPMem";
 TEST_F(SharedMemoryProcessTest, Tasks) {
   SharedMemoryProcessTest::CleanUp();
 
-  ProcessHandle handles[kNumTasks];
+  Process processes[kNumTasks];
   for (int index = 0; index < kNumTasks; ++index) {
-    handles[index] = SpawnChild("SharedMemoryTestMain");
-    ASSERT_TRUE(handles[index]);
+    processes[index] = SpawnChild("SharedMemoryTestMain");
+    ASSERT_TRUE(processes[index].IsValid());
   }
 
   int exit_code = 0;
   for (int index = 0; index < kNumTasks; ++index) {
-    EXPECT_TRUE(WaitForExitCode(handles[index], &exit_code));
+    EXPECT_TRUE(processes[index].WaitForExit(&exit_code));
     EXPECT_EQ(0, exit_code);
   }
 
diff --git a/base/metrics/stats_table_unittest.cc b/base/metrics/stats_table_unittest.cc
index 45b0a43..38a21cc 100644
--- a/base/metrics/stats_table_unittest.cc
+++ b/base/metrics/stats_table_unittest.cc
@@ -200,19 +200,19 @@ TEST_F(StatsTableTest, DISABLED_MultipleProcesses) {
   // Spin up a set of processes to go bang on the various counters.
   // After we join the processes, we'll make sure the counters
   // contain the values we expected.
-  ProcessHandle procs[kMaxProcs];
+  Process procs[kMaxProcs];
 
   // Spawn the processes.
   for (int16 index = 0; index < kMaxProcs; index++) {
     procs[index] = SpawnChild("StatsTableMultipleProcessMain");
-    EXPECT_NE(kNullProcessHandle, procs[index]);
+    EXPECT_TRUE(procs[index].IsValid());
   }
 
   // Wait for the processes to finish.
   for (int index = 0; index < kMaxProcs; index++) {
-    EXPECT_TRUE(WaitForSingleProcess(
-        procs[index], base::TimeDelta::FromMinutes(1)));
-    CloseProcessHandle(procs[index]);
+    EXPECT_TRUE(WaitForSingleProcess(procs[index].Handle(),
+                                     base::TimeDelta::FromMinutes(1)));
+    procs[index].Close();
   }
 
   StatsCounter zero_counter(kCounterZero);
diff --git a/base/process/process_util_unittest.cc b/base/process/process_util_unittest.cc
index af88fe1..80e103d 100644
--- a/base/process/process_util_unittest.cc
+++ b/base/process/process_util_unittest.cc
@@ -142,11 +142,10 @@ MULTIPROCESS_TEST_MAIN(SimpleChildProcess) {
 
 // TODO(viettrungluu): This should be in a "MultiProcessTestTest".
 TEST_F(ProcessUtilTest, SpawnChild) {
-  base::ProcessHandle handle = SpawnChild("SimpleChildProcess");
-  ASSERT_NE(base::kNullProcessHandle, handle);
-  EXPECT_TRUE(base::WaitForSingleProcess(
-                  handle, TestTimeouts::action_max_timeout()));
-  base::CloseProcessHandle(handle);
+  base::Process process = SpawnChild("SimpleChildProcess");
+  ASSERT_TRUE(process.IsValid());
+  EXPECT_TRUE(base::WaitForSingleProcess(process.Handle(),
+                                         TestTimeouts::action_max_timeout()));
 }
 
 MULTIPROCESS_TEST_MAIN(SlowChildProcess) {
@@ -158,12 +157,11 @@ TEST_F(ProcessUtilTest, KillSlowChild) {
   const std::string signal_file =
       ProcessUtilTest::GetSignalFilePath(kSignalFileSlow);
   remove(signal_file.c_str());
-  base::ProcessHandle handle = SpawnChild("SlowChildProcess");
-  ASSERT_NE(base::kNullProcessHandle, handle);
+  base::Process process = SpawnChild("SlowChildProcess");
+  ASSERT_TRUE(process.IsValid());
   SignalChildren(signal_file.c_str());
-  EXPECT_TRUE(base::WaitForSingleProcess(
-                  handle, TestTimeouts::action_max_timeout()));
-  base::CloseProcessHandle(handle);
+  EXPECT_TRUE(base::WaitForSingleProcess(process.Handle(),
+                                         TestTimeouts::action_max_timeout()));
   remove(signal_file.c_str());
 }
 
@@ -172,21 +170,20 @@ TEST_F(ProcessUtilTest, DISABLED_GetTerminationStatusExit) {
   const std::string signal_file =
       ProcessUtilTest::GetSignalFilePath(kSignalFileSlow);
   remove(signal_file.c_str());
-  base::ProcessHandle handle = SpawnChild("SlowChildProcess");
-  ASSERT_NE(base::kNullProcessHandle, handle);
+  base::Process process = SpawnChild("SlowChildProcess");
+  ASSERT_TRUE(process.IsValid());
 
   int exit_code = 42;
   EXPECT_EQ(base::TERMINATION_STATUS_STILL_RUNNING,
-            base::GetTerminationStatus(handle, &exit_code));
+            base::GetTerminationStatus(process.Handle(), &exit_code));
   EXPECT_EQ(kExpectedStillRunningExitCode, exit_code);
 
   SignalChildren(signal_file.c_str());
   exit_code = 42;
   base::TerminationStatus status =
-      WaitForChildTermination(handle, &exit_code);
+      WaitForChildTermination(process.Handle(), &exit_code);
   EXPECT_EQ(base::TERMINATION_STATUS_NORMAL_TERMINATION, status);
   EXPECT_EQ(0, exit_code);
-  base::CloseProcessHandle(handle);
   remove(signal_file.c_str());
 }
 
@@ -195,12 +192,11 @@ TEST_F(ProcessUtilTest, DISABLED_GetTerminationStatusExit) {
 TEST_F(ProcessUtilTest, GetProcId) {
   base::ProcessId id1 = base::GetProcId(GetCurrentProcess());
   EXPECT_NE(0ul, id1);
-  base::ProcessHandle handle = SpawnChild("SimpleChildProcess");
-  ASSERT_NE(base::kNullProcessHandle, handle);
-  base::ProcessId id2 = base::GetProcId(handle);
+  base::Process process = SpawnChild("SimpleChildProcess");
+  ASSERT_TRUE(process.IsValid());
+  base::ProcessId id2 = process.pid();
   EXPECT_NE(0ul, id2);
   EXPECT_NE(id1, id2);
-  base::CloseProcessHandle(handle);
 }
 #endif
 
@@ -239,18 +235,18 @@ TEST_F(ProcessUtilTest, MAYBE_GetTerminationStatusCrash) {
   const std::string signal_file =
     ProcessUtilTest::GetSignalFilePath(kSignalFileCrash);
   remove(signal_file.c_str());
-  base::ProcessHandle handle = SpawnChild("CrashingChildProcess");
-  ASSERT_NE(base::kNullProcessHandle, handle);
+  base::Process process = SpawnChild("CrashingChildProcess");
+  ASSERT_TRUE(process.IsValid());
 
   int exit_code = 42;
   EXPECT_EQ(base::TERMINATION_STATUS_STILL_RUNNING,
-            base::GetTerminationStatus(handle, &exit_code));
+            base::GetTerminationStatus(process.Handle(), &exit_code));
   EXPECT_EQ(kExpectedStillRunningExitCode, exit_code);
 
   SignalChildren(signal_file.c_str());
   exit_code = 42;
   base::TerminationStatus status =
-      WaitForChildTermination(handle, &exit_code);
+      WaitForChildTermination(process.Handle(), &exit_code);
   EXPECT_EQ(base::TERMINATION_STATUS_PROCESS_CRASHED, status);
 
 #if defined(OS_WIN)
@@ -261,7 +257,6 @@ TEST_F(ProcessUtilTest, MAYBE_GetTerminationStatusCrash) {
   int signal = WTERMSIG(exit_code);
   EXPECT_EQ(SIGSEGV, signal);
 #endif
-  base::CloseProcessHandle(handle);
 
   // Reset signal handlers back to "normal".
   base::debug::EnableInProcessStackDumping();
@@ -286,18 +281,18 @@ TEST_F(ProcessUtilTest, GetTerminationStatusKill) {
   const std::string signal_file =
     ProcessUtilTest::GetSignalFilePath(kSignalFileKill);
   remove(signal_file.c_str());
-  base::ProcessHandle handle = SpawnChild("KilledChildProcess");
-  ASSERT_NE(base::kNullProcessHandle, handle);
+  base::Process process = SpawnChild("KilledChildProcess");
+  ASSERT_TRUE(process.IsValid());
 
   int exit_code = 42;
   EXPECT_EQ(base::TERMINATION_STATUS_STILL_RUNNING,
-            base::GetTerminationStatus(handle, &exit_code));
+            base::GetTerminationStatus(process.Handle(), &exit_code));
   EXPECT_EQ(kExpectedStillRunningExitCode, exit_code);
 
   SignalChildren(signal_file.c_str());
   exit_code = 42;
   base::TerminationStatus status =
-      WaitForChildTermination(handle, &exit_code);
+      WaitForChildTermination(process.Handle(), &exit_code);
   EXPECT_EQ(base::TERMINATION_STATUS_PROCESS_WAS_KILLED, status);
 #if defined(OS_WIN)
   EXPECT_EQ(kExpectedKilledExitCode, exit_code);
@@ -307,7 +302,6 @@ TEST_F(ProcessUtilTest, GetTerminationStatusKill) {
   int signal = WTERMSIG(exit_code);
   EXPECT_EQ(SIGKILL, signal);
 #endif
-  base::CloseProcessHandle(handle);
   remove(signal_file.c_str());
 }
 
@@ -535,9 +529,9 @@ int ProcessUtilTest::CountOpenFDsInChild() {
   fd_mapping_vec.push_back(std::pair<int, int>(fds[1], kChildPipe));
   base::LaunchOptions options;
   options.fds_to_remap = &fd_mapping_vec;
-  base::ProcessHandle handle =
+  base::Process process =
       SpawnChildWithOptions("ProcessUtilsLeakFDChildProcess", options);
-  CHECK(handle);
+  CHECK(process.IsValid());
   int ret = IGNORE_EINTR(close(fds[1]));
   DPCHECK(ret == 0);
 
@@ -549,11 +543,12 @@ int ProcessUtilTest::CountOpenFDsInChild() {
 
 #if defined(THREAD_SANITIZER)
   // Compiler-based ThreadSanitizer makes this test slow.
-  CHECK(base::WaitForSingleProcess(handle, base::TimeDelta::FromSeconds(3)));
+  CHECK(base::WaitForSingleProcess(process.Handle(),
+                                   base::TimeDelta::FromSeconds(3)));
 #else
-  CHECK(base::WaitForSingleProcess(handle, base::TimeDelta::FromSeconds(1)));
+  CHECK(base::WaitForSingleProcess(process.Handle(),
+                                   base::TimeDelta::FromSeconds(1)));
 #endif
-  base::CloseProcessHandle(handle);
   ret = IGNORE_EINTR(close(fds[0]));
   DPCHECK(ret == 0);
 
@@ -888,7 +883,7 @@ bool IsProcessDead(base::ProcessHandle child) {
 }
 
 TEST_F(ProcessUtilTest, DelayedTermination) {
-  base::Process child_process(SpawnChild("process_util_test_never_die"));
+  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(),
@@ -906,7 +901,7 @@ MULTIPROCESS_TEST_MAIN(process_util_test_never_die) {
 }
 
 TEST_F(ProcessUtilTest, ImmediateTermination) {
-  base::Process child_process(SpawnChild("process_util_test_die_immediately"));
+  base::Process child_process = SpawnChild("process_util_test_die_immediately");
   ASSERT_TRUE(child_process.IsValid());
   // Give it time to die.
   sleep(2);
diff --git a/base/test/multiprocess_test.cc b/base/test/multiprocess_test.cc
index 306c109..b95ea98 100644
--- a/base/test/multiprocess_test.cc
+++ b/base/test/multiprocess_test.cc
@@ -10,7 +10,7 @@
 namespace base {
 
 #if !defined(OS_ANDROID)
-ProcessHandle SpawnMultiProcessTestChild(
+Process SpawnMultiProcessTestChild(
     const std::string& procname,
     const CommandLine& base_command_line,
     const LaunchOptions& options) {
@@ -21,9 +21,7 @@ ProcessHandle SpawnMultiProcessTestChild(
   if (!command_line.HasSwitch(switches::kTestChildProcess))
     command_line.AppendSwitchASCII(switches::kTestChildProcess, procname);
 
-  ProcessHandle handle = kNullProcessHandle;
-  LaunchProcess(command_line, options, &handle);
-  return handle;
+  return LaunchProcess(command_line, options);
 }
 #endif  // !defined(OS_ANDROID)
 
@@ -36,7 +34,7 @@ CommandLine GetMultiProcessTestChildBaseCommandLine() {
 MultiProcessTest::MultiProcessTest() {
 }
 
-ProcessHandle MultiProcessTest::SpawnChild(const std::string& procname) {
+Process MultiProcessTest::SpawnChild(const std::string& procname) {
   LaunchOptions options;
 #if defined(OS_WIN)
   options.start_hidden = true;
@@ -44,7 +42,7 @@ ProcessHandle MultiProcessTest::SpawnChild(const std::string& procname) {
   return SpawnChildWithOptions(procname, options);
 }
 
-ProcessHandle MultiProcessTest::SpawnChildWithOptions(
+Process MultiProcessTest::SpawnChildWithOptions(
     const std::string& procname,
     const LaunchOptions& options) {
   return SpawnMultiProcessTestChild(procname, MakeCmdLine(procname), options);
diff --git a/base/test/multiprocess_test.h b/base/test/multiprocess_test.h
index b830f73..e419503 100644
--- a/base/test/multiprocess_test.h
+++ b/base/test/multiprocess_test.h
@@ -9,7 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/process/launch.h"
-#include "base/process/process_handle.h"
+#include "base/process/process.h"
 #include "build/build_config.h"
 #include "testing/platform_test.h"
 
@@ -58,7 +58,7 @@ class CommandLine;
 // |command_line| should be as provided by
 // |GetMultiProcessTestChildBaseCommandLine()| (below), possibly with arguments
 // added. Note: On Windows, you probably want to set |options.start_hidden|.
-ProcessHandle SpawnMultiProcessTestChild(
+Process SpawnMultiProcessTestChild(
     const std::string& procname,
     const CommandLine& command_line,
     const LaunchOptions& options);
@@ -105,14 +105,14 @@ class MultiProcessTest : public PlatformTest {
   //         // do client work here
   //    }
   //
-  // Returns the handle to the child, or NULL on failure
-  ProcessHandle SpawnChild(const std::string& procname);
+  // Returns the child process.
+  Process SpawnChild(const std::string& procname);
 
   // Run a child process using the given launch options.
   //
   // Note: On Windows, you probably want to set |options.start_hidden|.
-  ProcessHandle SpawnChildWithOptions(const std::string& procname,
-                                      const LaunchOptions& options);
+  Process SpawnChildWithOptions(const std::string& procname,
+                                const LaunchOptions& options);
 
   // Set up the command line used to spawn the child process.
   // Override this to add things to the command line (calling this first in the
diff --git a/base/test/multiprocess_test_android.cc b/base/test/multiprocess_test_android.cc
index 8f54b82..dc489d1 100644
--- a/base/test/multiprocess_test_android.cc
+++ b/base/test/multiprocess_test_android.cc
@@ -19,9 +19,9 @@ namespace base {
 // and we don't have an executable to exec*. This implementation does the bare
 // minimum to execute the method specified by procname (in the child process).
 //  - All options except |fds_to_remap| are ignored.
-ProcessHandle SpawnMultiProcessTestChild(const std::string& procname,
-                                         const CommandLine& base_command_line,
-                                         const LaunchOptions& options) {
+Process SpawnMultiProcessTestChild(const std::string& procname,
+                                   const CommandLine& base_command_line,
+                                   const LaunchOptions& options) {
   // TODO(viettrungluu): The FD-remapping done below is wrong in the presence of
   // cycles (e.g., fd1 -> fd2, fd2 -> fd1). crbug.com/326576
   FileHandleMappingVector empty;
@@ -32,11 +32,11 @@ ProcessHandle SpawnMultiProcessTestChild(const std::string& procname,
 
   if (pid < 0) {
     PLOG(ERROR) << "fork";
-    return kNullProcessHandle;
+    return Process();
   }
   if (pid > 0) {
     // Parent process.
-    return pid;
+    return Process(pid);
   }
   // Child process.
   base::hash_set<int> fds_to_keep_open;
@@ -69,7 +69,7 @@ ProcessHandle SpawnMultiProcessTestChild(const std::string& procname,
     command_line->AppendSwitchASCII(switches::kTestChildProcess, procname);
 
   _exit(multi_process_function_list::InvokeChildProcessTest(procname));
-  return 0;
+  return Process();
 }
 
 }  // namespace base