Move OpenProcessHandle to Process::Open.

This removes another source of raw process handles.

BUG=417532

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

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

base/BUILD.gn

@@ -342,7 +342,6 @@ component("base") {
     "memory/ref_counted_delete_on_message_loop.h",
     "memory/ref_counted_memory.cc",
     "memory/ref_counted_memory.h",
-    "memory/scoped_open_process.h",
     "memory/scoped_policy.h",
     "memory/scoped_ptr.h",
     "memory/scoped_vector.h",

base/base.gypi

@@ -348,7 +348,6 @@
           'memory/ref_counted_delete_on_message_loop.h',
           'memory/ref_counted_memory.cc',
           'memory/ref_counted_memory.h',
-          'memory/scoped_open_process.h',
           'memory/scoped_policy.h',
           'memory/scoped_ptr.h',
           'memory/scoped_vector.h',

base/memory/scoped_open_process.h

-// Copyright (c) 2011 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.
-
-#ifndef BASE_MEMORY_SCOPED_OPEN_PROCESS_H_
-#define BASE_MEMORY_SCOPED_OPEN_PROCESS_H_
-
-#include "base/process/process_handle.h"
-
-namespace base {
-
-// A class that opens a process from its process id and closes it when the
-// instance goes out of scope.
-class ScopedOpenProcess {
- public:
-  ScopedOpenProcess() : handle_(kNullProcessHandle) {
-  }
-
-  // Automatically close the process.
-  ~ScopedOpenProcess() {
-    Close();
-  }
-
-  // Open a new process by pid. Closes any previously opened process (even if
-  // opening the new one fails).
-  bool Open(ProcessId pid) {
-    Close();
-    return OpenProcessHandle(pid, &handle_);
-  }
-
-  // Close the previously opened process.
-  void Close() {
-    if (handle_ == kNullProcessHandle)
-      return;
-
-    CloseProcessHandle(handle_);
-    handle_ = kNullProcessHandle;
-  }
-
-  ProcessHandle handle() const { return handle_; }
-
- private:
-  ProcessHandle handle_;
-  DISALLOW_COPY_AND_ASSIGN(ScopedOpenProcess);
-};
-}  // namespace base
-
-#endif  // BASE_MEMORY_SCOPED_OPEN_PROCESS_H_

base/process/process.h

@@ -49,6 +49,9 @@ class BASE_EXPORT Process {
   // Returns an object for the current process.
   static Process Current();
 
+  // Returns a Process for the given |pid|.
+  static Process Open(ProcessId pid);
+
   // Returns a Process for the given |pid|. On Windows the handle is opened
   // with more access rights and must only be used by trusted code (can read the
   // address space and duplicate handles).

base/process/process_handle.h

@@ -40,9 +40,7 @@ BASE_EXPORT ProcessId GetCurrentProcId();
 // Returns the ProcessHandle of the current process.
 BASE_EXPORT ProcessHandle GetCurrentProcessHandle();
 
-// Converts a PID to a process handle. This handle must be closed by
-// CloseProcessHandle when you are done with it. Returns true on success.
-BASE_EXPORT bool OpenProcessHandle(ProcessId pid, ProcessHandle* handle);
+
 
 // Closes the process handle opened by OpenProcessHandle.
 BASE_EXPORT void CloseProcessHandle(ProcessHandle process);

base/process/process_handle_posix.cc

@@ -16,13 +16,6 @@ ProcessHandle GetCurrentProcessHandle() {
   return GetCurrentProcId();
 }
 
-bool OpenProcessHandle(ProcessId pid, ProcessHandle* handle) {
-  // On Posix platforms, process handles are the same as PIDs, so we
-  // don't need to do anything.
-  *handle = pid;
-  return true;
-}
-
 void CloseProcessHandle(ProcessHandle process) {
   // See OpenProcessHandle, nothing to do.
   return;

base/process/process_handle_win.cc

@@ -20,22 +20,6 @@ ProcessHandle GetCurrentProcessHandle() {
   return ::GetCurrentProcess();
 }
 
-bool OpenProcessHandle(ProcessId pid, ProcessHandle* handle) {
-  // We try to limit privileges granted to the handle. If you need this
-  // for test code, consider using OpenPrivilegedProcessHandle instead of
-  // adding more privileges here.
-  ProcessHandle result = OpenProcess(PROCESS_TERMINATE |
-                                     PROCESS_QUERY_INFORMATION |
-                                     SYNCHRONIZE,
-                                     FALSE, pid);
-
-  if (result == NULL)
-    return false;
-
-  *handle = result;
-  return true;
-}
-
 void CloseProcessHandle(ProcessHandle process) {
   CloseHandle(process);
 }

base/process/process_posix.cc

@@ -38,15 +38,20 @@ Process Process::Current() {
 }
 
 // static
-Process Process::OpenWithExtraPriviles(ProcessId pid) {
+Process Process::Open(ProcessId pid) {
   if (pid == GetCurrentProcId())
     return Current();
 
-  // On POSIX process handles are the same as PIDs, and there are no privileges
-  // to set.
+  // On POSIX process handles are the same as PIDs.
   return Process(pid);
 }
 
+// static
+Process Process::OpenWithExtraPriviles(ProcessId pid) {
+  // On POSIX there are no privileges to set.
+  return Open(pid);
+}
+
 // static
 Process Process::DeprecatedGetProcessFromHandle(ProcessHandle handle) {
   DCHECK_NE(handle, GetCurrentProcessHandle());

base/process/process_win.cc

@@ -46,11 +46,15 @@ Process Process::Current() {
   return process.Pass();
 }
 
+// static
+Process Process::Open(ProcessId pid) {
+  return Process(::OpenProcess(kBasicProcessAccess, FALSE, pid));
+}
+
 // static
 Process Process::OpenWithExtraPriviles(ProcessId pid) {
   DWORD access = kBasicProcessAccess | PROCESS_DUP_HANDLE | PROCESS_VM_READ;
-  ProcessHandle handle = ::OpenProcess(access, FALSE, pid);
-  return Process(handle);
+  return Process(::OpenProcess(access, FALSE, pid));
 }
 
 // static

chrome/app/chrome_watcher_command_line_unittest_win.cc

@@ -8,24 +8,22 @@
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
-#include "base/process/process_handle.h"
+#include "base/process/process.h"
 #include "base/win/scoped_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 TEST(ChromeWatcherCommandLineTest, BasicTest) {
-  // Ownership of these handles is passed to the ScopedHandles below via
-  // InterpretChromeWatcherCommandLine().
-  base::ProcessHandle current = nullptr;
-  ASSERT_TRUE(base::OpenProcessHandle(base::GetCurrentProcId(), &current));
+  base::Process current = base::Process::Open(base::GetCurrentProcId());
+  ASSERT_TRUE(current.IsValid());
   HANDLE event = ::CreateEvent(nullptr, FALSE, FALSE, nullptr);
 
   base::CommandLine cmd_line = GenerateChromeWatcherCommandLine(
-      base::FilePath(L"example.exe"), current, event);
+      base::FilePath(L"example.exe"), current.Handle(), event);
 
   base::win::ScopedHandle current_result;
   base::win::ScopedHandle event_result;
   ASSERT_TRUE(InterpretChromeWatcherCommandLine(cmd_line, &current_result,
                                                 &event_result));
-  ASSERT_EQ(current, current_result.Get());
+  ASSERT_EQ(current.Handle(), current_result.Get());
   ASSERT_EQ(event, event_result.Get());
 }

chrome/browser/chromeos/policy/device_status_collector.cc

@@ -17,7 +17,7 @@
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
-#include "base/process/process_handle.h"
+#include "base/process/process.h"
 #include "base/process/process_iterator.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/string_number_conversions.h"
@@ -437,16 +437,15 @@ std::vector<double> DeviceStatusCollector::GetPerProcessCPUUsage() {
   const int num_processors = base::SysInfo::NumberOfProcessors();
   while (const base::ProcessEntry* process_entry =
          process_iter.NextProcessEntry()) {
-    base::ProcessHandle process;
-    if (!base::OpenProcessHandle(process_entry->pid(), &process)) {
+    base::Process process = base::Process::Open(process_entry->pid());
+    if (!process.IsValid()) {
       LOG(ERROR) << "Could not create process handle for process "
                   << process_entry->pid();
       continue;
     }
     scoped_ptr<base::ProcessMetrics> metrics(
-        base::ProcessMetrics::CreateProcessMetrics(process));
+        base::ProcessMetrics::CreateProcessMetrics(process.Handle()));
     const double usage = metrics->GetPlatformIndependentCPUUsage();
-    base::CloseProcessHandle(process);
     DCHECK_LE(0, usage);
     if (usage > 0) {
       // Convert CPU usage from "percentage of a single core" to "percentage of

chrome/browser/service_process/service_process_control_browsertest.cc

@@ -95,12 +95,10 @@ class ServiceProcessControlBrowserTest
     service_process_ =
         base::Process::OpenWithAccess(service_pid,
                                       SYNCHRONIZE | PROCESS_QUERY_INFORMATION);
-    EXPECT_TRUE(service_process_.IsValid());
 #else
-    base::ProcessHandle service_process_handle;
-    EXPECT_TRUE(base::OpenProcessHandle(service_pid, &service_process_handle));
-    service_process_ = base::Process(service_process_handle);
+    service_process_ = base::Process::Open(service_pid);
 #endif
+    EXPECT_TRUE(service_process_.IsValid());
     // Quit the current message. Post a QuitTask instead of just calling Quit()
     // because this can get invoked in the context of a Launch() call and we
     // may not be in Run() yet.

chrome/test/base/chrome_process_util.cc

@@ -10,6 +10,7 @@
 
 #include "base/command_line.h"
 #include "base/process/kill.h"
+#include "base/process/process.h"
 #include "base/process/process_iterator.h"
 #include "base/time/time.h"
 #include "chrome/common/chrome_constants.h"
@@ -49,15 +50,12 @@ std::vector<base::FilePath::StringType> GetRunningHelperExecutableNames() {
 void TerminateAllChromeProcesses(const ChromeProcessList& process_pids) {
   ChromeProcessList::const_iterator it;
   for (it = process_pids.begin(); it != process_pids.end(); ++it) {
-    base::ProcessHandle handle;
-    if (!base::OpenProcessHandle(*it, &handle)) {
+    base::Process process = base::Process::Open(*it);
+    if (process.IsValid()) {
       // Ignore processes for which we can't open the handle. We don't
       // guarantee that all processes will terminate, only try to do so.
-      continue;
+      base::KillProcess(process.Handle(), content::RESULT_CODE_KILLED, true);
     }
-
-    base::KillProcess(handle, content::RESULT_CODE_KILLED, true);
-    base::CloseProcessHandle(handle);
   }
 }
 

chrome/test/perf/perf_test.cc

@@ -7,6 +7,7 @@
 #include <stdio.h>
 
 #include "base/logging.h"
+#include "base/process/process.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/test/base/chrome_process_util.h"
@@ -52,17 +53,17 @@ std::string IOPerfInfoToString(const std::string& test_name,
   std::string output;
   ChromeProcessList::const_iterator it;
   for (it = chrome_processes.begin(); it != chrome_processes.end(); ++it) {
-    base::ProcessHandle process_handle;
-    if (!base::OpenProcessHandle(*it, &process_handle)) {
+    base::Process process = base::Process::Open(*it);
+    if (!process.IsValid()) {
       NOTREACHED();
       return output;
     }
 
     // TODO(sgk):  if/when base::ProcessMetrics returns real stats on mac:
     // scoped_ptr<base::ProcessMetrics> process_metrics(
-    //     base::ProcessMetrics::CreateProcessMetrics(process_handle));
+    //     base::ProcessMetrics::CreateProcessMetrics(process.Handle()));
     scoped_ptr<ChromeTestProcessMetrics> process_metrics(
-        ChromeTestProcessMetrics::CreateProcessMetrics(process_handle));
+        ChromeTestProcessMetrics::CreateProcessMetrics(process.Handle()));
     base::IoCounters io_counters;
     memset(&io_counters, 0, sizeof(io_counters));
 
@@ -109,8 +110,6 @@ std::string IOPerfInfoToString(const std::string& test_name,
         total_byte_r += total_byte;
       }
     }
-
-    base::CloseProcessHandle(process_handle);
   }
 
   std::string t_name(test_name);
@@ -268,17 +267,17 @@ std::string MemoryUsageInfoToString(const std::string& test_name,
   std::string output;
   ChromeProcessList::const_iterator it;
   for (it = chrome_processes.begin(); it != chrome_processes.end(); ++it) {
-    base::ProcessHandle process_handle;
-    if (!base::OpenProcessHandle(*it, &process_handle)) {
+    base::Process process = base::Process::Open(*it);
+    if (!process.IsValid()) {
       NOTREACHED();
       return output;
     }
 
     // TODO(sgk):  if/when base::ProcessMetrics returns real stats on mac:
     // scoped_ptr<base::ProcessMetrics> process_metrics(
-    //     base::ProcessMetrics::CreateProcessMetrics(process_handle));
+    //     base::ProcessMetrics::CreateProcessMetrics(process.Handle()));
     scoped_ptr<ChromeTestProcessMetrics> process_metrics(
-        ChromeTestProcessMetrics::CreateProcessMetrics(process_handle));
+        ChromeTestProcessMetrics::CreateProcessMetrics(process.Handle()));
 
     size_t current_virtual_size = process_metrics->GetPagefileUsage();
     size_t current_working_set_size = process_metrics->GetWorkingSetSize();
@@ -310,8 +309,6 @@ std::string MemoryUsageInfoToString(const std::string& test_name,
       renderer_total_peak_working_set_size += peak_working_set_size;
     }
 #endif
-
-    base::CloseProcessHandle(process_handle);
   }
 
   std::string trace_name(test_name);

components/browser_watcher/watcher_metrics_provider_win.cc

@@ -8,7 +8,7 @@
 #include <vector>
 
 #include "base/metrics/sparse_histogram.h"
-#include "base/process/process_handle.h"
+#include "base/process/process.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
@@ -51,10 +51,9 @@ bool IsDeadProcess(base::StringPiece16 key_or_value_name) {
   // This is more expensive than the above check, but should also be very rare,
   // as this only happens more than once for a given PID if a user is running
   // multiple Chrome instances concurrently.
-  base::ProcessHandle process = base::kNullProcessHandle;
-  if (base::OpenProcessHandle(static_cast<base::ProcessId>(pid), &process)) {
-    base::CloseProcessHandle(process);
-
+  base::Process process =
+      base::Process::Open(static_cast<base::ProcessId>(pid));
+  if (process.IsValid()) {
     // The fact that it was possible to open the process says it's live.
     return false;
   }

sandbox/linux/services/unix_domain_socket_unittest.cc

@@ -17,7 +17,7 @@
 #include "base/memory/scoped_vector.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/posix/unix_domain_socket_linux.h"
-#include "base/process/process_handle.h"
+#include "base/process/process.h"
 #include "sandbox/linux/services/syscall_wrappers.h"
 #include "sandbox/linux/tests/unit_tests.h"
 
@@ -57,10 +57,9 @@ base::ProcessId GetParentProcessId(base::ProcessId pid) {
   // base::GetParentProcessId() is defined as taking a ProcessHandle instead of
   // a ProcessId, even though it's a POSIX-only function and IDs and Handles
   // are both simply pid_t on POSIX... :/
-  base::ProcessHandle handle;
-  CHECK(base::OpenProcessHandle(pid, &handle));
-  base::ProcessId ret = base::GetParentProcessId(pid);
-  base::CloseProcessHandle(handle);
+  base::Process process = base::Process::Open(pid);
+  CHECK(process.IsValid());
+  base::ProcessId ret = base::GetParentProcessId(process.Handle());
   return ret;
 }