Update Mojo platform handle wrapping APIs

Updates platform handle wrapping/unwrapping API signatures to be more
extensible for a stable ABI, and adds support to shared memory region
wrapping/unwrapping for writable regions, which may be represented by
multiple platform handles on some platforms.

TBR=bajones@chromium.org

Bug: 826213,842037
Change-Id: Ia0e5da7b7e0f4c41853eb3a8d4a12da47071dd8c
Reviewed-on: https://chromium-review.googlesource.com/1058440Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559165}

device/vr/oculus/oculus_render_loop.cc

@@ -92,7 +92,7 @@ void OculusRenderLoop::SubmitFrameWithTextureHandle(
   MojoPlatformHandle platform_handle;
   platform_handle.struct_size = sizeof(platform_handle);
   MojoResult result = MojoUnwrapPlatformHandle(texture_handle.release().value(),
-                                               &platform_handle);
+                                               nullptr, &platform_handle);
   if (result != MOJO_RESULT_OK)
     return;
 

device/vr/openvr/openvr_render_loop.cc

@@ -85,7 +85,7 @@ void OpenVRRenderLoop::SubmitFrameWithTextureHandle(
   MojoPlatformHandle platform_handle;
   platform_handle.struct_size = sizeof(platform_handle);
   MojoResult result = MojoUnwrapPlatformHandle(texture_handle.release().value(),
-                                               &platform_handle);
+                                               nullptr, &platform_handle);
   if (result != MOJO_RESULT_OK)
     return;
 

ipc/ipc_message_attachment.cc

@@ -71,7 +71,7 @@ mojo::ScopedHandle MessageAttachment::TakeMojoHandle() {
           sizeof(platform_handle), MOJO_PLATFORM_HANDLE_TYPE_MACH_PORT,
           static_cast<uint64_t>(attachment->get_mach_port())};
       MojoHandle wrapped_handle;
-      if (MojoWrapPlatformHandle(&platform_handle, &wrapped_handle) !=
+      if (MojoWrapPlatformHandle(&platform_handle, nullptr, &wrapped_handle) !=
           MOJO_RESULT_OK) {
         return mojo::ScopedHandle();
       }
@@ -85,7 +85,7 @@ mojo::ScopedHandle MessageAttachment::TakeMojoHandle() {
           sizeof(platform_handle), MOJO_PLATFORM_HANDLE_TYPE_FUCHSIA_HANDLE,
           static_cast<uint64_t>(attachment->Take())};
       MojoHandle wrapped_handle;
-      if (MojoWrapPlatformHandle(&platform_handle, &wrapped_handle) !=
+      if (MojoWrapPlatformHandle(&platform_handle, nullptr, &wrapped_handle) !=
           MOJO_RESULT_OK) {
         return mojo::ScopedHandle();
       }
@@ -111,8 +111,8 @@ scoped_refptr<MessageAttachment> MessageAttachment::CreateFromMojoHandle(
     return new internal::MojoHandleAttachment(std::move(handle));
 
   MojoPlatformHandle platform_handle = {sizeof(platform_handle), 0, 0};
-  MojoResult unwrap_result =
-      MojoUnwrapPlatformHandle(handle.release().value(), &platform_handle);
+  MojoResult unwrap_result = MojoUnwrapPlatformHandle(
+      handle.release().value(), nullptr, &platform_handle);
   if (unwrap_result != MOJO_RESULT_OK)
     return nullptr;
 

mojo/edk/embedder/entrypoints.cc

@@ -245,34 +245,44 @@ MojoResult MojoArmTrapImpl(MojoHandle trap_handle,
                          ready_triggers, ready_results, ready_signals_states);
 }
 
-MojoResult MojoWrapPlatformHandleImpl(const MojoPlatformHandle* platform_handle,
-                                      MojoHandle* mojo_handle) {
-  return g_core->WrapPlatformHandle(platform_handle, mojo_handle);
+MojoResult MojoWrapPlatformHandleImpl(
+    const MojoPlatformHandle* platform_handle,
+    const MojoWrapPlatformHandleOptions* options,
+    MojoHandle* mojo_handle) {
+  return g_core->WrapPlatformHandle(platform_handle, options, mojo_handle);
 }
 
-MojoResult MojoUnwrapPlatformHandleImpl(MojoHandle mojo_handle,
-                                        MojoPlatformHandle* platform_handle) {
-  return g_core->UnwrapPlatformHandle(mojo_handle, platform_handle);
+MojoResult MojoUnwrapPlatformHandleImpl(
+    MojoHandle mojo_handle,
+    const MojoUnwrapPlatformHandleOptions* options,
+    MojoPlatformHandle* platform_handle) {
+  return g_core->UnwrapPlatformHandle(mojo_handle, options, platform_handle);
 }
 
-MojoResult MojoWrapPlatformSharedBufferHandleImpl(
-    const MojoPlatformHandle* platform_handle,
-    size_t num_bytes,
+MojoResult MojoWrapPlatformSharedMemoryRegionImpl(
+    const MojoPlatformHandle* platform_handles,
+    uint32_t num_platform_handles,
+    uint64_t num_bytes,
     const MojoSharedBufferGuid* guid,
-    MojoPlatformSharedBufferHandleFlags flags,
+    MojoPlatformSharedMemoryRegionAccessMode access_mode,
+    const MojoWrapPlatformSharedMemoryRegionOptions* options,
     MojoHandle* mojo_handle) {
-  return g_core->WrapPlatformSharedBufferHandle(platform_handle, num_bytes,
-                                                guid, flags, mojo_handle);
+  return g_core->WrapPlatformSharedMemoryRegion(
+      platform_handles, num_platform_handles, num_bytes, guid, access_mode,
+      options, mojo_handle);
 }
 
-MojoResult MojoUnwrapPlatformSharedBufferHandleImpl(
+MojoResult MojoUnwrapPlatformSharedMemoryRegionImpl(
     MojoHandle mojo_handle,
-    MojoPlatformHandle* platform_handle,
-    size_t* num_bytes,
+    const MojoUnwrapPlatformSharedMemoryRegionOptions* options,
+    MojoPlatformHandle* platform_handles,
+    uint32_t* num_platform_handles,
+    uint64_t* num_bytes,
     MojoSharedBufferGuid* guid,
-    MojoPlatformSharedBufferHandleFlags* flags) {
-  return g_core->UnwrapPlatformSharedBufferHandle(mojo_handle, platform_handle,
-                                                  num_bytes, guid, flags);
+    MojoPlatformSharedMemoryRegionAccessMode* access_mode) {
+  return g_core->UnwrapPlatformSharedMemoryRegion(
+      mojo_handle, options, platform_handles, num_platform_handles, num_bytes,
+      guid, access_mode);
 }
 
 }  // extern "C"
@@ -312,8 +322,8 @@ MojoSystemThunks g_thunks = {sizeof(MojoSystemThunks),
                              MojoArmTrapImpl,
                              MojoWrapPlatformHandleImpl,
                              MojoUnwrapPlatformHandleImpl,
-                             MojoWrapPlatformSharedBufferHandleImpl,
-                             MojoUnwrapPlatformSharedBufferHandleImpl};
+                             MojoWrapPlatformSharedMemoryRegionImpl,
+                             MojoUnwrapPlatformSharedMemoryRegionImpl};
 
 }  // namespace
 

mojo/edk/system/core.cc

@@ -1035,8 +1035,10 @@ MojoResult Core::GetBufferInfo(MojoHandle buffer_handle,
   return dispatcher->GetBufferInfo(info);
 }
 
-MojoResult Core::WrapPlatformHandle(const MojoPlatformHandle* platform_handle,
-                                    MojoHandle* mojo_handle) {
+MojoResult Core::WrapPlatformHandle(
+    const MojoPlatformHandle* platform_handle,
+    const MojoWrapPlatformHandleOptions* options,
+    MojoHandle* mojo_handle) {
   ScopedPlatformHandle handle;
   MojoResult result =
       MojoPlatformHandleToScopedPlatformHandle(platform_handle, &handle);
@@ -1046,8 +1048,10 @@ MojoResult Core::WrapPlatformHandle(const MojoPlatformHandle* platform_handle,
   return CreatePlatformHandleWrapper(std::move(handle), mojo_handle);
 }
 
-MojoResult Core::UnwrapPlatformHandle(MojoHandle mojo_handle,
-                                      MojoPlatformHandle* platform_handle) {
+MojoResult Core::UnwrapPlatformHandle(
+    MojoHandle mojo_handle,
+    const MojoUnwrapPlatformHandleOptions* options,
+    MojoPlatformHandle* platform_handle) {
   ScopedPlatformHandle handle;
   MojoResult result = PassWrappedPlatformHandle(mojo_handle, &handle);
   if (result != MOJO_RESULT_OK)
@@ -1057,43 +1061,68 @@ MojoResult Core::UnwrapPlatformHandle(MojoHandle mojo_handle,
                                                   platform_handle);
 }
 
-MojoResult Core::WrapPlatformSharedBufferHandle(
-    const MojoPlatformHandle* platform_handle,
-    size_t size,
+MojoResult Core::WrapPlatformSharedMemoryRegion(
+    const MojoPlatformHandle* platform_handles,
+    uint32_t num_platform_handles,
+    uint64_t size,
     const MojoSharedBufferGuid* guid,
-    MojoPlatformSharedBufferHandleFlags flags,
+    MojoPlatformSharedMemoryRegionAccessMode access_mode,
+    const MojoWrapPlatformSharedMemoryRegionOptions* options,
     MojoHandle* mojo_handle) {
   DCHECK(size);
-  ScopedPlatformHandle handle;
-  MojoResult result =
-      MojoPlatformHandleToScopedPlatformHandle(platform_handle, &handle);
-  if (result != MOJO_RESULT_OK)
-    return result;
+
+#if defined(OS_POSIX) && !defined(OS_ANDROID) && !defined(OS_FUCHSIA) && \
+    (!defined(OS_MACOSX) || defined(OS_IOS))
+  if (access_mode == MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_WRITABLE) {
+    if (num_platform_handles != 2)
+      return MOJO_RESULT_INVALID_ARGUMENT;
+  }
+#else
+  if (num_platform_handles != 1)
+    return MOJO_RESULT_INVALID_ARGUMENT;
+#endif
+
+  ScopedPlatformHandle handles[2];
+  bool handles_ok = true;
+  for (size_t i = 0; i < num_platform_handles; ++i) {
+    MojoResult result = MojoPlatformHandleToScopedPlatformHandle(
+        &platform_handles[i], &handles[i]);
+    if (result != MOJO_RESULT_OK)
+      handles_ok = false;
+  }
+  if (!handles_ok)
+    return MOJO_RESULT_INVALID_ARGUMENT;
 
   base::UnguessableToken token =
       base::UnguessableToken::Deserialize(guid->high, guid->low);
-  const bool read_only =
-      flags & MOJO_PLATFORM_SHARED_BUFFER_HANDLE_FLAG_HANDLE_IS_READ_ONLY;
 
-  // TODO(https://crbug.com/826213): Support proper wrapping/unwrapping of
-  // shared memory region handles instead of forcing the wrapped handles to
-  // always be read-only or unsafe. This requires first extending the
-  // wrap/unwrap API to support multiple platform handles as input/output.
-  base::subtle::PlatformSharedMemoryRegion::Mode mode =
-      read_only ? base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly
-                : base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe;
+  base::subtle::PlatformSharedMemoryRegion::Mode mode;
+  switch (access_mode) {
+    case MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_READ_ONLY:
+      mode = base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly;
+      break;
+    case MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_WRITABLE:
+      mode = base::subtle::PlatformSharedMemoryRegion::Mode::kWritable;
+      break;
+    case MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_UNSAFE:
+      mode = base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe;
+      break;
+    default:
+      return MOJO_RESULT_INVALID_ARGUMENT;
+  }
 
   base::subtle::PlatformSharedMemoryRegion region =
       base::subtle::PlatformSharedMemoryRegion::Take(
           CreateSharedMemoryRegionHandleFromPlatformHandles(
-              std::move(handle), ScopedPlatformHandle()),
+              std::move(handles[0]), std::move(handles[1])),
           mode, size, token);
   if (!region.IsValid())
     return MOJO_RESULT_UNKNOWN;
 
   scoped_refptr<SharedBufferDispatcher> dispatcher;
-  result = SharedBufferDispatcher::CreateFromPlatformSharedMemoryRegion(
-      std::move(region), &dispatcher);
+  MojoResult result =
+      SharedBufferDispatcher::CreateFromPlatformSharedMemoryRegion(
+          std::move(region), &dispatcher);
   if (result != MOJO_RESULT_OK)
     return result;
 
@@ -1107,12 +1136,14 @@ MojoResult Core::WrapPlatformSharedBufferHandle(
   return MOJO_RESULT_OK;
 }
 
-MojoResult Core::UnwrapPlatformSharedBufferHandle(
+MojoResult Core::UnwrapPlatformSharedMemoryRegion(
     MojoHandle mojo_handle,
-    MojoPlatformHandle* platform_handle,
-    size_t* size,
+    const MojoUnwrapPlatformSharedMemoryRegionOptions* options,
+    MojoPlatformHandle* platform_handles,
+    uint32_t* num_platform_handles,
+    uint64_t* size,
     MojoSharedBufferGuid* guid,
-    MojoPlatformSharedBufferHandleFlags* flags) {
+    MojoPlatformSharedMemoryRegionAccessMode* access_mode) {
   scoped_refptr<Dispatcher> dispatcher;
   MojoResult result = MOJO_RESULT_OK;
   {
@@ -1139,11 +1170,19 @@ MojoResult Core::UnwrapPlatformSharedBufferHandle(
   guid->high = token.GetHighForSerialization();
   guid->low = token.GetLowForSerialization();
 
-  DCHECK(flags);
-  *flags = MOJO_PLATFORM_SHARED_BUFFER_HANDLE_FLAG_NONE;
-  if (region.GetMode() ==
-      base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly) {
-    *flags |= MOJO_PLATFORM_SHARED_BUFFER_HANDLE_FLAG_HANDLE_IS_READ_ONLY;
+  DCHECK(access_mode);
+  switch (region.GetMode()) {
+    case base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly:
+      *access_mode = MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_READ_ONLY;
+      break;
+    case base::subtle::PlatformSharedMemoryRegion::Mode::kWritable:
+      *access_mode = MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_WRITABLE;
+      break;
+    case base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe:
+      *access_mode = MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_UNSAFE;
+      break;
+    default:
+      return MOJO_RESULT_INVALID_ARGUMENT;
   }
 
   ScopedPlatformHandle handle;
@@ -1151,11 +1190,31 @@ MojoResult Core::UnwrapPlatformSharedBufferHandle(
   ExtractPlatformHandlesFromSharedMemoryRegionHandle(
       region.PassPlatformHandle(), &handle, &read_only_handle);
 
-  // TODO(https://crbug.com/826213): Once we support proper wrapping of shared
-  // memory handles, don't drop |read_only_handle| on the ground.
+  const uint32_t available_handle_storage_slots = *num_platform_handles;
+  if (available_handle_storage_slots < 1)
+    return MOJO_RESULT_RESOURCE_EXHAUSTED;
+  *num_platform_handles = 1;
+#if defined(OS_POSIX) && !defined(OS_ANDROID) && !defined(OS_FUCHSIA) && \
+    (!defined(OS_MACOSX) || defined(OS_IOS))
+  if (region.GetMode() ==
+      base::subtle::PlatformSharedMemoryRegion::Mode::kWritable) {
+    if (available_handle_storage_slots < 2)
+      return MOJO_RESULT_INVALID_ARGUMENT;
+    if (ScopedPlatformHandleToMojoPlatformHandle(std::move(read_only_handle),
+                                                 &platform_handles[1]) !=
+        MOJO_RESULT_OK) {
+      return MOJO_RESULT_INVALID_ARGUMENT;
+    }
+    *num_platform_handles = 2;
+  }
+#endif
+
+  if (ScopedPlatformHandleToMojoPlatformHandle(
+          std::move(handle), &platform_handles[0]) != MOJO_RESULT_OK) {
+    return MOJO_RESULT_INVALID_ARGUMENT;
+  }
 
-  return ScopedPlatformHandleToMojoPlatformHandle(std::move(handle),
-                                                  platform_handle);
+  return MOJO_RESULT_OK;
 }
 
 void Core::GetActiveHandlesForTest(std::vector<MojoHandle>* handles) {

mojo/edk/system/core.h

@@ -281,21 +281,28 @@ class MOJO_SYSTEM_IMPL_EXPORT Core {
   // These methods correspond to the API functions defined in
   // "mojo/public/c/system/platform_handle.h".
   MojoResult WrapPlatformHandle(const MojoPlatformHandle* platform_handle,
+                                const MojoWrapPlatformHandleOptions* options,
                                 MojoHandle* mojo_handle);
-  MojoResult UnwrapPlatformHandle(MojoHandle mojo_handle,
-                                  MojoPlatformHandle* platform_handle);
-  MojoResult WrapPlatformSharedBufferHandle(
-      const MojoPlatformHandle* platform_handle,
-      size_t size,
+  MojoResult UnwrapPlatformHandle(
+      MojoHandle mojo_handle,
+      const MojoUnwrapPlatformHandleOptions* options,
+      MojoPlatformHandle* platform_handle);
+  MojoResult WrapPlatformSharedMemoryRegion(
+      const MojoPlatformHandle* platform_handles,
+      uint32_t num_platform_handles,
+      uint64_t size,
       const MojoSharedBufferGuid* guid,
-      MojoPlatformSharedBufferHandleFlags flags,
+      MojoPlatformSharedMemoryRegionAccessMode access_mode,
+      const MojoWrapPlatformSharedMemoryRegionOptions* options,
       MojoHandle* mojo_handle);
-  MojoResult UnwrapPlatformSharedBufferHandle(
+  MojoResult UnwrapPlatformSharedMemoryRegion(
       MojoHandle mojo_handle,
-      MojoPlatformHandle* platform_handle,
-      size_t* size,
+      const MojoUnwrapPlatformSharedMemoryRegionOptions* options,
+      MojoPlatformHandle* platform_handles,
+      uint32_t* num_platform_handles,
+      uint64_t* size,
       MojoSharedBufferGuid* guid,
-      MojoPlatformSharedBufferHandleFlags* flags);
+      MojoPlatformSharedMemoryRegionAccessMode* access_mode);
 
   void GetActiveHandlesForTest(std::vector<MojoHandle>* handles);
 

mojo/edk/system/message_pipe_unittest.cc

@@ -318,7 +318,7 @@ TEST_F(MessagePipeTest, BasicWaiting) {
 
 #if !defined(OS_IOS)
 
-const size_t kPingPongHandlesPerIteration = 50;
+const size_t kPingPongHandlesPerIteration = 30;
 const size_t kPingPongIterations = 500;
 
 DEFINE_TEST_CLIENT_TEST_WITH_PIPE(HandlePingPong, MessagePipeTest, h) {

mojo/edk/system/platform_wrapper_unittest.cc

@@ -83,7 +83,7 @@ TEST_F(PlatformWrapperTest, WrapPlatformHandle) {
     os_file.value =
         PlatformHandleValueFromPlatformFile(file.TakePlatformFile());
     EXPECT_EQ(MOJO_RESULT_OK,
-              MojoWrapPlatformHandle(&os_file, &wrapped_handle));
+              MojoWrapPlatformHandle(&os_file, nullptr, &wrapped_handle));
 
     WriteMessageWithHandles(h, kMessage, &wrapped_handle, 1);
   });
@@ -98,8 +98,8 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(ReadPlatformFile, PlatformWrapperTest, h) {
 
   MojoPlatformHandle platform_handle;
   platform_handle.struct_size = sizeof(MojoPlatformHandle);
-  ASSERT_EQ(MOJO_RESULT_OK,
-            MojoUnwrapPlatformHandle(wrapped_handle, &platform_handle));
+  ASSERT_EQ(MOJO_RESULT_OK, MojoUnwrapPlatformHandle(wrapped_handle, nullptr,
+                                                     &platform_handle));
   EXPECT_EQ(SIMPLE_PLATFORM_HANDLE_TYPE, platform_handle.type);
   base::File file(PlatformFileFromPlatformHandleValue(platform_handle.value));
 
@@ -110,7 +110,7 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(ReadPlatformFile, PlatformWrapperTest, h) {
   EXPECT_TRUE(std::equal(message.begin(), message.end(), data.begin()));
 }
 
-TEST_F(PlatformWrapperTest, WrapPlatformSharedBufferHandle) {
+TEST_F(PlatformWrapperTest, WrapPlatformSharedMemoryRegion) {
   // Allocate a new platform shared buffer and write a message into it.
   const std::string kMessage = "Hello, world!";
   base::SharedMemory buffer;
@@ -142,10 +142,11 @@ TEST_F(PlatformWrapperTest, WrapPlatformSharedBufferHandle) {
     mojo_guid.low = guid.GetLowForSerialization();
 
     MojoHandle wrapped_handle;
-    ASSERT_EQ(MOJO_RESULT_OK, MojoWrapPlatformSharedBufferHandle(
-                                  &os_buffer, kMessage.size(), &mojo_guid,
-                                  MOJO_PLATFORM_SHARED_BUFFER_HANDLE_FLAG_NONE,
-                                  &wrapped_handle));
+    ASSERT_EQ(MOJO_RESULT_OK,
+              MojoWrapPlatformSharedMemoryRegion(
+                  &os_buffer, 1, kMessage.size(), &mojo_guid,
+                  MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_UNSAFE,
+                  nullptr, &wrapped_handle));
     WriteMessageWithHandles(h, kMessage, &wrapped_handle, 1);
 
     // As a sanity check, send the GUID explicitly in a second message. We'll
@@ -170,14 +171,14 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(ReadPlatformSharedBuffer,
   // works as expected.
   MojoPlatformHandle os_buffer;
   os_buffer.struct_size = sizeof(MojoPlatformHandle);
-  size_t size;
+  uint32_t num_handles = 1;
+  uint64_t size;
   MojoSharedBufferGuid mojo_guid;
-  MojoPlatformSharedBufferHandleFlags flags;
-  ASSERT_EQ(MOJO_RESULT_OK,
-            MojoUnwrapPlatformSharedBufferHandle(wrapped_handle, &os_buffer,
-                                                 &size, &mojo_guid, &flags));
-  bool read_only = flags & MOJO_PLATFORM_SHARED_BUFFER_HANDLE_FLAG_NONE;
-  EXPECT_FALSE(read_only);
+  MojoPlatformSharedMemoryRegionAccessMode access_mode;
+  ASSERT_EQ(MOJO_RESULT_OK, MojoUnwrapPlatformSharedMemoryRegion(
+                                wrapped_handle, nullptr, &os_buffer,
+                                &num_handles, &size, &mojo_guid, &access_mode));
+  EXPECT_EQ(MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_UNSAFE, access_mode);
 
   base::UnguessableToken guid =
       base::UnguessableToken::Deserialize(mojo_guid.high, mojo_guid.low);
@@ -200,7 +201,7 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(ReadPlatformSharedBuffer,
       guid);
 #endif
 
-  base::SharedMemory memory(memory_handle, read_only);
+  base::SharedMemory memory(memory_handle, false);
   memory.Map(message.size());
   ASSERT_TRUE(memory.memory());
 
@@ -228,9 +229,9 @@ TEST_F(PlatformWrapperTest, InvalidHandle) {
   invalid_handle.struct_size = sizeof(MojoPlatformHandle);
   invalid_handle.type = MOJO_PLATFORM_HANDLE_TYPE_INVALID;
   EXPECT_EQ(MOJO_RESULT_OK,
-            MojoWrapPlatformHandle(&invalid_handle, &wrapped_handle));
+            MojoWrapPlatformHandle(&invalid_handle, nullptr, &wrapped_handle));
   EXPECT_EQ(MOJO_RESULT_OK,
-            MojoUnwrapPlatformHandle(wrapped_handle, &invalid_handle));
+            MojoUnwrapPlatformHandle(wrapped_handle, nullptr, &invalid_handle));
   EXPECT_EQ(MOJO_PLATFORM_HANDLE_TYPE_INVALID, invalid_handle.type);
 }
 
@@ -240,7 +241,7 @@ TEST_F(PlatformWrapperTest, InvalidArgument) {
   MojoPlatformHandle platform_handle;
   platform_handle.struct_size = 0;
   EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
-            MojoWrapPlatformHandle(&platform_handle, &wrapped_handle));
+            MojoWrapPlatformHandle(&platform_handle, nullptr, &wrapped_handle));
 }
 
 }  // namespace

mojo/edk/system/shared_buffer_dispatcher.cc

@@ -19,6 +19,7 @@
 #include "mojo/edk/system/node_controller.h"
 #include "mojo/edk/system/options_validation.h"
 #include "mojo/edk/system/platform_shared_memory_mapping.h"
+#include "mojo/public/c/system/platform_handle.h"
 
 namespace mojo {
 namespace edk {
@@ -29,14 +30,12 @@ namespace {
 
 struct SerializedState {
   uint64_t num_bytes;
-  uint32_t flags;
+  uint32_t access_mode;
   uint64_t guid_high;
   uint64_t guid_low;
   uint32_t padding;
 };
 
-const uint32_t kSerializedStateFlagsReadOnly = 1 << 0;
-
 #pragma pack(pop)
 
 static_assert(sizeof(SerializedState) % 8 == 0,
@@ -138,24 +137,48 @@ scoped_refptr<SharedBufferDispatcher> SharedBufferDispatcher::Deserialize(
     return nullptr;
   }
 
-  if (num_platform_handles != 1 || num_ports) {
-    LOG(ERROR)
-        << "Invalid serialized shared buffer dispatcher (missing handles)";
+  if (num_ports)
     return nullptr;
+
+  ScopedPlatformHandle handles[2];
+#if defined(OS_POSIX) && !defined(OS_ANDROID) && !defined(OS_FUCHSIA) && \
+    (!defined(OS_MACOSX) || defined(OS_IOS))
+  if (serialized_state->access_mode ==
+      MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_WRITABLE) {
+    if (num_platform_handles != 2)
+      return nullptr;
+    handles[1] = std::move(platform_handles[1]);
+  } else {
+    if (num_platform_handles != 1)
+      return nullptr;
   }
+#else
+  if (num_platform_handles != 1)
+    return nullptr;
+#endif
+  handles[0] = std::move(platform_handles[0]);
 
   base::UnguessableToken guid = base::UnguessableToken::Deserialize(
       serialized_state->guid_high, serialized_state->guid_low);
 
-  // TODO(https://crbug.com/826213): Support proper serialization and
-  // deserialization of writable regions as well.
-  base::subtle::PlatformSharedMemoryRegion::Mode mode =
-      serialized_state->flags & kSerializedStateFlagsReadOnly
-          ? base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly
-          : base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe;
+  base::subtle::PlatformSharedMemoryRegion::Mode mode;
+  switch (serialized_state->access_mode) {
+    case MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_READ_ONLY:
+      mode = base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly;
+      break;
+    case MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_WRITABLE:
+      mode = base::subtle::PlatformSharedMemoryRegion::Mode::kWritable;
+      break;
+    case MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_UNSAFE:
+      mode = base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe;
+      break;
+    default:
+      LOG(ERROR) << "Invalid serialized shared buffer access mode.";
+      return nullptr;
+  }
   auto region = base::subtle::PlatformSharedMemoryRegion::Take(
-      CreateSharedMemoryRegionHandleFromPlatformHandles(
-          std::move(platform_handles[0]), ScopedPlatformHandle()),
+      CreateSharedMemoryRegionHandleFromPlatformHandles(std::move(handles[0]),
+                                                        std::move(handles[1])),
       mode, static_cast<size_t>(serialized_state->num_bytes), guid);
   if (!region.IsValid()) {
     LOG(ERROR)
@@ -288,6 +311,13 @@ void SharedBufferDispatcher::StartSerialize(uint32_t* num_bytes,
   *num_bytes = sizeof(SerializedState);
   *num_ports = 0;
   *num_platform_handles = 1;
+#if defined(OS_POSIX) && !defined(OS_ANDROID) && !defined(OS_FUCHSIA) && \
+    (!defined(OS_MACOSX) || defined(OS_IOS))
+  if (region_.GetMode() ==
+      base::subtle::PlatformSharedMemoryRegion::Mode::kWritable) {
+    *num_platform_handles = 2;
+  }
+#endif
 }
 
 bool SharedBufferDispatcher::EndSerialize(void* destination,
@@ -297,20 +327,43 @@ bool SharedBufferDispatcher::EndSerialize(void* destination,
       static_cast<SerializedState*>(destination);
   base::AutoLock lock(lock_);
   serialized_state->num_bytes = region_.GetSize();
-  serialized_state->flags =
-      region_.GetMode() ==
-              base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly
-          ? kSerializedStateFlagsReadOnly
-          : 0;
+  switch (region_.GetMode()) {
+    case base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly:
+      serialized_state->access_mode =
+          MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_READ_ONLY;
+      break;
+    case base::subtle::PlatformSharedMemoryRegion::Mode::kWritable:
+      serialized_state->access_mode =
+          MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_WRITABLE;
+      break;
+    case base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe:
+      serialized_state->access_mode =
+          MOJO_PLATFORM_SHARED_MEMORY_REGION_ACCESS_MODE_UNSAFE;
+      break;
+    default:
+      NOTREACHED();
+      return false;
+  }
+
   const base::UnguessableToken& guid = region_.GetGUID();
   serialized_state->guid_high = guid.GetHighForSerialization();
   serialized_state->guid_low = guid.GetLowForSerialization();
   serialized_state->padding = 0;
 
+  auto region = std::move(region_);
+#if defined(OS_POSIX) && !defined(OS_ANDROID) && !defined(OS_FUCHSIA) && \
+    (!defined(OS_MACOSX) || defined(OS_IOS))
+  if (region.GetMode() ==
+      base::subtle::PlatformSharedMemoryRegion::Mode::kWritable) {
+    ExtractPlatformHandlesFromSharedMemoryRegionHandle(
+        region.PassPlatformHandle(), &handles[0], &handles[1]);
+    return true;
+  }
+#endif
+
   ScopedPlatformHandle ignored_handle;
   ExtractPlatformHandlesFromSharedMemoryRegionHandle(
-      region_.PassPlatformHandle(), &handles[0], &ignored_handle);
-  region_ = base::subtle::PlatformSharedMemoryRegion();
+      region.PassPlatformHandle(), &handles[0], &ignored_handle);
   return true;
 }
 

mojo/public/c/system/README.md

@@ -543,7 +543,7 @@ platform_handle.struct_size = sizeof(platform_handle);
 platform_handle.type = MOJO_PLATFORM_HANDLE_TYPE_FILE_DESCRIPTOR;
 platform_handle.value = (uint64_t)fd;
 MojoHandle handle;
-MojoResult result = MojoWrapPlatformHandle(&platform_handle, &handle);
+MojoResult result = MojoWrapPlatformHandle(&platform_handle, nullptr, &handle);
 ```
 
 Note that at this point `handle` effectively owns the file descriptor
@@ -554,7 +554,7 @@ over a message pipe, and now we want to unwrap it on the other side:
 ``` c
 MojoPlatformHandle platform_handle;
 platform_handle.struct_size = sizeof(platform_handle);
-MojoResult result = MojoUnwrapPlatformHandle(handle, &platform_handle);
+MojoResult result = MojoUnwrapPlatformHandle(handle, nullptr, &platform_handle);
 int fd = (int)platform_handle.value;
 ```
 

mojo/public/c/system/thunks.cc

@@ -312,36 +312,43 @@ MojoResult MojoNotifyBadMessage(MojoMessageHandle message,
   return g_thunks.NotifyBadMessage(message, error, error_num_bytes, options);
 }
 
-MojoResult MojoWrapPlatformHandle(
-    const struct MojoPlatformHandle* platform_handle,
-    MojoHandle* mojo_handle) {
-  return g_thunks.WrapPlatformHandle(platform_handle, mojo_handle);
+MojoResult MojoWrapPlatformHandle(const MojoPlatformHandle* platform_handle,
+                                  const MojoWrapPlatformHandleOptions* options,
+                                  MojoHandle* mojo_handle) {
+  return g_thunks.WrapPlatformHandle(platform_handle, options, mojo_handle);
 }
 
 MojoResult MojoUnwrapPlatformHandle(
     MojoHandle mojo_handle,
-    struct MojoPlatformHandle* platform_handle) {
-  return g_thunks.UnwrapPlatformHandle(mojo_handle, platform_handle);
-}
-
-MojoResult MojoWrapPlatformSharedBufferHandle(
-    const struct MojoPlatformHandle* platform_handle,
-    size_t num_bytes,
-    const struct MojoSharedBufferGuid* guid,
-    MojoPlatformSharedBufferHandleFlags flags,
+    const MojoUnwrapPlatformHandleOptions* options,
+    MojoPlatformHandle* platform_handle) {
+  return g_thunks.UnwrapPlatformHandle(mojo_handle, options, platform_handle);
+}
+
+MojoResult MojoWrapPlatformSharedMemoryRegion(
+    const struct MojoPlatformHandle* platform_handles,
+    uint32_t num_platform_handles,
+    uint64_t num_bytes,
+    const MojoSharedBufferGuid* guid,
+    MojoPlatformSharedMemoryRegionAccessMode access_mode,
+    const MojoWrapPlatformSharedMemoryRegionOptions* options,
     MojoHandle* mojo_handle) {
-  return g_thunks.WrapPlatformSharedBufferHandle(platform_handle, num_bytes,
-                                                 guid, flags, mojo_handle);
+  return g_thunks.WrapPlatformSharedMemoryRegion(
+      platform_handles, num_platform_handles, num_bytes, guid, access_mode,
+      options, mojo_handle);
 }
 
-MojoResult MojoUnwrapPlatformSharedBufferHandle(
+MojoResult MojoUnwrapPlatformSharedMemoryRegion(
     MojoHandle mojo_handle,
-    struct MojoPlatformHandle* platform_handle,
-    size_t* num_bytes,
+    const MojoUnwrapPlatformSharedMemoryRegionOptions* options,
+    struct MojoPlatformHandle* platform_handles,
+    uint32_t* num_platform_handles,
+    uint64_t* num_bytes,
     struct MojoSharedBufferGuid* guid,
-    MojoPlatformSharedBufferHandleFlags* flags) {
-  return g_thunks.UnwrapPlatformSharedBufferHandle(mojo_handle, platform_handle,
-                                                   num_bytes, guid, flags);
+    MojoPlatformSharedMemoryRegionAccessMode* access_mode) {
+  return g_thunks.UnwrapPlatformSharedMemoryRegion(
+      mojo_handle, options, platform_handles, num_platform_handles, num_bytes,
+      guid, access_mode);
 }
 
 }  // extern "C"

mojo/public/c/system/thunks.h

@@ -153,22 +153,28 @@ struct MojoSystemThunks {
   // Platform handle API.
   MojoResult (*WrapPlatformHandle)(
       const struct MojoPlatformHandle* platform_handle,
+      const struct MojoWrapPlatformHandleOptions* options,
       MojoHandle* mojo_handle);
   MojoResult (*UnwrapPlatformHandle)(
       MojoHandle mojo_handle,
+      const MojoUnwrapPlatformHandleOptions* options,
       struct MojoPlatformHandle* platform_handle);
-  MojoResult (*WrapPlatformSharedBufferHandle)(
-      const struct MojoPlatformHandle* platform_handle,
-      size_t num_bytes,
+  MojoResult (*WrapPlatformSharedMemoryRegion)(
+      const struct MojoPlatformHandle* platform_handles,
+      uint32_t num_platform_handles,
+      uint64_t num_bytes,
       const struct MojoSharedBufferGuid* guid,
-      MojoPlatformSharedBufferHandleFlags flags,
+      MojoPlatformSharedMemoryRegionAccessMode access_mode,
+      const struct MojoWrapPlatformSharedMemoryRegionOptions* options,
       MojoHandle* mojo_handle);
-  MojoResult (*UnwrapPlatformSharedBufferHandle)(
+  MojoResult (*UnwrapPlatformSharedMemoryRegion)(
       MojoHandle mojo_handle,
-      struct MojoPlatformHandle* platform_handle,
-      size_t* num_bytes,
+      const struct MojoUnwrapPlatformSharedMemoryRegionOptions* options,
+      struct MojoPlatformHandle* platform_handles,
+      uint32_t* num_platform_handles,
+      uint64_t* num_bytes,
       struct MojoSharedBufferGuid* guid,
-      MojoPlatformSharedBufferHandleFlags* flags);
+      MojoPlatformSharedMemoryRegionAccessMode* access_mode);
 };
 #pragma pack(pop)
 

mojo/public/cpp/base/shared_memory_mojom_traits.cc

@@ -35,7 +35,8 @@ StructTraits<mojo_base::mojom::PlatformSharedMemoryHandleDataView,
   platform_handle.value = static_cast<uint64_t>(handle.fd.release());
 #endif
   MojoHandle mojo_handle;
-  MojoResult result = MojoWrapPlatformHandle(&platform_handle, &mojo_handle);
+  MojoResult result =
+      MojoWrapPlatformHandle(&platform_handle, nullptr, &mojo_handle);
   if (result != MOJO_RESULT_OK)
     return mojo::ScopedHandle();
   return mojo::ScopedHandle(mojo::Handle(mojo_handle));
@@ -57,7 +58,8 @@ StructTraits<mojo_base::mojom::PlatformSharedMemoryHandleDataView,
   platform_handle.value = static_cast<uint64_t>(handle.readonly_fd.release());
 
   MojoHandle mojo_handle;
-  MojoResult result = MojoWrapPlatformHandle(&platform_handle, &mojo_handle);
+  MojoResult result =
+      MojoWrapPlatformHandle(&platform_handle, nullptr, &mojo_handle);
   if (result != MOJO_RESULT_OK)
     return mojo::ScopedHandle();
 
@@ -76,8 +78,8 @@ bool StructTraits<
 
   MojoPlatformHandle platform_handle;
   platform_handle.struct_size = sizeof(platform_handle);
-  MojoResult result =
-      MojoUnwrapPlatformHandle(mojo_handle.release().value(), &platform_handle);
+  MojoResult result = MojoUnwrapPlatformHandle(mojo_handle.release().value(),
+                                               nullptr, &platform_handle);
   if (result != MOJO_RESULT_OK)
     return false;
 
@@ -110,8 +112,9 @@ bool StructTraits<
   readonly_platform_handle.struct_size = sizeof(readonly_platform_handle);
   readonly_platform_handle.type = MOJO_PLATFORM_HANDLE_TYPE_INVALID;
   if (readonly_mojo_handle.is_valid()) {
-    MojoResult result = MojoUnwrapPlatformHandle(
-        readonly_mojo_handle.release().value(), &readonly_platform_handle);
+    MojoResult result =
+        MojoUnwrapPlatformHandle(readonly_mojo_handle.release().value(),
+                                 nullptr, &readonly_platform_handle);
     if (result != MOJO_RESULT_OK ||
         readonly_platform_handle.type !=
             MOJO_PLATFORM_HANDLE_TYPE_FILE_DESCRIPTOR) {

mojo/public/cpp/system/platform_handle.h

@@ -76,6 +76,7 @@ enum class UnwrappedSharedMemoryHandleProtection {
 };
 
 // Wraps a PlatformFile as a Mojo handle. Takes ownership of the file object.
+// If |platform_file| is valid, this will return a valid handle.
 MOJO_CPP_SYSTEM_EXPORT
 ScopedHandle WrapPlatformFile(base::PlatformFile platform_file);
 
@@ -83,6 +84,11 @@ ScopedHandle WrapPlatformFile(base::PlatformFile platform_file);
 MOJO_CPP_SYSTEM_EXPORT
 MojoResult UnwrapPlatformFile(ScopedHandle handle, base::PlatformFile* file);
 
+// DEPRECATED: Don't introduce new uses of base::SharedMemoryHandle, and please
+// attempt to avoid using this function. Use the new base shared memory APIs
+// (base::ReadOnlySharedMemoryRegion et al) and the corresponding wrap/unwrap
+// calls defined below instead.
+//
 // Wraps a base::SharedMemoryHandle as a Mojo handle. Takes ownership of the
 // SharedMemoryHandle. |size| indicates the size of the underlying
 // base::SharedMemory object, and |current_protection| indicates whether or
@@ -111,6 +117,11 @@ ScopedSharedBufferHandle WrapSharedMemoryHandle(
     size_t size,
     UnwrappedSharedMemoryHandleProtection current_protection);
 
+// DEPRECATED: Don't introduce new uses of base::SharedMemoryHandle, and please
+// attempt to avoid using this function. Use the new base shared memory APIs
+// (base::ReadOnlySharedMemoryRegion et al) and the corresponding wrap/unwrap
+// calls defined below instead.
+//
 // Unwraps a base::SharedMemoryHandle from a Mojo handle. The caller assumes
 // responsibility for the lifetime of the SharedMemoryHandle. On success,
 // |*memory_handle| is set to a valid handle, |*size| is is set to the size of
@@ -129,6 +140,8 @@ UnwrapSharedMemoryHandle(ScopedSharedBufferHandle handle,
                          UnwrappedSharedMemoryHandleProtection* protection);
 
 // Helpers for wrapping and unwrapping new base shared memory API primitives.
+// If the input |region| is valid for the Wrap* functions, they will always
+// succeed and return a valid Mojo shared buffer handle.
 
 MOJO_CPP_SYSTEM_EXPORT ScopedSharedBufferHandle
 WrapReadOnlySharedMemoryRegion(base::ReadOnlySharedMemoryRegion region);