Remove uses of KillProcess()

chrome/browser/process_singleton_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This test validates that the ProcessSingleton class properly makes sure
 // that there is only one main browser process.
 //
 // It is currently compiled and run on Windows and Posix(non-Mac) platforms.
 // Mac uses system services and ProcessSingletonMac is a noop.  (Maybe it still
 // makes sense to test that the system services are giving the behavior we
 // want?)
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
-#include "base/process/kill.h"
 #include "base/process/launch.h"
 #include "base/process/process.h"
 #include "base/process/process_iterator.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/thread.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ skipping 123 matching lines @@
   }
 
   // This method is used to make sure we kill the main browser process after
   // all of its child processes have successfully attached to it. This was added
   // when we realized that if we just kill the parent process right away, we
   // sometimes end up with dangling child processes. If we Sleep for a certain
   // amount of time, we are OK... So we introduced this method to avoid a
   // flaky wait. Instead, we kill all descendants of the main process after we
   // killed it, relying on the fact that we can still get the parent id of a
   // child process, even when the parent dies.
-  void KillProcessTree(base::ProcessHandle process_handle) {
+  void KillProcessTree(const base::Process& process) {
     class ProcessTreeFilter : public base::ProcessFilter {
      public:
       explicit ProcessTreeFilter(base::ProcessId parent_pid) {
         ancestor_pids_.insert(parent_pid);
       }
       bool Includes(const base::ProcessEntry& entry) const override {
         if (ancestor_pids_.find(entry.parent_pid()) != ancestor_pids_.end()) {
           ancestor_pids_.insert(entry.pid());
           return true;
         } else {
           return false;
         }
       }
      private:
       mutable std::set<base::ProcessId> ancestor_pids_;
-    } process_tree_filter(base::GetProcId(process_handle));
+    } process_tree_filter(process.Pid());
 
     // Start by explicitly killing the main process we know about...
     static const int kExitCode = 42;
-    EXPECT_TRUE(base::KillProcess(process_handle, kExitCode, true /* wait */));
+    EXPECT_TRUE(process.Terminate(kExitCode, true /* wait */));

[cpu_(ooo_6.6-7.5), 2015/03/17 20:36:31]

woa, Terminate is a const method?

[rvargas (doing something else), 2015/03/17 22:40:53]

Yes. I added const when adding the second argument (last cl?) to enable this use
case (a couple of cases in this CL).

The alternative was to pass around pointers to Process, and that is less clear,
IMO.

On the other hand, given that a caller can always do Handle() (of a const
object) and then use that handle to kill the process (for example), it seems
fine. In other words, don't worry about the underlying effects of a method and
instead treat it as "modify my members or not" when thinking about const :)

 
     // Then loop until we can't find any of its descendant.
     // But don't try more than kNbTries times...
     static const int kNbTries = 10;
     int num_tries = 0;
     base::FilePath program;
     ASSERT_TRUE(PathService::Get(base::FILE_EXE, &program));
     base::FilePath::StringType exe_name = program.BaseName().value();
     while (base::GetProcessCount(exe_name, &process_tree_filter) > 0 &&
            num_tries++ < kNbTries) {
@@ skipping 102 matching lines @@
       // 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_) {
         // This will stop the "for kNbAttempts" loop.
         failed = true;
         // But we let the last loop turn finish so that we can properly
         // kill all remaining processes. Starting with this one...
         if (chrome_starters_[starter_index]->process_.IsValid()) {
-          KillProcessTree(chrome_starters_[starter_index]->process_.Handle());
+          KillProcessTree(chrome_starters_[starter_index]->process_);
         }
       }
       pending_starters.erase(pending_starters.begin() + done_index);
     }
 
     // "There can be only one!" :-)
     ASSERT_EQ(static_cast<size_t>(1), pending_starters.size());
     size_t last_index = pending_starters.front();
     pending_starters.clear();
     if (chrome_starters_[last_index]->process_.IsValid()) {
-      KillProcessTree(chrome_starters_[last_index]->process_.Handle());
+      KillProcessTree(chrome_starters_[last_index]->process_);
       chrome_starters_[last_index]->done_event_.Wait();
     }
   }
 }