Convert callers of mojo::WrapMachPort to handle<platform>

Bug: 778317
Change-Id: I3fae34b8d7cc9d1d3431604f9537bdb6774c76a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2050138Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740537}

base/mac/scoped_mach_port.cc

@@ -63,5 +63,14 @@ bool CreateMachPort(ScopedMachReceiveRight* receive,
   return true;
 }
 
+ScopedMachSendRight RetainMachSendRight(mach_port_t port) {
+  kern_return_t kr =
+      mach_port_mod_refs(mach_task_self(), port, MACH_PORT_RIGHT_SEND, 1);
+  if (kr == KERN_SUCCESS)
+    return ScopedMachSendRight(port);
+  MACH_DLOG(ERROR, kr) << "mach_port_mod_refs +1";
+  return {};
+}
+
 }  // namespace mac
 }  // namespace base

base/mac/scoped_mach_port.h

@@ -71,6 +71,10 @@ BASE_EXPORT bool CreateMachPort(
     ScopedMachSendRight* send,
     Optional<mach_port_msgcount_t> queue_limit = nullopt);
 
+// Increases the user reference count for MACH_PORT_RIGHT_SEND by 1 and returns
+// a new scoper to manage the additional right.
+BASE_EXPORT ScopedMachSendRight RetainMachSendRight(mach_port_t port);
+
 }  // namespace mac
 }  // namespace base
 

content/browser/child_process_task_port_provider_mac.cc

@@ -53,15 +53,12 @@ ChildProcessTaskPortProvider::~ChildProcessTaskPortProvider() {}
 
 void ChildProcessTaskPortProvider::OnTaskPortReceived(
     base::ProcessHandle pid,
-    mojo::ScopedHandle task_port) {
-  base::mac::ScopedMachSendRight port;
-  if (mojo::UnwrapMachPort(
-          std::move(task_port),
-          base::mac::ScopedMachSendRight::Receiver(port).get()) !=
-      MOJO_RESULT_OK) {
-    DLOG(ERROR) << "Failed to unwrap task port for pid " << pid;
+    mojo::PlatformHandle task_port) {
+  if (!task_port.is_mach_send()) {
+    DLOG(ERROR) << "Invalid handle received as task port for pid " << pid;
     return;
   }
+  base::mac::ScopedMachSendRight port = task_port.TakeMachSendRight();
 
   // Request a notification from the kernel for when the port becomes a dead
   // name, indicating that the process has died.

content/browser/child_process_task_port_provider_mac.h

@@ -16,7 +16,7 @@
 #include "base/synchronization/lock.h"
 #include "content/common/child_process.mojom-forward.h"
 #include "content/common/content_export.h"
-#include "mojo/public/cpp/system/handle.h"
+#include "mojo/public/cpp/platform/platform_handle.h"
 
 namespace content {
 
@@ -51,7 +51,7 @@ class CONTENT_EXPORT ChildProcessTaskPortProvider : public base::PortProvider {
 
   // Callback for mojom::ChildProcess::GetTaskPort reply.
   void OnTaskPortReceived(base::ProcessHandle pid,
-                          mojo::ScopedHandle task_port);
+                          mojo::PlatformHandle task_port);
 
   // Event handler for |notification_source_|, invoked for
   // MACH_NOTIFY_DEAD_NAME.

content/browser/child_process_task_port_provider_mac_unittest.cc

@@ -131,7 +131,8 @@ TEST_F(ChildProcessTaskPortProviderTest, ChildLifecycle) {
   EXPECT_CALL(child_process, GetTaskPort(_))
       .WillOnce(WithArgs<0>(
           [&send_right](mojom::ChildProcess::GetTaskPortCallback callback) {
-            std::move(callback).Run(mojo::WrapMachPort(send_right.get()));
+            std::move(callback).Run(mojo::PlatformHandle(
+                base::mac::RetainMachSendRight(send_right.get())));
           }));
 
   provider()->OnChildProcessLaunched(99, &child_process);
@@ -177,8 +178,8 @@ TEST_F(ChildProcessTaskPortProviderTest, DeadTaskPort) {
       .WillOnce(
           WithArgs<0>([&task_runner, &receive_right, &send_right](
                           mojom::ChildProcess::GetTaskPortCallback callback) {
-            mojo::ScopedHandle mach_handle =
-                mojo::WrapMachPort(send_right.get());
+            mojo::PlatformHandle mach_handle(
+                base::mac::RetainMachSendRight(send_right.get()));
 
             // Destroy the receive right.
             task_runner->PostTask(
@@ -206,8 +207,10 @@ TEST_F(ChildProcessTaskPortProviderTest, DeadTaskPort) {
                           mojom::ChildProcess::GetTaskPortCallback callback) {
             task_runner->PostTask(
                 FROM_HERE,
-                base::BindOnce(std::move(callback),
-                               mojo::WrapMachPort(send_right2.get())));
+                base::BindOnce(
+                    std::move(callback),
+                    mojo::PlatformHandle(
+                        base::mac::RetainMachSendRight(send_right2.get()))));
           }));
 
   provider()->OnChildProcessLaunched(123, &child_contol2);
@@ -246,7 +249,8 @@ TEST_F(ChildProcessTaskPortProviderTest, ReplacePort) {
       .Times(2)
       .WillRepeatedly(WithArgs<0>(
           [&receive_right](mojom::ChildProcess::GetTaskPortCallback callback) {
-            std::move(callback).Run(mojo::WrapMachPort(receive_right.get()));
+            std::move(callback).Run(mojo::PlatformHandle(
+                base::mac::RetainMachSendRight(receive_right.get())));
           }));
 
   provider()->OnChildProcessLaunched(42, &child_process);
@@ -276,7 +280,8 @@ TEST_F(ChildProcessTaskPortProviderTest, ReplacePort) {
   EXPECT_CALL(child_process2, GetTaskPort(_))
       .WillOnce(
           [&send_right2](mojom::ChildProcess::GetTaskPortCallback callback) {
-            std::move(callback).Run(mojo::WrapMachPort(send_right2.get()));
+            std::move(callback).Run(mojo::PlatformHandle(
+                base::mac::RetainMachSendRight(send_right2.get())));
           });
 
   provider()->OnChildProcessLaunched(42, &child_process2);

content/child/child_thread_impl.cc

@@ -295,7 +295,8 @@ class ChildThreadImpl::IOThreadState
 
 #if defined(OS_MACOSX)
   void GetTaskPort(GetTaskPortCallback callback) override {
-    mojo::ScopedHandle task_port = mojo::WrapMachPort(mach_task_self());
+    mojo::PlatformHandle task_port(
+        (base::mac::ScopedMachSendRight(task_self_trap())));
     std::move(callback).Run(std::move(task_port));
   }
 #endif

content/common/child_process.mojom

@@ -48,7 +48,7 @@ interface ChildProcess {
 
   // Requests the child process send its Mach task port to the caller.
   [EnableIf=is_mac]
-  GetTaskPort() => (handle task_port);
+  GetTaskPort() => (handle<platform> task_port);
 
   // Tells the child process to begin or end IPC message logging.
   [EnableIf=ipc_logging]

mojo/public/cpp/system/platform_handle.cc

@@ -8,11 +8,6 @@
 #include "base/numerics/safe_conversions.h"
 #include "build/build_config.h"
 
-#if defined(OS_MACOSX) && !defined(OS_IOS)
-#include <mach/mach.h>
-#include "base/mac/mach_logging.h"
-#endif
-
 namespace mojo {
 
 namespace {
@@ -273,41 +268,4 @@ base::WritableSharedMemoryRegion UnwrapWritableSharedMemoryRegion(
       UnwrapPlatformSharedMemoryRegion(std::move(handle)));
 }
 
-#if defined(OS_MACOSX) && !defined(OS_IOS)
-ScopedHandle WrapMachPort(mach_port_t port) {
-  kern_return_t kr =
-      mach_port_mod_refs(mach_task_self(), port, MACH_PORT_RIGHT_SEND, 1);
-  MACH_LOG_IF(ERROR, kr != KERN_SUCCESS, kr)
-      << "MachPortAttachmentMac mach_port_mod_refs";
-  if (kr != KERN_SUCCESS)
-    return ScopedHandle();
-
-  MojoPlatformHandle platform_handle;
-  platform_handle.struct_size = sizeof(MojoPlatformHandle);
-  platform_handle.type = MOJO_PLATFORM_HANDLE_TYPE_MACH_PORT;
-  platform_handle.value = static_cast<uint64_t>(port);
-
-  MojoHandle mojo_handle;
-  MojoResult result =
-      MojoWrapPlatformHandle(&platform_handle, nullptr, &mojo_handle);
-  CHECK_EQ(result, MOJO_RESULT_OK);
-
-  return ScopedHandle(Handle(mojo_handle));
-}
-
-MojoResult UnwrapMachPort(ScopedHandle handle, mach_port_t* port) {
-  MojoPlatformHandle platform_handle;
-  platform_handle.struct_size = sizeof(MojoPlatformHandle);
-  MojoResult result = MojoUnwrapPlatformHandle(handle.release().value(),
-                                               nullptr, &platform_handle);
-  if (result != MOJO_RESULT_OK)
-    return result;
-
-  CHECK(platform_handle.type == MOJO_PLATFORM_HANDLE_TYPE_MACH_PORT ||
-        platform_handle.type == MOJO_PLATFORM_HANDLE_TYPE_INVALID);
-  *port = static_cast<mach_port_t>(platform_handle.value);
-  return MOJO_RESULT_OK;
-}
-#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
-
 }  // namespace mojo

mojo/public/cpp/system/platform_handle.h

@@ -122,17 +122,6 @@ UnwrapUnsafeSharedMemoryRegion(ScopedSharedBufferHandle handle);
 MOJO_CPP_SYSTEM_EXPORT base::WritableSharedMemoryRegion
 UnwrapWritableSharedMemoryRegion(ScopedSharedBufferHandle handle);
 
-#if defined(OS_MACOSX) && !defined(OS_IOS)
-// Wraps a mach_port_t as a Mojo handle. This takes a reference to the
-// Mach port.
-MOJO_CPP_SYSTEM_EXPORT ScopedHandle WrapMachPort(mach_port_t port);
-
-// Unwraps a mach_port_t from a Mojo handle. The caller gets ownership of the
-// Mach port.
-MOJO_CPP_SYSTEM_EXPORT MojoResult UnwrapMachPort(ScopedHandle handle,
-                                                 mach_port_t* port);
-#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
-
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_SYSTEM_PLATFORM_HANDLE_H_

ui/gfx/mojom/buffer_types.mojom

@@ -98,7 +98,7 @@ union GpuMemoryBufferPlatformHandle {
   NativePixmapHandle native_pixmap_handle;
 
   [EnableIf=is_mac]
-  handle mach_port;
+  handle<platform> mach_port;
 
   [EnableIf=is_win]
   handle dxgi_handle;

ui/gfx/mojom/buffer_types_mojom_traits.cc

@@ -97,7 +97,8 @@ gfx::mojom::GpuMemoryBufferPlatformHandlePtr StructTraits<
     case gfx::IO_SURFACE_BUFFER:
 #if defined(OS_MACOSX) && !defined(OS_IOS)
       return gfx::mojom::GpuMemoryBufferPlatformHandle::NewMachPort(
-          mojo::WrapMachPort(handle.mach_port.get()));
+          mojo::PlatformHandle(
+              base::mac::RetainMachSendRight(handle.mach_port.get())));
 #else
       break;
 #endif
@@ -174,12 +175,10 @@ bool StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
     case gfx::mojom::GpuMemoryBufferPlatformHandleDataView::Tag::MACH_PORT: {
       out->type = gfx::IO_SURFACE_BUFFER;
-      mach_port_t mach_port;
-      MojoResult unwrap_result = mojo::UnwrapMachPort(
-          std::move(platform_handle->get_mach_port()), &mach_port);
-      if (unwrap_result != MOJO_RESULT_OK)
+      if (!platform_handle->get_mach_port().is_mach_send())
         return false;
-      out->mach_port.reset(mach_port);
+      out->mach_port.reset(
+          platform_handle->get_mach_port().ReleaseMachSendRight());
       return true;
     }
 #elif defined(OS_WIN)

ui/gfx/mojom/ca_layer_params.mojom

@@ -8,7 +8,7 @@ import "ui/gfx/geometry/mojom/geometry.mojom";
 
 union CALayerContent {
   uint32 ca_context_id;
-  handle io_surface_mach_port;
+  handle<platform> io_surface_mach_port;
 };
 
 // gfx::CALayerParams

ui/gfx/mojom/ca_layer_params_mojom_traits.cc

@@ -17,7 +17,8 @@ StructTraits<gfx::mojom::CALayerParamsDataView, gfx::CALayerParams>::content(
   if (ca_layer_params.io_surface_mach_port) {
     DCHECK(!ca_layer_params.ca_context_id);
     return gfx::mojom::CALayerContent::NewIoSurfaceMachPort(
-        mojo::WrapMachPort(ca_layer_params.io_surface_mach_port.get()));
+        mojo::PlatformHandle(base::mac::RetainMachSendRight(
+            ca_layer_params.io_surface_mach_port.get())));
   }
 #endif
   return gfx::mojom::CALayerContent::NewCaContextId(
@@ -37,12 +38,11 @@ bool StructTraits<gfx::mojom::CALayerParamsDataView, gfx::CALayerParams>::Read(
       break;
     case gfx::mojom::CALayerContentDataView::Tag::IO_SURFACE_MACH_PORT:
 #if defined(OS_MACOSX) && !defined(OS_IOS)
-      mach_port_t io_surface_mach_port;
-      MojoResult unwrap_result = mojo::UnwrapMachPort(
-          content_data.TakeIoSurfaceMachPort(), &io_surface_mach_port);
-      if (unwrap_result != MOJO_RESULT_OK)
+      mojo::PlatformHandle platform_handle =
+          content_data.TakeIoSurfaceMachPort();
+      if (!platform_handle.is_mach_send())
         return false;
-      out->io_surface_mach_port.reset(io_surface_mach_port);
+      out->io_surface_mach_port.reset(platform_handle.ReleaseMachSendRight());
 #else
       return false;
 #endif