Further clean up pepper's use of SharedMemory.

This CL uses the new methods content::BrokerDuplicateSharedMemoryHandle() and base::SharedMemory::DuplicateHandle() to remove some duplicated code. This CL changes the implementation of ShareSharedMemoryhandleWithRemote() to no longer assume that the SharedMemoryHandle is backed by a POSIX fd. BUG=466437 Review URL: https://codereview.chromium.org/1166443004 Cr-Commit-Position: refs/heads/master@{#332903}
Denger-Network · Jun 4, 2015 · fc29ad9 · fc29ad9
1 parent a7f6fc4
commit fc29ad9
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 49 deletions.
diff --git a/content/ppapi_plugin/ppapi_thread.cc b/content/ppapi_plugin/ppapi_thread.cc
@@ -202,16 +202,24 @@ IPC::PlatformFileForTransit PpapiThread::ShareHandleWithRemote(
 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.
+#if defined(OS_WIN)
+  if (peer_handle_.IsValid()) {
+    DCHECK(is_broker_);
+    return IPC::GetFileHandleForProcess(handle, peer_handle_.Get(), false);
+  }
 #endif
-  return PpapiThread::ShareHandleWithRemote(local_platform_file, remote_pid,
-                                            false);
+
+  DCHECK(remote_pid != base::kNullProcessId);
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  base::SharedMemoryHandle duped_handle;
+  bool success =
+      BrokerDuplicateSharedMemoryHandle(handle, remote_pid, &duped_handle);
+  if (success)
+    return duped_handle;
+  return base::SharedMemory::NULLHandle();
+#else
+  return base::SharedMemory::DuplicateHandle(handle);
+#endif  // defined(OS_WIN) || defined(OS_MACOSX)
 }
 
 std::set<PP_Instance>* PpapiThread::GetGloballySeenInstanceIDSet() {

diff --git a/content/renderer/pepper/pepper_proxy_channel_delegate_impl.cc b/content/renderer/pepper/pepper_proxy_channel_delegate_impl.cc
@@ -7,6 +7,10 @@
 #include "content/child/child_process.h"
 #include "content/common/sandbox_util.h"
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+#include "content/public/common/sandbox_init.h"
+#endif  // defined(OS_WIN) || defined(OS_MACOSX)
+
 namespace content {
 
 PepperProxyChannelDelegateImpl::~PepperProxyChannelDelegateImpl() {}
@@ -35,15 +39,16 @@ 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;
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  base::SharedMemoryHandle duped_handle;
+  bool success =
+      BrokerDuplicateSharedMemoryHandle(handle, remote_pid, &duped_handle);
+  if (success)
+    return duped_handle;
+  return base::SharedMemory::NULLHandle();
 #else
-#error Not implemented.
-#endif
-  return ShareHandleWithRemote(local_platform_file, remote_pid, false);
+  return base::SharedMemory::DuplicateHandle(handle);
+#endif  // defined(OS_WIN) || defined(OS_MACOSX)
 }
 
 }  // namespace content
diff --git a/content/renderer/pepper/renderer_ppapi_host_impl.cc b/content/renderer/pepper/renderer_ppapi_host_impl.cc
@@ -234,15 +234,11 @@ IPC::PlatformFileForTransit RendererPpapiHostImpl::ShareHandleWithRemote(
 base::SharedMemoryHandle
 RendererPpapiHostImpl::ShareSharedMemoryHandleWithRemote(
     const base::SharedMemoryHandle& handle) {
-  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, false);
+  if (!dispatcher_) {
+    DCHECK(is_running_in_process_);
+    return base::SharedMemory::DuplicateHandle(handle);
+  }
+  return dispatcher_->ShareSharedMemoryHandleWithRemote(handle);
 }
 
 bool RendererPpapiHostImpl::IsRunningInProcess() const {

diff --git a/ppapi/proxy/ppapi_proxy_test.cc b/ppapi/proxy/ppapi_proxy_test.cc
@@ -260,14 +260,8 @@ PluginProxyTestHarness::PluginDelegateMock::ShareHandleWithRemote(
 base::SharedMemoryHandle
 PluginProxyTestHarness::PluginDelegateMock::ShareSharedMemoryHandleWithRemote(
     const base::SharedMemoryHandle& handle,
-    base::ProcessId remote_pid) {
-#if defined(OS_POSIX)
-  return ShareHandleWithRemote(handle.fd, remote_pid, false);
-#elif defined(OS_WIN)
-  return ShareHandleWithRemote(handle, remote_pid, false);
-#else
-#error Not implemented.
-#endif
+    base::ProcessId /* remote_pid */) {
+  return base::SharedMemory::DuplicateHandle(handle);
 }
 
 std::set<PP_Instance>*
@@ -506,14 +500,8 @@ HostProxyTestHarness::DelegateMock::ShareHandleWithRemote(
 base::SharedMemoryHandle
 HostProxyTestHarness::DelegateMock::ShareSharedMemoryHandleWithRemote(
     const base::SharedMemoryHandle& handle,
-    base::ProcessId remote_pid) {
-#if defined(OS_POSIX)
-  return ShareHandleWithRemote(handle.fd, remote_pid, false);
-#elif defined(OS_WIN)
-  return ShareHandleWithRemote(handle, remote_pid, false);
-#else
-#error Not implemented.
-#endif
+    base::ProcessId /*remote_pid*/) {
+  return base::SharedMemory::DuplicateHandle(handle);
 }
 
 // HostProxyTest ---------------------------------------------------------------

diff --git a/ppapi/proxy/proxy_channel.cc b/ppapi/proxy/proxy_channel.cc
@@ -76,13 +76,11 @@ IPC::PlatformFileForTransit ProxyChannel::ShareHandleWithRemote(
 
 base::SharedMemoryHandle ProxyChannel::ShareSharedMemoryHandleWithRemote(
     const base::SharedMemoryHandle& handle) {
-#if defined(OS_POSIX)
-  return ShareHandleWithRemote(handle.fd, false);
-#elif defined(OS_WIN)
-  return ShareHandleWithRemote(handle, false);
-#else
-#error Not implemented.
-#endif
+  if (!channel_.get())
+    return base::SharedMemory::NULLHandle();
+
+  DCHECK(peer_pid_ != base::kNullProcessId);
+  return delegate_->ShareSharedMemoryHandleWithRemote(handle, peer_pid_);
 }
 
 bool ProxyChannel::Send(IPC::Message* msg) {