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

base/memory/shared_memory_mac.cc

 // Copyright (c) 2012 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/memory/shared_memory.h"
 
 #include <errno.h>
 #include <mach/mach_vm.h>
 #include <stddef.h>
 #include <sys/mman.h>
@@ skipping 65 matching lines @@
 
   *new_handle = SharedMemoryHandle(named_right, size, base::GetCurrentProcId());
   return true;
 }
 
 
 }  // namespace
 
 SharedMemory::SharedMemory()
     : mapped_memory_mechanism_(SharedMemoryHandle::MACH),
-      readonly_mapped_file_(-1),
       mapped_size_(0),
       memory_(NULL),
       read_only_(false),
       requested_size_(0) {}
 
 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),
       requested_size_(0) {}
 
 SharedMemory::~SharedMemory() {
   Unmap();
   Close();
 }
 
@@ skipping 73 matching lines @@
   if (fstat(fileno(fp.get()), &stat) != 0)
     return false;
   const size_t current_size = stat.st_size;
   if (current_size != options.size) {
     if (HANDLE_EINTR(ftruncate(fileno(fp.get()), options.size)) != 0)
       return false;
   }
   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;
 }
 
 bool SharedMemory::MapAt(off_t offset, size_t bytes) {
   if (!shm_.IsValid())
     return false;
   if (bytes > static_cast<size_t>(std::numeric_limits<int>::max()))
     return false;
   if (memory_)
     return false;
@@ skipping 44 matching lines @@
 SharedMemoryHandle SharedMemory::TakeHandle() {
   SharedMemoryHandle dup = DuplicateHandle(handle());
   Close();
   return dup;
 }
 
 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) {
+SharedMemoryHandle SharedMemory::GetReadOnlyHandle() {
+  if (shm_.type_ == SharedMemoryHandle::POSIX) {
+    CHECK(readonly_shm_.IsValid());
+    return readonly_shm_.Duplicate();
+  }
+
+  DCHECK(shm_.IsValid());
+  base::SharedMemoryHandle new_handle;
+  bool success = MakeMachSharedMemoryHandleReadOnly(&new_handle, shm_, memory_);
+  if (success)
+    new_handle.SetOwnershipPassesToIPC(true);
+  return new_handle;
+}
+
+bool SharedMemory::Share(SharedMemoryHandle* new_handle) {
   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;
+    *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.

   }
 
-  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;
-  }
-
-  const int new_fd = HANDLE_EINTR(dup(handle_to_dup));
-  if (new_fd < 0) {
-    DPLOG(ERROR) << "dup() failed.";
-    return false;
-  }
-
-  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();
   }
   return success;
 }
 
 }  // namespace base