Pepper: remove the SHARED_MEMORY SerializedHandle type.

With the migration of the rest of PPAPI away from the legacy shared memory API, the SHARED_MEMORY kind of SerializedHandle is no longer needed and is removed. Bug: 795291 Change-Id: I547b3b88825da0311ea4b684150d049a1f52768d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724512 Reviewed-by: Bill Budge <bbudge@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Derek Schuff <dschuff@chromium.org> Commit-Queue: Matthew Cary (CET) <mattcary@chromium.org> Cr-Commit-Position: refs/heads/master@{#685022}
marcosholgado · Aug 7, 2019 · 36e2ec8 · 36e2ec8
1 parent 2f10e9c
commit 36e2ec8
Show file tree

Hide file tree

Showing 7 changed files with 5 additions and 146 deletions.
diff --git a/components/nacl/loader/nacl_ipc_adapter.cc b/components/nacl/loader/nacl_ipc_adapter.cc
@@ -236,24 +236,6 @@ class NaClDescWrapper {
   DISALLOW_COPY_AND_ASSIGN(NaClDescWrapper);
 };
 
-std::unique_ptr<NaClDescWrapper> MakeShmNaClDesc(
-    const base::SharedMemoryHandle& handle,
-    size_t size) {
-#if defined(OS_MACOSX)
-  return std::unique_ptr<NaClDescWrapper>(new NaClDescWrapper(
-      NaClDescImcShmMachMake(handle.GetMemoryObject(), size)));
-#else
-  return std::unique_ptr<NaClDescWrapper>(
-      new NaClDescWrapper(NaClDescImcShmMake(
-#if defined(OS_WIN)
-          handle.GetHandle(),
-#else
-          base::SharedMemory::GetFdFromSharedMemoryHandle(handle),
-#endif
-          size)));
-#endif
-}
-
 std::unique_ptr<NaClDescWrapper> MakeShmRegionNaClDesc(
     base::subtle::PlatformSharedMemoryRegion region) {
   // Writable regions are not supported in NaCl.
@@ -578,11 +560,6 @@ bool NaClIPCAdapter::RewriteMessage(const IPC::Message& msg, uint32_t type) {
     for (ppapi::proxy::SerializedHandle& handle : handles) {
       std::unique_ptr<NaClDescWrapper> nacl_desc;
       switch (handle.type()) {
-        case ppapi::proxy::SerializedHandle::SHARED_MEMORY: {
-          nacl_desc = MakeShmNaClDesc(handle.shmem(),
-                                      static_cast<size_t>(handle.size()));
-          break;
-        }
         case ppapi::proxy::SerializedHandle::SHARED_MEMORY_REGION: {
           nacl_desc = MakeShmRegionNaClDesc(handle.TakeSharedMemoryRegion());
           break;

diff --git a/ppapi/proxy/nacl_message_scanner.cc b/ppapi/proxy/nacl_message_scanner.cc
@@ -59,19 +59,7 @@ void WriteHandle(int handle_index,
                  base::Pickle* msg) {
   SerializedHandle::WriteHeader(handle.header(), msg);
 
-  if (handle.type() == SerializedHandle::SHARED_MEMORY) {
-    // Now write the handle itself in POSIX style.
-    // This serialization must be kept in sync with
-    // ParamTraits<SharedMemoryHandle>::Write.
-    if (handle.shmem().IsValid()) {
-      msg->WriteBool(true);  // valid == true
-      msg->WriteInt(handle_index);
-      IPC::WriteParam(msg, handle.shmem().GetGUID());
-      msg->WriteUInt64(handle.shmem().GetSize());
-    } else {
-      msg->WriteBool(false);  // valid == false
-    }
-  } else if (handle.type() == SerializedHandle::SHARED_MEMORY_REGION) {
+  if (handle.type() == SerializedHandle::SHARED_MEMORY_REGION) {
     // Write the region in POSIX style.
     // This serialization must be kept in sync with
     // ParamTraits<PlatformSharedMemoryRegion>::Write.

diff --git a/ppapi/proxy/ppapi_param_traits.cc b/ppapi/proxy/ppapi_param_traits.cc
@@ -235,9 +235,6 @@ void ParamTraits<ppapi::proxy::SerializedHandle>::Write(base::Pickle* m,
                                                         const param_type& p) {
   ppapi::proxy::SerializedHandle::WriteHeader(p.header(), m);
   switch (p.type()) {
-    case ppapi::proxy::SerializedHandle::SHARED_MEMORY:
-      WriteParam(m, p.shmem());
-      break;
     case ppapi::proxy::SerializedHandle::SHARED_MEMORY_REGION:
       WriteParam(m, const_cast<param_type&>(p).TakeSharedMemoryRegion());
       break;
@@ -260,13 +257,6 @@ bool ParamTraits<ppapi::proxy::SerializedHandle>::Read(
   if (!ppapi::proxy::SerializedHandle::ReadHeader(iter, &header))
     return false;
   switch (header.type) {
-    case ppapi::proxy::SerializedHandle::SHARED_MEMORY: {
-      base::SharedMemoryHandle handle;
-      if (!ReadParam(m, iter, &handle))
-        return false;
-      r->set_shmem(handle, header.size);
-      break;
-    }
     case ppapi::proxy::SerializedHandle::SHARED_MEMORY_REGION: {
       base::subtle::PlatformSharedMemoryRegion region;
       if (!ReadParam(m, iter, &region))

diff --git a/ppapi/proxy/resource_message_params.cc b/ppapi/proxy/resource_message_params.cc
@@ -90,17 +90,6 @@ SerializedHandle ResourceMessageParams::TakeHandleOfTypeAtIndex(
   return handle;
 }
 
-bool ResourceMessageParams::TakeSharedMemoryHandleAtIndex(
-    size_t index,
-    base::SharedMemoryHandle* handle) const {
-  SerializedHandle serialized = TakeHandleOfTypeAtIndex(
-      index, SerializedHandle::SHARED_MEMORY);
-  if (!serialized.is_shmem())
-    return false;
-  *handle = serialized.shmem();
-  return true;
-}
-
 bool ResourceMessageParams::TakeReadOnlySharedMemoryRegionAtIndex(
     size_t index,
     base::ReadOnlySharedMemoryRegion* region) const {
@@ -147,15 +136,6 @@ bool ResourceMessageParams::TakeFileHandleAtIndex(
   return true;
 }
 
-void ResourceMessageParams::TakeAllSharedMemoryHandles(
-    std::vector<base::SharedMemoryHandle>* handles) const {
-  for (size_t i = 0; i < handles_->data().size(); ++i) {
-    base::SharedMemoryHandle handle;
-    if (TakeSharedMemoryHandleAtIndex(i, &handle))
-      handles->push_back(handle);
-  }
-}
-
 void ResourceMessageParams::TakeAllHandles(
     std::vector<SerializedHandle>* handles) const {
   std::vector<SerializedHandle>& data = handles_->data();

diff --git a/ppapi/proxy/resource_message_params.h b/ppapi/proxy/resource_message_params.h
@@ -62,8 +62,6 @@ class PPAPI_PROXY_EXPORT ResourceMessageParams {
   //          type and the functions will succeed.
   //       2) the caller is responsible for closing the returned handle, if it
   //          is valid.
-  bool TakeSharedMemoryHandleAtIndex(size_t index,
-                                     base::SharedMemoryHandle* handle) const;
   bool TakeReadOnlySharedMemoryRegionAtIndex(
       size_t index,
       base::ReadOnlySharedMemoryRegion* region) const;
@@ -74,8 +72,6 @@ class PPAPI_PROXY_EXPORT ResourceMessageParams {
                                IPC::PlatformFileForTransit* handle) const;
   bool TakeFileHandleAtIndex(size_t index,
                              IPC::PlatformFileForTransit* handle) const;
-  void TakeAllSharedMemoryHandles(
-      std::vector<base::SharedMemoryHandle>* handles) const;
   void TakeAllHandles(std::vector<SerializedHandle>* handles) const;
 
   // Appends the given handle to the list of handles sent with the call or

diff --git a/ppapi/proxy/serialized_handle.cc b/ppapi/proxy/serialized_handle.cc
@@ -5,7 +5,6 @@
 #include "ppapi/proxy/serialized_handle.h"
 
 #include "base/files/file.h"
-#include "base/memory/shared_memory.h"
 #include "base/pickle.h"
 #include "build/build_config.h"
 #include "ipc/ipc_platform_file.h"
@@ -19,16 +18,13 @@ namespace proxy {
 
 SerializedHandle::SerializedHandle()
     : type_(INVALID),
-      size_(0),
       descriptor_(IPC::InvalidPlatformFileForTransit()),
       open_flags_(0),
       file_io_(0) {
 }
 
 SerializedHandle::SerializedHandle(SerializedHandle&& other)
     : type_(other.type_),
-      shm_handle_(other.shm_handle_),
-      size_(other.size_),
       shm_region_(std::move(other.shm_region_)),
       descriptor_(other.descriptor_),
       open_flags_(other.open_flags_),
@@ -39,8 +35,6 @@ SerializedHandle::SerializedHandle(SerializedHandle&& other)
 SerializedHandle& SerializedHandle::operator=(SerializedHandle&& other) {
   Close();
   type_ = other.type_;
-  shm_handle_ = other.shm_handle_;
-  size_ = other.size_;
   shm_region_ = std::move(other.shm_region_);
   descriptor_ = other.descriptor_;
   open_flags_ = other.open_flags_;
@@ -51,21 +45,11 @@ SerializedHandle& SerializedHandle::operator=(SerializedHandle&& other) {
 
 SerializedHandle::SerializedHandle(Type type_param)
     : type_(type_param),
-      size_(0),
       descriptor_(IPC::InvalidPlatformFileForTransit()),
       open_flags_(0),
       file_io_(0) {
 }
 
-SerializedHandle::SerializedHandle(const base::SharedMemoryHandle& handle,
-                                   uint32_t size)
-    : type_(SHARED_MEMORY),
-      shm_handle_(handle),
-      size_(size),
-      descriptor_(IPC::InvalidPlatformFileForTransit()),
-      open_flags_(0),
-      file_io_(0) {}
-
 SerializedHandle::SerializedHandle(base::ReadOnlySharedMemoryRegion region)
     : SerializedHandle(
           base::ReadOnlySharedMemoryRegion::TakeHandleForSerialization(
@@ -79,7 +63,6 @@ SerializedHandle::SerializedHandle(base::UnsafeSharedMemoryRegion region)
 SerializedHandle::SerializedHandle(
     base::subtle::PlatformSharedMemoryRegion region)
     : type_(SHARED_MEMORY_REGION),
-      size_(0),
       shm_region_(std::move(region)),
       descriptor_(IPC::InvalidPlatformFileForTransit()),
       open_flags_(0),
@@ -93,16 +76,13 @@ SerializedHandle::SerializedHandle(
     Type type,
     const IPC::PlatformFileForTransit& socket_descriptor)
     : type_(type),
-      size_(0),
       descriptor_(socket_descriptor),
       open_flags_(0),
       file_io_(0) {
 }
 
 bool SerializedHandle::IsHandleValid() const {
   switch (type_) {
-    case SHARED_MEMORY:
-      return base::SharedMemory::IsHandleValid(shm_handle_);
     case SHARED_MEMORY_REGION:
       return shm_region_.IsValid();
     case SOCKET:
@@ -121,9 +101,6 @@ void SerializedHandle::Close() {
       case INVALID:
         NOTREACHED();
         break;
-      case SHARED_MEMORY:
-        base::SharedMemory::CloseHandle(shm_handle_);
-        break;
       case SHARED_MEMORY_REGION:
         shm_region_ = base::subtle::PlatformSharedMemoryRegion();
         break;
@@ -140,30 +117,20 @@ void SerializedHandle::Close() {
 // static
 void SerializedHandle::WriteHeader(const Header& hdr, base::Pickle* pickle) {
   pickle->WriteInt(hdr.type);
-  if (hdr.type == SHARED_MEMORY) {
-    pickle->WriteUInt32(hdr.size);
-  } else if (hdr.type == FILE) {
+  if (hdr.type == FILE) {
     pickle->WriteInt(hdr.open_flags);
     pickle->WriteInt(hdr.file_io);
   }
 }
 
 // static
 bool SerializedHandle::ReadHeader(base::PickleIterator* iter, Header* hdr) {
-  *hdr = Header(INVALID, 0, 0, 0);
+  *hdr = Header(INVALID, 0, 0);
   int type = 0;
   if (!iter->ReadInt(&type))
     return false;
   bool valid_type = false;
   switch (type) {
-    case SHARED_MEMORY: {
-      uint32_t size = 0;
-      if (!iter->ReadUInt32(&size))
-        return false;
-      hdr->size = size;
-      valid_type = true;
-      break;
-    }
     case FILE: {
       int open_flags = 0;
       PP_Resource file_io = 0;

diff --git a/ppapi/proxy/serialized_handle.h b/ppapi/proxy/serialized_handle.h
@@ -15,7 +15,6 @@
 #include "base/memory/platform_shared_memory_region.h"
 #include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/shared_memory.h"
 #include "base/memory/unsafe_shared_memory_region.h"
 #include "build/build_config.h"
 #include "ipc/ipc_platform_file.h"
@@ -35,19 +34,15 @@ namespace proxy {
 // NaClIPCAdapter for use in NaCl.
 class PPAPI_PROXY_EXPORT SerializedHandle {
  public:
-  // TODO(https://crbug.com/845985): Remove SHARED_MEMORY type after all clients
-  // will be converted to the SHARED_MEMORY_REGION.
-  enum Type { INVALID, SHARED_MEMORY, SHARED_MEMORY_REGION, SOCKET, FILE };
+  enum Type { INVALID, SHARED_MEMORY_REGION, SOCKET, FILE };
   // Header contains the fields that we send in IPC messages, apart from the
   // actual handle. See comments on the SerializedHandle fields below.
   struct Header {
     Header() : type(INVALID), size(0), open_flags(0) {}
     Header(Type type_arg,
-           uint32_t size_arg,
            int32_t open_flags_arg,
            PP_Resource file_io_arg)
         : type(type_arg),
-          size(size_arg),
           open_flags(open_flags_arg),
           file_io(file_io_arg) {}
 
@@ -64,9 +59,6 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
   // Create an invalid handle of the given type.
   explicit SerializedHandle(Type type);
 
-  // Create a shared memory handle.
-  SerializedHandle(const base::SharedMemoryHandle& handle, uint32_t size);
-
   // Create a shared memory region handle.
   explicit SerializedHandle(base::ReadOnlySharedMemoryRegion region);
   explicit SerializedHandle(base::UnsafeSharedMemoryRegion region);
@@ -77,18 +69,9 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
                    const IPC::PlatformFileForTransit& descriptor);
 
   Type type() const { return type_; }
-  bool is_shmem() const { return type_ == SHARED_MEMORY; }
   bool is_shmem_region() const { return type_ == SHARED_MEMORY_REGION; }
   bool is_socket() const { return type_ == SOCKET; }
   bool is_file() const { return type_ == FILE; }
-  const base::SharedMemoryHandle& shmem() const {
-    DCHECK(is_shmem());
-    return shm_handle_;
-  }
-  uint32_t size() const {
-    DCHECK(is_shmem());
-    return size_;
-  }
   const base::subtle::PlatformSharedMemoryRegion& shmem_region() const {
     DCHECK(is_shmem_region());
     return shm_region_;
@@ -105,14 +88,6 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
   PP_Resource file_io() const {
     return file_io_;
   }
-  void set_shmem(const base::SharedMemoryHandle& handle, uint32_t size) {
-    type_ = SHARED_MEMORY;
-    shm_handle_ = handle;
-    size_ = size;
-
-    descriptor_ = IPC::InvalidPlatformFileForTransit();
-    shm_region_ = base::subtle::PlatformSharedMemoryRegion();
-  }
   void set_shmem_region(base::subtle::PlatformSharedMemoryRegion region) {
     type_ = SHARED_MEMORY_REGION;
     shm_region_ = std::move(region);
@@ -121,8 +96,6 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
               base::subtle::PlatformSharedMemoryRegion::Mode::kWritable);
 
     descriptor_ = IPC::InvalidPlatformFileForTransit();
-    shm_handle_ = base::SharedMemoryHandle();
-    size_ = 0;
   }
   void set_unsafe_shmem_region(base::UnsafeSharedMemoryRegion region) {
     set_shmem_region(base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
@@ -133,8 +106,6 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
     descriptor_ = socket;
 
     shm_region_ = base::subtle::PlatformSharedMemoryRegion();
-    shm_handle_ = base::SharedMemoryHandle();
-    size_ = 0;
   }
   void set_file_handle(const IPC::PlatformFileForTransit& descriptor,
                        int32_t open_flags,
@@ -143,20 +114,15 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
 
     descriptor_ = descriptor;
     shm_region_ = base::subtle::PlatformSharedMemoryRegion();
-    shm_handle_ = base::SharedMemoryHandle();
-    size_ = 0;
     open_flags_ = open_flags;
     file_io_ = file_io;
   }
   void set_null() {
     type_ = INVALID;
 
-    shm_handle_ = base::SharedMemoryHandle();
     shm_region_ = base::subtle::PlatformSharedMemoryRegion();
-    size_ = 0;
     descriptor_ = IPC::InvalidPlatformFileForTransit();
   }
-  void set_null_shmem() { set_shmem(base::SharedMemoryHandle(), 0); }
   void set_null_shmem_region() {
     set_shmem_region(base::subtle::PlatformSharedMemoryRegion());
   }
@@ -168,9 +134,7 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
   }
   bool IsHandleValid() const;
 
-  Header header() const {
-    return Header(type_, size_, open_flags_, file_io_);
-  }
+  Header header() const { return Header(type_, open_flags_, file_io_); }
 
   // Closes the handle and sets it to invalid.
   void Close();
@@ -189,9 +153,6 @@ class PPAPI_PROXY_EXPORT SerializedHandle {
   // because we hold non-POD types. But these types are pretty light-weight. If
   // we add more complex things later, we should come up with a more memory-
   // efficient strategy.
-  // These are valid if type == SHARED_MEMORY.
-  base::SharedMemoryHandle shm_handle_;
-  uint32_t size_;
 
   // This is valid if type == SHARED_MEMORY_REGION.
   base::subtle::PlatformSharedMemoryRegion shm_region_;