Skip to content

Commit

Permalink
Update pepper to not assume that SharedMemoryHandle is an int.
Browse files Browse the repository at this point in the history
This CL is a refactor. This CL contains no intended behavior changes.

Pepper code assumes that SharedMemoryHandle is backed by a PlatformFile, and
that the relevant HANDLE or fd can be cast to an int. These assumptions will no
longer be true once SharedMemory is backed by Mach primitives on Mac.

This CL adds the method ShareSharedMemoryHandleWithRemote() to
ProxyChannel::Delegate. This method is used in place of ShareHandleWithRemote()
when a SharedMemory object is being shared between processes. This CL updates
the type of all SharedMemory handles to be SharedMemoryHandle.

BUG=466437

Review URL: https://codereview.chromium.org/1154613006

Cr-Commit-Position: refs/heads/master@{#332325}
  • Loading branch information
erikchen authored and Commit bot committed Jun 2, 2015
1 parent 4ed27c2 commit 4fc32d5
Show file tree
Hide file tree
Showing 47 changed files with 249 additions and 177 deletions.
12 changes: 2 additions & 10 deletions chrome/renderer/pepper/pepper_shared_memory_message_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,9 @@ void PepperSharedMemoryMessageFilter::OnHostMsgCreateSharedMemory(
->GetVarTracker()
->TrackSharedMemoryHandle(instance, host_shm_handle, size);

base::PlatformFile host_handle =
#if defined(OS_WIN)
host_shm_handle;
#elif defined(OS_POSIX)
host_shm_handle.fd;
#else
#error Not implemented.
#endif
// We set auto_close to false since we need our file descriptor to
// actually be duplicated on linux. The shared memory destructor will
// close the original handle for us.
plugin_handle->set_shmem(host_->ShareHandleWithRemote(host_handle, false),
size);
plugin_handle->set_shmem(
host_->ShareSharedMemoryHandleWithRemote(host_shm_handle), size);
}
11 changes: 0 additions & 11 deletions content/common/pepper_file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@ storage::FileSystemType PepperFileSystemTypeToFileSystemType(
}
}

base::PlatformFile PlatformFileFromSharedMemoryHandle(
const base::SharedMemoryHandle& shm_handle) {
#if defined(OS_WIN)
return shm_handle;
#elif defined(OS_POSIX)
return shm_handle.fd;
#else
#error Platform not supported.
#endif
}

int IntegerFromSyncSocketHandle(
const base::SyncSocket::Handle& socket_handle) {
#if defined(OS_WIN)
Expand Down
3 changes: 0 additions & 3 deletions content/common/pepper_file_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ namespace content {
storage::FileSystemType PepperFileSystemTypeToFileSystemType(
PP_FileSystemType type);

base::PlatformFile PlatformFileFromSharedMemoryHandle(
const base::SharedMemoryHandle& shm_handle);

int IntegerFromSyncSocketHandle(
const base::SyncSocket::Handle& socket_handle);

Expand Down
15 changes: 15 additions & 0 deletions content/ppapi_plugin/ppapi_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,21 @@ IPC::PlatformFileForTransit PpapiThread::ShareHandleWithRemote(
return BrokerGetFileHandleForProcess(handle, peer_pid, should_close_source);
}

base::SharedMemoryHandle PpapiThread::ShareSharedMemoryHandleWithRemote(
const base::SharedMemoryHandle& handle,
base::ProcessId remote_pid) {
base::PlatformFile local_platform_file =
#if defined(OS_POSIX)
handle.fd;
#elif defined(OS_WIN)
handle;
#else
#error Not implemented.
#endif
return PpapiThread::ShareHandleWithRemote(local_platform_file, remote_pid,
false);
}

std::set<PP_Instance>* PpapiThread::GetGloballySeenInstanceIDSet() {
return &globally_seen_instance_ids_;
}
Expand Down
3 changes: 3 additions & 0 deletions content/ppapi_plugin/ppapi_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ class PpapiThread : public ChildThreadImpl,
base::PlatformFile handle,
base::ProcessId peer_pid,
bool should_close_source) override;
base::SharedMemoryHandle ShareSharedMemoryHandleWithRemote(
const base::SharedMemoryHandle& handle,
base::ProcessId remote_pid) override;
uint32 Register(ppapi::proxy::PluginDispatcher* plugin_dispatcher) override;
void Unregister(uint32 plugin_dispatcher_id) override;

Expand Down
8 changes: 8 additions & 0 deletions content/public/renderer/renderer_ppapi_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/callback_forward.h"
#include "base/files/file.h"
#include "base/memory/ref_counted.h"
#include "base/memory/shared_memory.h"
#include "base/process/process.h"
#include "content/common/content_export.h"
#include "ipc/ipc_platform_file.h"
Expand Down Expand Up @@ -120,6 +121,13 @@ class RendererPpapiHost {
base::PlatformFile handle,
bool should_close_source) = 0;

// Shares a shared memory handle with the remote side. It
// returns a handle that should be sent in exactly one IPC message. Upon
// receipt, the remote side then owns that handle. Note: if sending the
// message fails, the returned handle is properly closed by the IPC system.
virtual base::SharedMemoryHandle ShareSharedMemoryHandleWithRemote(
const base::SharedMemoryHandle& handle) = 0;

// Returns true if the plugin is running in process.
virtual bool IsRunningInProcess() const = 0;

Expand Down
9 changes: 1 addition & 8 deletions content/renderer/npapi/webplugin_delegate_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -498,14 +498,7 @@ static void CopySharedMemoryHandleForMessage(
base::SharedMemoryHandle* handle_out,
base::ProcessId peer_pid) {
#if defined(OS_POSIX)
// On POSIX, base::ShardMemoryHandle is typedef'ed to FileDescriptor, and
// FileDescriptor message fields needs to remain valid until the message is
// sent or else the sendmsg() call will fail.
if ((handle_out->fd = HANDLE_EINTR(dup(handle_in.fd))) < 0) {
PLOG(ERROR) << "dup()";
return;
}
handle_out->auto_close = true;
*handle_out = base::SharedMemory::DeepCopyHandle(handle_in, true);
#elif defined(OS_WIN)
// On Windows we need to duplicate the handle for the plugin process.
*handle_out = NULL;
Expand Down
6 changes: 3 additions & 3 deletions content/renderer/pepper/audio_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ int32_t AudioHelper::GetSyncSocketImpl(int* sync_socket) {
return PP_ERROR_FAILED;
}

int32_t AudioHelper::GetSharedMemoryImpl(int* shm_handle, uint32_t* shm_size) {
int32_t AudioHelper::GetSharedMemoryImpl(base::SharedMemory** shm,
uint32_t* shm_size) {
if (shared_memory_for_create_callback_) {
*shm_handle = reinterpret_cast<int>(PlatformFileFromSharedMemoryHandle(
shared_memory_for_create_callback_->handle()));
*shm = shared_memory_for_create_callback_.get();
*shm_size = shared_memory_size_for_create_callback_;
return PP_OK;
}
Expand Down
2 changes: 1 addition & 1 deletion content/renderer/pepper/audio_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class AudioHelper {

// To be called by implementations of |PPB_Audio_API|/|PPB_AudioInput_API|.
int32_t GetSyncSocketImpl(int* sync_socket);
int32_t GetSharedMemoryImpl(int* shm_handle, uint32_t* shm_size);
int32_t GetSharedMemoryImpl(base::SharedMemory** shm, uint32_t* shm_size);

// To be implemented by subclasses to call their |SetStreamInfo()|.
virtual void OnSetStreamInfo(base::SharedMemoryHandle shared_memory_handle,
Expand Down
13 changes: 4 additions & 9 deletions content/renderer/pepper/host_array_buffer_var.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "content/common/sandbox_util.h"
#include "content/renderer/pepper/host_globals.h"
#include "content/renderer/pepper/plugin_module.h"
#include "content/renderer/pepper/renderer_ppapi_host_impl.h"
#include "content/renderer/render_thread_impl.h"
#include "ppapi/c/pp_instance.h"

Expand Down Expand Up @@ -74,16 +75,10 @@ bool HostArrayBufferVar::CopyToNewShmem(
// its handle on us.
HostGlobals* hg = HostGlobals::Get();
PluginModule* pm = hg->GetModule(hg->GetModuleForInstance(instance));
base::ProcessId p = pm->GetPeerProcessId();
if (p == base::kNullProcessId) {
// In-process, clone for ourselves.
p = base::GetCurrentProcId();
}

base::PlatformFile platform_file =
PlatformFileFromSharedMemoryHandle(shm->handle());

*plugin_shm_handle = BrokerGetFileHandleForProcess(platform_file, p, false);
*plugin_shm_handle =
pm->renderer_ppapi_host()->ShareSharedMemoryHandleWithRemote(
shm->handle());
*host_shm_handle_id = -1;
return true;
}
Expand Down
7 changes: 7 additions & 0 deletions content/renderer/pepper/mock_renderer_ppapi_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ IPC::PlatformFileForTransit MockRendererPpapiHost::ShareHandleWithRemote(
return IPC::InvalidPlatformFileForTransit();
}

base::SharedMemoryHandle
MockRendererPpapiHost::ShareSharedMemoryHandleWithRemote(
const base::SharedMemoryHandle& handle) {
NOTIMPLEMENTED();
return base::SharedMemory::NULLHandle();
}

bool MockRendererPpapiHost::IsRunningInProcess() const { return false; }

std::string MockRendererPpapiHost::GetPluginName() const {
Expand Down
2 changes: 2 additions & 0 deletions content/renderer/pepper/mock_renderer_ppapi_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class MockRendererPpapiHost : public RendererPpapiHost {
IPC::PlatformFileForTransit ShareHandleWithRemote(
base::PlatformFile handle,
bool should_close_source) override;
base::SharedMemoryHandle ShareSharedMemoryHandleWithRemote(
const base::SharedMemoryHandle& handle) override;
bool IsRunningInProcess() const override;
std::string GetPluginName() const override;
void SetToExternalPluginHost() override;
Expand Down
7 changes: 4 additions & 3 deletions content/renderer/pepper/pepper_audio_input_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,10 @@ int32_t PepperAudioInputHost::GetRemoteHandles(
if (*remote_socket_handle == IPC::InvalidPlatformFileForTransit())
return PP_ERROR_FAILED;

*remote_shared_memory_handle = renderer_ppapi_host_->ShareHandleWithRemote(
PlatformFileFromSharedMemoryHandle(shared_memory.handle()), false);
if (*remote_shared_memory_handle == IPC::InvalidPlatformFileForTransit())
*remote_shared_memory_handle =
renderer_ppapi_host_->ShareSharedMemoryHandleWithRemote(
shared_memory.handle());
if (!base::SharedMemory::IsHandleValid(*remote_shared_memory_handle))
return PP_ERROR_FAILED;

return PP_OK;
Expand Down
14 changes: 5 additions & 9 deletions content/renderer/pepper/pepper_compositor_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,20 @@ int32_t VerifyCommittedLayer(
return PP_ERROR_BADARGUMENT;
}

int handle;
base::SharedMemoryHandle handle;
uint32_t byte_count;
if (enter.object()->GetSharedMemory(&handle, &byte_count) != PP_OK)
return PP_ERROR_FAILED;

#if defined(OS_WIN)
base::SharedMemoryHandle shm_handle;
if (!::DuplicateHandle(::GetCurrentProcess(),
reinterpret_cast<base::SharedMemoryHandle>(handle),
::GetCurrentProcess(),
&shm_handle,
0,
FALSE,
DUPLICATE_SAME_ACCESS)) {
if (!::DuplicateHandle(::GetCurrentProcess(), handle, ::GetCurrentProcess(),
&shm_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
return PP_ERROR_FAILED;
}
#else
base::SharedMemoryHandle shm_handle(dup(handle), false);
base::SharedMemoryHandle shm_handle =
base::SharedMemory::DeepCopyHandle(handle, false);
#endif
image_shm->reset(new base::SharedMemory(shm_handle, true));
if (!(*image_shm)->Map(desc.stride * desc.size.height)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ bool PepperMediaStreamTrackHostBase::InitBuffers(int32_t number_of_buffers,
return false;
}

base::PlatformFile platform_file =
PlatformFileFromSharedMemoryHandle(shm_handle);
SerializedHandle handle(host_->ShareHandleWithRemote(platform_file, false),
SerializedHandle handle(host_->ShareSharedMemoryHandleWithRemote(shm_handle),
size.ValueOrDie());
bool readonly = (track_type == kRead);
host()->SendUnsolicitedReplyWithHandles(
Expand Down
2 changes: 1 addition & 1 deletion content/renderer/pepper/pepper_platform_audio_input.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ void PepperPlatformAudioInput::OnStreamCreated(
DCHECK(handle);
DCHECK(socket_handle);
#else
DCHECK_NE(-1, handle.fd);
DCHECK(base::SharedMemory::IsHandleValid(handle));
DCHECK_NE(-1, socket_handle);
#endif
DCHECK(length);
Expand Down
2 changes: 1 addition & 1 deletion content/renderer/pepper/pepper_platform_audio_output.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void PepperPlatformAudioOutput::OnStreamCreated(
DCHECK(handle);
DCHECK(socket_handle);
#else
DCHECK_NE(-1, handle.fd);
DCHECK(base::SharedMemory::IsHandleValid(handle));
DCHECK_NE(-1, socket_handle);
#endif
DCHECK(length);
Expand Down
15 changes: 15 additions & 0 deletions content/renderer/pepper/pepper_proxy_channel_delegate_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,19 @@ PepperProxyChannelDelegateImpl::ShareHandleWithRemote(
return BrokerGetFileHandleForProcess(handle, remote_pid, should_close_source);
}

base::SharedMemoryHandle
PepperProxyChannelDelegateImpl::ShareSharedMemoryHandleWithRemote(
const base::SharedMemoryHandle& handle,
base::ProcessId remote_pid) {
base::PlatformFile local_platform_file =
#if defined(OS_POSIX)
handle.fd;
#elif defined(OS_WIN)
handle;
#else
#error Not implemented.
#endif
return ShareHandleWithRemote(local_platform_file, remote_pid, false);
}

} // namespace content
3 changes: 3 additions & 0 deletions content/renderer/pepper/pepper_proxy_channel_delegate_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class PepperProxyChannelDelegateImpl
base::PlatformFile handle,
base::ProcessId remote_pid,
bool should_close_source) override;
base::SharedMemoryHandle ShareSharedMemoryHandleWithRemote(
const base::SharedMemoryHandle& handle,
base::ProcessId remote_pid) override;
};

} // namespace content
Expand Down
16 changes: 3 additions & 13 deletions content/renderer/pepper/pepper_video_capture_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,21 +220,11 @@ void PepperVideoCaptureHost::AllocBuffers(const gfx::Size& resolution,
{
EnterResourceNoLock<PPB_Buffer_API> enter(res, true);
DCHECK(enter.succeeded());
int handle;
int32_t result = enter.object()->GetSharedMemory(&handle);
base::SharedMemory* shm;
int32_t result = enter.object()->GetSharedMemory(&shm);
DCHECK(result == PP_OK);
// TODO(piman/brettw): Change trusted interface to return a PP_FileHandle,
// those casts are ugly.
base::PlatformFile platform_file =
#if defined(OS_WIN)
reinterpret_cast<HANDLE>(static_cast<intptr_t>(handle));
#elif defined(OS_POSIX)
handle;
#else
#error Not implemented.
#endif
params.AppendHandle(ppapi::proxy::SerializedHandle(
dispatcher->ShareHandleWithRemote(platform_file, false), size));
dispatcher->ShareSharedMemoryHandleWithRemote(shm->handle()), size));
}
}

Expand Down
4 changes: 1 addition & 3 deletions content/renderer/pepper/pepper_video_decoder_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,8 @@ int32_t PepperVideoDecoderHost::OnHostMsgGetShm(
shm_buffers_[shm_id] = shm.release();
}

base::PlatformFile platform_file =
PlatformFileFromSharedMemoryHandle(shm_handle);
SerializedHandle handle(
renderer_ppapi_host_->ShareHandleWithRemote(platform_file, false),
renderer_ppapi_host_->ShareSharedMemoryHandleWithRemote(shm_handle),
shm_size);
ppapi::host::ReplyMessageContext reply_context =
context->MakeReplyMessageContext();
Expand Down
14 changes: 6 additions & 8 deletions content/renderer/pepper/pepper_video_encoder_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,8 @@ void PepperVideoEncoderHost::RequireBitstreamBuffers(
for (size_t i = 0; i < shm_buffers_.size(); ++i) {
encoder_->UseOutputBitstreamBuffer(shm_buffers_[i]->ToBitstreamBuffer());
handles.push_back(SerializedHandle(
renderer_ppapi_host_->ShareHandleWithRemote(
PlatformFileFromSharedMemoryHandle(shm_buffers_[i]->shm->handle()),
false),
renderer_ppapi_host_->ShareSharedMemoryHandleWithRemote(
shm_buffers_[i]->shm->handle()),
output_buffer_size));
}

Expand Down Expand Up @@ -601,11 +600,10 @@ void PepperVideoEncoderHost::AllocateVideoFrames() {
}

DCHECK(get_video_frames_reply_context_.is_valid());
get_video_frames_reply_context_.params.AppendHandle(SerializedHandle(
renderer_ppapi_host_->ShareHandleWithRemote(
PlatformFileFromSharedMemoryHandle(buffer_manager_.shm()->handle()),
false),
total_size));
get_video_frames_reply_context_.params.AppendHandle(
SerializedHandle(renderer_ppapi_host_->ShareSharedMemoryHandleWithRemote(
buffer_manager_.shm()->handle()),
total_size));

host()->SendReply(get_video_frames_reply_context_,
PpapiPluginMsg_VideoEncoder_GetVideoFramesReply(
Expand Down
15 changes: 4 additions & 11 deletions content/renderer/pepper/pepper_video_source_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,15 @@ void PepperVideoSourceHost::SendGetFrameReply() {
// Note: We try to reuse the shared memory for the previous frame here. This
// means that the previous frame may be overwritten and is no longer valid
// after calling this function again.
IPC::PlatformFileForTransit image_handle;
base::SharedMemoryHandle image_handle;
uint32_t byte_count;
if (shared_image_.get() && dst_size.width() == shared_image_->width() &&
dst_size.height() == shared_image_->height()) {
// We have already allocated the correct size in shared memory. We need to
// duplicate the handle for IPC however, which will close down the
// duplicated handle when it's done.
int local_fd = 0;
if (shared_image_->GetSharedMemory(&local_fd, &byte_count) != PP_OK) {
base::SharedMemoryHandle local_handle;
if (shared_image_->GetSharedMemory(&local_handle, &byte_count) != PP_OK) {
SendGetFrameErrorReply(PP_ERROR_FAILED);
return;
}
Expand All @@ -140,14 +140,7 @@ void PepperVideoSourceHost::SendGetFrameReply() {
return;
}

#if defined(OS_WIN)
image_handle = dispatcher->ShareHandleWithRemote(
reinterpret_cast<HANDLE>(static_cast<intptr_t>(local_fd)), false);
#elif defined(OS_POSIX)
image_handle = dispatcher->ShareHandleWithRemote(local_fd, false);
#else
#error Not implemented.
#endif
image_handle = dispatcher->ShareSharedMemoryHandleWithRemote(local_handle);
} else {
// We need to allocate new shared memory.
shared_image_ = NULL; // Release any previous image.
Expand Down
Loading

0 comments on commit 4fc32d5

Please sign in to comment.