Replace base::SharedMemory read-only methods with GetReadOnlyHandle.

base/memory/shared_memory_mac.cc

Index: base/memory/shared_memory_mac.cc
diff --git a/base/memory/shared_memory_mac.cc b/base/memory/shared_memory_mac.cc
index 6acaee425ffa48c9bf0eda8ee0749ee21f8e51ca..28dbb4e9a146a7472a0773c52b9b94fe01ece1db 100644
--- a/base/memory/shared_memory_mac.cc
+++ b/base/memory/shared_memory_mac.cc
@@ -83,7 +83,6 @@ bool MakeMachSharedMemoryHandleReadOnly(SharedMemoryHandle* new_handle,
 
 SharedMemory::SharedMemory()
     : mapped_memory_mechanism_(SharedMemoryHandle::MACH),
-      readonly_mapped_file_(-1),
       mapped_size_(0),
       memory_(NULL),
       read_only_(false),
@@ -92,7 +91,6 @@ SharedMemory::SharedMemory()
 SharedMemory::SharedMemory(const SharedMemoryHandle& handle, bool read_only)
     : shm_(handle),
       mapped_memory_mechanism_(SharedMemoryHandle::POSIX),
-      readonly_mapped_file_(-1),
       mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
@@ -186,10 +184,12 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
   requested_size_ = options.size;
 
   int mapped_file = -1;
+  int readonly_mapped_file = -1;
   result = PrepareMapFile(std::move(fp), std::move(readonly_fd), &mapped_file,
-                          &readonly_mapped_file_);
-
+                          &readonly_mapped_file);
   shm_ = SharedMemoryHandle(FileDescriptor(mapped_file, false));
+  readonly_shm_ =
+      SharedMemoryHandle(FileDescriptor(readonly_mapped_file, false));
   return result;
 }
 
@@ -254,65 +254,42 @@ void SharedMemory::Close() {
   shm_.Close();
   shm_ = SharedMemoryHandle();
   if (shm_.type_ == SharedMemoryHandle::POSIX) {
-    if (readonly_mapped_file_ > 0) {
-      if (IGNORE_EINTR(close(readonly_mapped_file_)) < 0)
-        PLOG(ERROR) << "close";
-      readonly_mapped_file_ = -1;
+    if (readonly_shm_.IsValid()) {
+      readonly_shm_.Close();
+      readonly_shm_ = SharedMemoryHandle();
     }
   }
 }
 
-bool SharedMemory::Share(SharedMemoryHandle* new_handle, ShareMode share_mode) {
-  if (shm_.type_ == SharedMemoryHandle::MACH) {
-    DCHECK(shm_.IsValid());
-
-    bool success = false;
-    switch (share_mode) {
-      case SHARE_CURRENT_MODE:
-        *new_handle = shm_.Duplicate();
-        success = true;
-        break;
-      case SHARE_READONLY:
-        success = MakeMachSharedMemoryHandleReadOnly(new_handle, shm_, memory_);
-        break;
-    }
-
-    if (success)
-      new_handle->SetOwnershipPassesToIPC(true);
-
-    return success;
+SharedMemoryHandle SharedMemory::GetReadOnlyHandle() {
+  if (shm_.type_ == SharedMemoryHandle::POSIX) {
+    CHECK(readonly_shm_.IsValid());
+    return readonly_shm_.Duplicate();
   }
 
-  int handle_to_dup = -1;
-  switch (share_mode) {
-    case SHARE_CURRENT_MODE:
-      handle_to_dup = shm_.file_descriptor_.fd;
-      break;
-    case SHARE_READONLY:
-      // We could imagine re-opening the file from /dev/fd, but that can't make

[Jeffrey Yasskin, 2017/05/01 22:16:34]

This comment should probably survive the refactoring.

[erikchen, 2017/05/02 01:53:08]

Done.

-      // it readonly on Mac: https://codereview.chromium.org/27265002/#msg10
-      CHECK_GE(readonly_mapped_file_, 0);
-      handle_to_dup = readonly_mapped_file_;
-      break;
-  }
+  DCHECK(shm_.IsValid());
+  base::SharedMemoryHandle new_handle;
+  bool success = MakeMachSharedMemoryHandleReadOnly(&new_handle, shm_, memory_);
+  if (success)
+    new_handle.SetOwnershipPassesToIPC(true);
+  return new_handle;
+}
 
-  const int new_fd = HANDLE_EINTR(dup(handle_to_dup));
-  if (new_fd < 0) {
-    DPLOG(ERROR) << "dup() failed.";
-    return false;
+bool SharedMemory::Share(SharedMemoryHandle* new_handle) {
+  if (shm_.type_ == SharedMemoryHandle::MACH) {
+    DCHECK(shm_.IsValid());
+    *new_handle = shm_.Duplicate();
+    return true;

[Jeffrey Yasskin, 2017/05/01 22:16:34]

return new_handle->IsValid()?

[erikchen, 2017/05/02 01:53:08]

Done.

   }
 
-  new_handle->file_descriptor_.fd = new_fd;
-  new_handle->type_ = SharedMemoryHandle::POSIX;
-
-  return true;
+  *new_handle = shm_.Duplicate();
+  return new_handle->IsValid();
 }
 
 bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
                                         SharedMemoryHandle* new_handle,
-                                        bool close_self,
-                                        ShareMode share_mode) {
-  bool success = Share(new_handle, share_mode);
+                                        bool close_self) {
+  bool success = Share(new_handle);
   if (close_self) {
     Unmap();
     Close();