mojo: Remove ScopedProcessHandle in favor of base::Process.

This adds a process_stubs.cc implementation for iOS and NaCl
to have an implementation of process.h.

Bug: 1008512
Change-Id: I44a478a790bfe8c3aabc17298a2ddb99efb2bf5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517924
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#826494}

base/BUILD.gn

@@ -1674,6 +1674,7 @@ component("base") {
     sources += [
       "files/file_path_watcher_stub.cc",
       "process/process_metrics_nacl.cc",
+      "process/process_stubs.cc",
       "sync_socket_nacl.cc",
       "threading/platform_thread_linux.cc",
     ]
@@ -2022,6 +2023,7 @@ component("base") {
       "message_loop/message_pump_mac.mm",
       "power_monitor/power_monitor_device_source_ios.mm",
       "process/memory_stubs.cc",
+      "process/process_stubs.cc",
       "strings/sys_string_conversions_mac.mm",
       "synchronization/waitable_event_mac.cc",
       "system/sys_info_ios.mm",

base/process/process.h

@@ -96,6 +96,11 @@ class BASE_EXPORT Process {
   // Returns a second object that represents this process.
   Process Duplicate() const;
 
+  // Relinquishes ownership of the handle and sets this to kNullProcessHandle.
+  // The result may be a pseudo-handle, depending on the OS and value stored in
+  // this.
+  ProcessHandle Release() WARN_UNUSED_RESULT;
+
   // Get the PID for this process.
   ProcessId Pid() const;
 
@@ -166,7 +171,7 @@ class BASE_EXPORT Process {
   // process though that should be avoided.
   void Exited(int exit_code) const;
 
-#if defined(OS_APPLE)
+#if defined(OS_MAC)
   // The Mac needs a Mach port in order to manipulate a process's priority,
   // and there's no good way to get that from base given the pid. These Mac
   // variants of the IsProcessBackgrounded and SetProcessBackgrounded API take

base/process/process_fuchsia.cc

@@ -168,6 +168,21 @@ Process Process::Duplicate() const {
   return Process(out.release());
 }
 
+ProcessHandle Process::Release() {
+  if (is_current()) {
+    // Caller expects to own the reference, so duplicate the self handle.
+    zx::process handle;
+    zx_status_t result =
+        zx::process::self()->duplicate(ZX_RIGHT_SAME_RIGHTS, &handle);
+    if (result != ZX_OK) {
+      return kNullProcessHandle;
+    }
+    is_current_process_ = false;
+    return handle.release();
+  }
+  return process_.release();
+}
+
 ProcessId Process::Pid() const {
   DCHECK(IsValid());
   return GetProcId(Handle());

base/process/process_posix.cc

@@ -10,6 +10,8 @@
 #include <sys/resource.h>
 #include <sys/wait.h>
 
+#include <utility>
+
 #include "base/clang_profiling_buildflags.h"
 #include "base/debug/activity_tracker.h"
 #include "base/files/scoped_file.h"
@@ -299,6 +301,10 @@ Process Process::Duplicate() const {
   return Process(process_);
 }
 
+ProcessHandle Process::Release() {
+  return std::exchange(process_, kNullProcessHandle);
+}
+
 ProcessId Process::Pid() const {
   DCHECK(IsValid());
   return GetProcId(process_);

base/process/process_stubs.cc

@@ -0,0 +1,101 @@
+// Copyright 2020 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.
+
+#include "base/process/process.h"
+
+#include <limits>
+
+namespace base {
+
+static constexpr ProcessHandle kCurrentProcessHandle =
+    std::numeric_limits<ProcessHandle>::max();
+
+Process::Process(ProcessHandle handle) : process_(handle) {
+  DCHECK(handle == kNullProcessHandle || handle == kCurrentProcessHandle);
+}
+
+Process::Process(Process&& other) : process_(other.process_) {
+  other.Close();
+}
+
+Process::~Process() = default;
+
+Process& Process::operator=(Process&& other) {
+  process_ = other.process_;
+  other.Close();
+  return *this;
+}
+
+// static
+Process Process::Current() {
+  return Process(kCurrentProcessHandle);
+}
+
+// static
+Process Process::Open(ProcessId pid) {
+  return Process(pid);
+}
+
+// static
+Process Process::OpenWithExtraPrivileges(ProcessId pid) {
+  return Process(pid);
+}
+
+bool Process::IsValid() const {
+  return process_ != kNullProcessHandle;
+}
+
+ProcessHandle Process::Handle() const {
+  return process_;
+}
+
+Process Process::Duplicate() const {
+  return Process(process_);
+}
+
+ProcessHandle Process::Release() {
+  ProcessHandle handle = process_;
+  Close();
+  return handle;
+}
+
+ProcessId Process::Pid() const {
+  return process_;
+}
+
+Time Process::CreationTime() const {
+  return Time();
+}
+
+bool Process::is_current() const {
+  return Handle() == kCurrentProcessHandle;
+}
+
+void Process::Close() {
+  process_ = kNullProcessHandle;
+}
+
+bool Process::WaitForExit(int* exit_code) const {
+  return false;
+}
+
+bool Process::WaitForExitWithTimeout(TimeDelta timeout, int* exit_code) const {
+  return false;
+}
+
+void Process::Exited(int exit_code) const {}
+
+bool Process::IsProcessBackgrounded() const {
+  return false;
+}
+
+bool Process::SetProcessBackgrounded(bool value) {
+  return false;
+}
+
+int Process::GetPriority() const {
+  return -1;
+}
+
+}  // namespace base

base/process/process_win.cc

@@ -124,6 +124,12 @@ Process Process::Duplicate() const {
   return Process(out_handle);
 }
 
+ProcessHandle Process::Release() {
+  if (is_current())
+    return ::GetCurrentProcess();
+  return process_.Take();
+}
+
 ProcessId Process::Pid() const {
   DCHECK(IsValid());
   return GetProcId(Handle());

mojo/core/BUILD.gn

@@ -66,7 +66,6 @@ template("core_impl_source_set") {
       "platform_handle_utils.h",
       "platform_shared_memory_mapping.h",
       "request_context.h",
-      "scoped_process_handle.h",
       "shared_buffer_dispatcher.h",
       "user_message_impl.h",
     ]
@@ -94,7 +93,6 @@ template("core_impl_source_set") {
       "platform_handle_utils.cc",
       "platform_shared_memory_mapping.cc",
       "request_context.cc",
-      "scoped_process_handle.cc",
       "shared_buffer_dispatcher.cc",
       "user_message_impl.cc",
       "watch.cc",

mojo/core/broker_host.cc

@@ -21,13 +21,13 @@
 namespace mojo {
 namespace core {
 
-BrokerHost::BrokerHost(base::ProcessHandle client_process,
+BrokerHost::BrokerHost(base::Process client_process,
                        ConnectionParams connection_params,
                        const ProcessErrorCallback& process_error_callback)
     : process_error_callback_(process_error_callback)
 #if defined(OS_WIN)
       ,
-      client_process_(ScopedProcessHandle::CloneFrom(client_process))
+      client_process_(std::move(client_process))
 #endif
 {
   CHECK(connection_params.endpoint().is_valid() ||
@@ -54,7 +54,7 @@ bool BrokerHost::PrepareHandlesForClient(
 #if defined(OS_WIN)
   bool handles_ok = true;
   for (auto& handle : *handles) {
-    if (!handle.TransferToProcess(client_process_.Clone()))
+    if (!handle.TransferToProcess(client_process_.Duplicate()))
       handles_ok = false;
   }
   return handles_ok;

mojo/core/broker_host.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/process/process.h"
 #include "base/process/process_handle.h"
 #include "base/strings/string_piece.h"
 #include "base/task/current_thread.h"
@@ -17,7 +18,6 @@
 #include "mojo/core/connection_params.h"
 #include "mojo/core/embedder/process_error_callback.h"
 #include "mojo/core/platform_handle_in_transit.h"
-#include "mojo/core/scoped_process_handle.h"
 #include "mojo/public/cpp/platform/platform_handle.h"
 
 namespace mojo {
@@ -28,7 +28,7 @@ namespace core {
 class BrokerHost : public Channel::Delegate,
                    public base::CurrentThread::DestructionObserver {
  public:
-  BrokerHost(base::ProcessHandle client_process,
+  BrokerHost(base::Process client_process,
              ConnectionParams connection_params,
              const ProcessErrorCallback& process_error_callback);
 
@@ -59,7 +59,7 @@ class BrokerHost : public Channel::Delegate,
   const ProcessErrorCallback process_error_callback_;
 
 #if defined(OS_WIN)
-  ScopedProcessHandle client_process_;
+  base::Process client_process_;
 #endif
 
   scoped_refptr<Channel> channel_;

mojo/core/channel.h

@@ -11,12 +11,12 @@
 #include "base/macros.h"
 #include "base/memory/aligned_memory.h"
 #include "base/memory/ref_counted.h"
+#include "base/process/process.h"
 #include "base/process/process_handle.h"
 #include "base/single_thread_task_runner.h"
 #include "build/build_config.h"
 #include "mojo/core/connection_params.h"
 #include "mojo/core/platform_handle_in_transit.h"
-#include "mojo/core/scoped_process_handle.h"
 #include "mojo/public/cpp/platform/platform_handle.h"
 
 namespace mojo {
@@ -295,11 +295,11 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel
 
   // Sets the process handle of the remote endpoint to which this Channel is
   // connected. If called at all, must be called only once, and before Start().
-  void set_remote_process(ScopedProcessHandle remote_process) {
-    DCHECK(!remote_process_.is_valid());
+  void set_remote_process(base::Process remote_process) {
+    DCHECK(!remote_process_.IsValid());
     remote_process_ = std::move(remote_process);
   }
-  const ScopedProcessHandle& remote_process() const { return remote_process_; }
+  const base::Process& remote_process() const { return remote_process_; }
 
   // Begin processing I/O events. Delegate methods must only be invoked after
   // this call.
@@ -410,7 +410,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel
   const std::unique_ptr<ReadBuffer> read_buffer_;
 
   // Handle to the process on the other end of this Channel, iff known.
-  ScopedProcessHandle remote_process_;
+  base::Process remote_process_;
 
   DISALLOW_COPY_AND_ASSIGN(Channel);
 };

mojo/core/channel_fuzzer.cc

@@ -66,12 +66,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) {
   // receiver has to assume that the broker has already duplicated the HANDLE
   // into the non-broker's process), but fuzzing that direction is not
   // interesting since a compromised broker process has much bigger problems.
-  //
-  // Note that in order for this hack to work properly, the remote process
-  // handle needs to be a "real" process handle rather than the pseudo-handle
-  // returned by GetCurrentProcessHandle(). Hence the use of OpenProcess().
-  receiver->set_remote_process(mojo::core::ScopedProcessHandle(
-      ::OpenProcess(PROCESS_ALL_ACCESS, FALSE, ::GetCurrentProcessId())));
+  receiver->set_remote_process(base::Process::Current());
 #endif
 
   receiver->Start();

mojo/core/channel_win.cc

@@ -114,13 +114,13 @@ class ChannelWin : public Channel,
   }
 
   void Write(MessagePtr message) override {
-    if (remote_process().is_valid()) {
+    if (remote_process().IsValid()) {
       // If we know the remote process handle, we transfer all outgoing handles
       // to the process now rewriting them in the message.
       std::vector<PlatformHandleInTransit> handles = message->TakeHandles();
       for (auto& handle : handles) {
         if (handle.handle().is_valid())
-          handle.TransferToProcess(remote_process().Clone());
+          handle.TransferToProcess(remote_process().Duplicate());
       }
       message->SetHandles(std::move(handles));
     }
@@ -172,12 +172,12 @@ class ChannelWin : public Channel,
           base::win::Uint32ToHandle(extra_header_handles[i].handle);
       if (PlatformHandleInTransit::IsPseudoHandle(handle_value))
         return false;
-      if (remote_process().is_valid()) {
+      if (remote_process().IsValid()) {
         // If we know the remote process's handle, we assume it doesn't know
         // ours; that means any handle values still belong to that process, and
         // we need to transfer them to this process.
         handle_value = PlatformHandleInTransit::TakeIncomingRemoteHandle(
-                           handle_value, remote_process().get())
+                           handle_value, remote_process().Handle())
                            .ReleaseHandle();
       }
       handles->emplace_back(base::win::ScopedHandle(std::move(handle_value)));