From 36e2ec8625a2fcb5c3166933f49c120b6f2fb6ba Mon Sep 17 00:00:00 2001 From: Matthew Cary Date: Wed, 7 Aug 2019 23:58:56 +0000 Subject: [PATCH] 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 Reviewed-by: Robert Sesek Reviewed-by: Derek Schuff Commit-Queue: Matthew Cary (CET) Cr-Commit-Position: refs/heads/master@{#685022} --- components/nacl/loader/nacl_ipc_adapter.cc | 23 ------------ ppapi/proxy/nacl_message_scanner.cc | 14 +------ ppapi/proxy/ppapi_param_traits.cc | 10 ----- ppapi/proxy/resource_message_params.cc | 20 ---------- ppapi/proxy/resource_message_params.h | 4 -- ppapi/proxy/serialized_handle.cc | 37 +------------------ ppapi/proxy/serialized_handle.h | 43 +--------------------- 7 files changed, 5 insertions(+), 146 deletions(-) diff --git a/components/nacl/loader/nacl_ipc_adapter.cc b/components/nacl/loader/nacl_ipc_adapter.cc index 15d8d128c89017..1e259afb13b017 100644 --- 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 MakeShmNaClDesc( - const base::SharedMemoryHandle& handle, - size_t size) { -#if defined(OS_MACOSX) - return std::unique_ptr(new NaClDescWrapper( - NaClDescImcShmMachMake(handle.GetMemoryObject(), size))); -#else - return std::unique_ptr( - new NaClDescWrapper(NaClDescImcShmMake( -#if defined(OS_WIN) - handle.GetHandle(), -#else - base::SharedMemory::GetFdFromSharedMemoryHandle(handle), -#endif - size))); -#endif -} - std::unique_ptr 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 nacl_desc; switch (handle.type()) { - case ppapi::proxy::SerializedHandle::SHARED_MEMORY: { - nacl_desc = MakeShmNaClDesc(handle.shmem(), - static_cast(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 index 0e478c2214962a..a6e2092c4b5b69 100644 --- 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::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::Write. diff --git a/ppapi/proxy/ppapi_param_traits.cc b/ppapi/proxy/ppapi_param_traits.cc index b663e8200b9310..f9310b73597487 100644 --- a/ppapi/proxy/ppapi_param_traits.cc +++ b/ppapi/proxy/ppapi_param_traits.cc @@ -235,9 +235,6 @@ void ParamTraits::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(p).TakeSharedMemoryRegion()); break; @@ -260,13 +257,6 @@ bool ParamTraits::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, ®ion)) diff --git a/ppapi/proxy/resource_message_params.cc b/ppapi/proxy/resource_message_params.cc index fa4df1073483c9..82e0d35bc5f882 100644 --- 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* 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* handles) const { std::vector& data = handles_->data(); diff --git a/ppapi/proxy/resource_message_params.h b/ppapi/proxy/resource_message_params.h index db3fefc86fbbef..0102e4d6ad27dd 100644 --- 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* handles) const; void TakeAllHandles(std::vector* 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 index ee06d0a325f8ef..1321651c27c776 100644 --- 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,7 +18,6 @@ namespace proxy { SerializedHandle::SerializedHandle() : type_(INVALID), - size_(0), descriptor_(IPC::InvalidPlatformFileForTransit()), open_flags_(0), file_io_(0) { @@ -27,8 +25,6 @@ SerializedHandle::SerializedHandle() 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,7 +76,6 @@ SerializedHandle::SerializedHandle( Type type, const IPC::PlatformFileForTransit& socket_descriptor) : type_(type), - size_(0), descriptor_(socket_descriptor), open_flags_(0), file_io_(0) { @@ -101,8 +83,6 @@ SerializedHandle::SerializedHandle( 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,9 +117,7 @@ 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); } @@ -150,20 +125,12 @@ void SerializedHandle::WriteHeader(const Header& hdr, base::Pickle* pickle) { // 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 index 24a18b5f5ce889..92370446772da2 100644 --- 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_;