Implement out-of-process proxy for PPB_Buffer_Dev.

BUG=85427,85441
TEST=./ninja/ui_tests --gtest_filter=*PPAPITest.Buffer

Review URL: http://codereview.chromium.org/7108051

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@88630 0039d316-1c4b-4281-b951-d872f2087c98

chrome/test/ui/ppapi_uitest.cc

@@ -26,6 +26,8 @@ const char library_name[] = "libppapi_tests.so";
 
 }  // namespace
 
+// In-process plugin test runner.  See OutOfProcessPPAPITest below for the
+// out-of-process version.
 class PPAPITest : public UITest {
  public:
   PPAPITest() {
@@ -116,6 +118,16 @@ class PPAPITest : public UITest {
   }
 };
 
+// Variant of PPAPITest that runs plugins out-of-process to test proxy
+// codepaths.
+class OutOfProcessPPAPITest : public PPAPITest {
+ public:
+  OutOfProcessPPAPITest() {
+    // Run PPAPI out-of-process to exercise proxy implementations.
+    launch_arguments_.AppendSwitch(switches::kPpapiOutOfProcess);
+  }
+};
+
 TEST_F(PPAPITest, Broker) {
   RunTest("Broker");
 }
@@ -139,6 +151,11 @@ TEST_F(PPAPITest, ImageData) {
 TEST_F(PPAPITest, Buffer) {
   RunTest("Buffer");
 }
+#if !defined(OS_MACOSX)
+TEST_F(OutOfProcessPPAPITest, Buffer) {
+  RunTest("Buffer");
+}
+#endif
 
 TEST_F(PPAPITest, URLLoader) {
   RunTestViaHTTP("URLLoader");

ppapi/c/trusted/ppb_buffer_trusted.h

+/* Copyright (c) 2011 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.
+ */
+#ifndef PPAPI_C_TRUSTED_PPB_BUFFER_TRUSTED_H_
+#define PPAPI_C_TRUSTED_PPB_BUFFER_TRUSTED_H_
+
+#include "ppapi/c/pp_stdint.h"
+#include "ppapi/c/pp_resource.h"
+
+#define PPB_BUFFER_TRUSTED_INTERFACE "PPB_BufferTrusted;0.1"
+
+struct PPB_BufferTrusted {
+  /**
+   * Returns the internal shared memory pointer associated with the given
+   * Buffer resource.  Used for proxying.  Returns PP_OK on success, or
+   * PP_ERROR_* on failure.  On success, the size in bytes of the shared
+   * memory region will be placed into |*byte_count|, and the handle for
+   * the shared memory in |*handle|.
+   */
+  int32_t (*GetSharedMemory)(PP_Resource buffer, int* handle);
+};
+
+#endif  // PPAPI_C_TRUSTED_PPB_BUFFER_TRUSTED_H_

ppapi/cpp/dev/buffer_dev.cc

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -41,5 +41,8 @@ Buffer_Dev::Buffer_Dev(Instance* instance, uint32_t size)
     *this = Buffer_Dev();
 }
 
-}  // namespace pp
+Buffer_Dev::~Buffer_Dev() {
+  get_interface<PPB_Buffer_Dev>()->Unmap(pp_resource());
+}
 
+}  // namespace pp

ppapi/cpp/dev/buffer_dev.h

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -15,13 +15,15 @@ class Buffer_Dev : public Resource {
  public:
   // Creates an is_null() Buffer object.
   Buffer_Dev();
-
   Buffer_Dev(const Buffer_Dev& other);
 
-  // Allocates a new Buffer in the browser with the given size. The
-  // resulting object will be is_null() if the allocation failed.
+  // Creates & Maps a new Buffer in the browser with the given size. The
+  // resulting object will be is_null() if either Create() or Map() fails.
   Buffer_Dev(Instance* instance, uint32_t size);
 
+  // Unmap the underlying shared memory.
+  virtual ~Buffer_Dev();
+
   uint32_t size() const { return size_; }
   void* data() const { return data_; }
 
@@ -33,4 +35,3 @@ class Buffer_Dev : public Resource {
 }  // namespace pp
 
 #endif  // PPAPI_CPP_DEV_BUFFER_DEV_H_
-

ppapi/ppapi_cpp.gypi

@@ -102,8 +102,9 @@
 
         # Trusted interfaces.
         'c/trusted/ppb_audio_trusted.h',
-        'c/trusted/ppb_image_data_trusted.h',
         'c/trusted/ppb_broker_trusted.h',
+        'c/trusted/ppb_buffer_trusted.h',
+        'c/trusted/ppb_image_data_trusted.h',
         'c/trusted/ppb_url_loader_trusted.h',
         'c/trusted/ppp_broker.h',
       ],

ppapi/ppapi_shared.gypi

@@ -59,6 +59,8 @@
         'thunk/ppb_broker_thunk.cc',
         'thunk/ppb_buffer_api.h',
         'thunk/ppb_buffer_thunk.cc',
+        'thunk/ppb_buffer_trusted_api.h',
+        'thunk/ppb_buffer_trusted_thunk.cc',
         'thunk/ppb_char_set_api.h',
         'thunk/ppb_char_set_thunk.cc',
         'thunk/ppb_cursor_control_api.h',

ppapi/proxy/ppapi_messages.h

@@ -289,7 +289,7 @@ IPC_SYNC_MESSAGE_ROUTED2_2(PpapiHostMsg_PPBBuffer_Create,
                            PP_Instance /* instance */,
                            uint32_t /* size */,
                            pp::proxy::HostResource /* result_resource */,
-                           int32_t /* result_shm_handle */)
+                           base::SharedMemoryHandle /* result_shm_handle */)
 
 // PPB_Console.
 IPC_MESSAGE_ROUTED3(PpapiHostMsg_PPBConsole_Log,
@@ -789,4 +789,3 @@ IPC_SYNC_MESSAGE_ROUTED4_3(PpapiHostMsg_ResourceCreation_ImageData,
                            pp::proxy::HostResource /* result_resource */,
                            std::string /* image_data_desc */,
                            pp::proxy::ImageHandle /* result */)
-

ppapi/proxy/ppb_buffer_proxy.cc

@@ -9,12 +9,16 @@
 #include "base/logging.h"
 #include "build/build_config.h"
 #include "ppapi/c/pp_completion_callback.h"
+#include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_resource.h"
 #include "ppapi/c/dev/ppb_buffer_dev.h"
+#include "ppapi/proxy/host_dispatcher.h"
 #include "ppapi/proxy/plugin_dispatcher.h"
 #include "ppapi/proxy/plugin_resource.h"
 #include "ppapi/proxy/ppapi_messages.h"
+#include "ppapi/thunk/enter.h"
 #include "ppapi/thunk/ppb_buffer_api.h"
+#include "ppapi/thunk/ppb_buffer_trusted_api.h"
 #include "ppapi/thunk/thunk.h"
 
 namespace pp {
@@ -33,7 +37,7 @@ class Buffer : public ppapi::thunk::PPB_Buffer_API,
                public PluginResource {
  public:
   Buffer(const HostResource& resource,
-         int memory_handle,
+         const base::SharedMemoryHandle& shm_handle,
          uint32_t size);
   virtual ~Buffer();
 
@@ -50,19 +54,18 @@ class Buffer : public ppapi::thunk::PPB_Buffer_API,
   virtual void Unmap() OVERRIDE;
 
  private:
-  int memory_handle_;
+  base::SharedMemory shm_;
   uint32_t size_;
-
   void* mapped_data_;
 
   DISALLOW_COPY_AND_ASSIGN(Buffer);
 };
 
 Buffer::Buffer(const HostResource& resource,
-               int memory_handle,
+               const base::SharedMemoryHandle& shm_handle,
                uint32_t size)
     : PluginResource(resource),
-      memory_handle_(memory_handle),
+      shm_(shm_handle, false),
       size_(size),
       mapped_data_(NULL) {
 }
@@ -89,12 +92,13 @@ PP_Bool Buffer::IsMapped() {
 }
 
 void* Buffer::Map() {
-  // TODO(brettw) implement this.
-  return mapped_data_;
+  if (!shm_.memory())
+    shm_.Map(size_);
+  return shm_.memory();
 }
 
 void Buffer::Unmap() {
-  // TODO(brettw) implement this.
+  shm_.Unmap();
 }
 
 PPB_Buffer_Proxy::PPB_Buffer_Proxy(Dispatcher* dispatcher,
@@ -125,15 +129,14 @@ PP_Resource PPB_Buffer_Proxy::CreateProxyResource(PP_Instance instance,
     return 0;
 
   HostResource result;
-  int32_t shm_handle = -1;
+  base::SharedMemoryHandle shm_handle = base::SharedMemory::NULLHandle();
   dispatcher->Send(new PpapiHostMsg_PPBBuffer_Create(
       INTERFACE_ID_PPB_BUFFER, instance, size,
       &result, &shm_handle));
-  if (result.is_null())
+  if (result.is_null() || !base::SharedMemory::IsHandleValid(shm_handle))
     return 0;
 
-  linked_ptr<Buffer> object(new Buffer(result,
-                                       static_cast<int>(shm_handle), size));
+  linked_ptr<Buffer> object(new Buffer(result, shm_handle, size));
   return PluginResourceTracker::GetInstance()->AddResource(object);
 }
 
@@ -147,15 +150,41 @@ bool PPB_Buffer_Proxy::OnMessageReceived(const IPC::Message& msg) {
   return handled;
 }
 
-void PPB_Buffer_Proxy::OnMsgCreate(PP_Instance instance,
-                                   uint32_t size,
-                                   HostResource* result_resource,
-                                   int* result_shm_handle) {
-  result_resource->SetHostResource(
-      instance,
-      ppb_buffer_target()->Create(instance, size));
-  // TODO(brettw) set the shm handle from a trusted interface.
-  *result_shm_handle = 0;
+void PPB_Buffer_Proxy::OnMsgCreate(
+    PP_Instance instance,
+    uint32_t size,
+    HostResource* result_resource,
+    base::SharedMemoryHandle* result_shm_handle) {
+  // Overwritten below on success.
+  *result_shm_handle = base::SharedMemory::NULLHandle();
+  HostDispatcher* dispatcher = HostDispatcher::GetForInstance(instance);
+  if (!dispatcher)
+    return;
+  PP_Resource local_buffer_resource =
+      ppb_buffer_target()->Create(instance, size);
+  if (local_buffer_resource == 0)
+    return;
+  ::ppapi::thunk::EnterResourceNoLock< ::ppapi::thunk::PPB_BufferTrusted_API>
+        trusted_buffer(local_buffer_resource, false);
+  if (trusted_buffer.failed())
+    return;
+  int local_fd;
+  if (trusted_buffer.object()->GetSharedMemory(&local_fd) != PP_OK)
+    return;
+
+  result_resource->SetHostResource(instance, local_buffer_resource);
+
+  // 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>(local_fd));
+#elif defined(OS_POSIX)
+      local_fd;
+#else
+  #error Not implemented.
+#endif
+  *result_shm_handle = dispatcher->ShareHandleWithRemote(platform_file, false);
 }
 
 }  // namespace proxy

ppapi/proxy/ppb_buffer_proxy.h

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
 #ifndef PPAPI_PPB_BUFFER_PROXY_H_
 #define PPAPI_PPB_BUFFER_PROXY_H_
 
+#include "base/shared_memory.h"
 #include "ppapi/c/pp_instance.h"
 #include "ppapi/proxy/interface_proxy.h"
 
@@ -37,7 +38,7 @@ class PPB_Buffer_Proxy : public InterfaceProxy {
   void OnMsgCreate(PP_Instance instance,
                    uint32_t size,
                    HostResource* result_resource,
-                   int* result_shm_handle);
+                   base::SharedMemoryHandle* result_shm_handle);
 };
 
 }  // namespace proxy

ppapi/shared_impl/resource_object_base.h

@@ -13,6 +13,7 @@
   F(PPB_AudioTrusted_API) \
   F(PPB_Broker_API) \
   F(PPB_Buffer_API) \
+  F(PPB_BufferTrusted_API) \
   F(PPB_DirectoryReader_API) \
   F(PPB_FileChooser_API) \
   F(PPB_FileIO_API) \

ppapi/tests/test_buffer.cc

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -23,6 +23,7 @@ void TestBuffer::RunTest() {
   instance_->LogTest("InvalidSize", TestInvalidSize());
   instance_->LogTest("InitToZero", TestInitToZero());
   instance_->LogTest("IsBuffer", TestIsBuffer());
+  instance_->LogTest("BasicLifecyle", TestBasicLifeCycle());
 }
 
 std::string TestBuffer::TestInvalidSize() {
@@ -75,3 +76,28 @@ std::string TestBuffer::TestIsBuffer() {
   PASS();
 }
 
+std::string TestBuffer::TestBasicLifeCycle() {
+  enum { kBufferSize = 100 };
+
+  pp::Buffer_Dev *buffer = new pp::Buffer_Dev(instance_, kBufferSize);
+  if (buffer->is_null() ||
+      !buffer_interface_->IsBuffer(buffer->pp_resource()) ||
+      buffer->size() != kBufferSize) {
+    return "Error creating buffer (earlier test should have failed)";
+  }
+
+  // Test that the buffer got created & mapped.
+  if (buffer->data() == NULL)
+    return "Failed to Map() buffer";
+
+  // Test that the buffer is writeable.
+  char* data = static_cast<char*>(buffer->data());
+  for (int i = 0; i < kBufferSize; ++i)
+    data[i] = 'X';
+
+  // Implicitly test that destroying the buffer doesn't encounter a fatal error
+  // in Unmap.
+  delete buffer;
+
+  PASS();
+}

ppapi/tests/test_buffer.h

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -23,6 +23,7 @@ class TestBuffer : public TestCase {
   std::string TestInvalidSize();
   std::string TestInitToZero();
   std::string TestIsBuffer();
+  std::string TestBasicLifeCycle();
 
   // Used by the tests that access the C API directly.
   const PPB_Buffer_Dev* buffer_interface_;

ppapi/thunk/ppb_buffer_trusted_api.h

+// Copyright (c) 2011 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.
+
+#ifndef PPAPI_THUNK_BUFFER_TRUSTED_API_H_
+#define PPAPI_THUNK_BUFFER_TRUSTED_API_H_
+
+#include "ppapi/c/dev/ppb_buffer_dev.h"
+#include "ppapi/c/trusted/ppb_buffer_trusted.h"
+
+namespace ppapi {
+namespace thunk {
+
+class PPB_BufferTrusted_API {
+ public:
+  virtual int32_t GetSharedMemory(int* handle) = 0;
+};
+
+}  // namespace thunk
+}  // namespace ppapi
+
+#endif  // PPAPI_THUNK_BUFFER_TRUSTED_API_H_

ppapi/thunk/ppb_buffer_trusted_thunk.cc

+// Copyright (c) 2011 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 "ppapi/c/pp_errors.h"
+#include "ppapi/thunk/thunk.h"
+#include "ppapi/thunk/enter.h"
+#include "ppapi/thunk/ppb_buffer_trusted_api.h"
+#include "ppapi/thunk/resource_creation_api.h"
+
+namespace ppapi {
+namespace thunk {
+
+namespace {
+
+int32_t GetSharedMemory(PP_Resource buffer_id, int* shm_handle) {
+  EnterResource<PPB_BufferTrusted_API> enter(buffer_id, true);
+  if (enter.failed())
+    return PP_ERROR_BADRESOURCE;
+  return enter.object()->GetSharedMemory(shm_handle);
+}
+
+const PPB_BufferTrusted g_ppb_buffer_trusted_thunk = {
+  &GetSharedMemory,
+};
+
+}  // namespace
+
+const PPB_BufferTrusted* GetPPB_BufferTrusted_Thunk() {
+  return &g_ppb_buffer_trusted_thunk;
+}
+
+}  // namespace thunk
+}  // namespace ppapi

ppapi/thunk/resource_creation_api.h

@@ -24,7 +24,7 @@ namespace thunk {
 // A functional API for creating resource types. Separating out the creation
 // functions here allows us to implement most resources as a pure "resource
 // API", meaning all calls are routed on a per-resource-object basis. The
-// creationg functions are not per-object (since there's no object during
+// creation functions are not per-object (since there's no object during
 // creation) so need functional routing based on the instance ID.
 class ResourceCreationAPI {
  public:

ppapi/thunk/thunk.h

@@ -10,6 +10,7 @@ struct PPB_AudioConfig;
 struct PPB_AudioTrusted;
 struct PPB_BrokerTrusted;
 struct PPB_Buffer_Dev;
+struct PPB_BufferTrusted;
 struct PPB_CharSet_Dev;
 struct PPB_CursorControl_Dev;
 struct PPB_DirectoryReader_Dev;
@@ -31,6 +32,7 @@ const PPB_AudioConfig* GetPPB_AudioConfig_Thunk();
 const PPB_AudioTrusted* GetPPB_AudioTrusted_Thunk();
 const PPB_BrokerTrusted* GetPPB_Broker_Thunk();
 const PPB_Buffer_Dev* GetPPB_Buffer_Thunk();
+const PPB_BufferTrusted* GetPPB_BufferTrusted_Thunk();
 const PPB_CharSet_Dev* GetPPB_CharSet_Thunk();
 const PPB_CursorControl_Dev* GetPPB_CursorControl_Thunk();
 const PPB_DirectoryReader_Dev* GetPPB_DirectoryReader_Thunk();
@@ -48,5 +50,3 @@ const PPB_ImageData* GetPPB_ImageData_Thunk();
 }  // namespace ppapi
 
 #endif  // PPAPI_THUNK_THUNK_H_
-
-

webkit/plugins/ppapi/plugin_module.cc

@@ -66,6 +66,7 @@
 #include "ppapi/c/private/ppb_uma_private.h"
 #include "ppapi/c/trusted/ppb_audio_trusted.h"
 #include "ppapi/c/trusted/ppb_broker_trusted.h"
+#include "ppapi/c/trusted/ppb_buffer_trusted.h"
 #include "ppapi/c/trusted/ppb_image_data_trusted.h"
 #include "ppapi/c/trusted/ppb_url_loader_trusted.h"
 #include "ppapi/thunk/thunk.h"
@@ -257,6 +258,8 @@ const void* GetInterface(const char* name) {
     return ::ppapi::thunk::GetPPB_Broker_Thunk();
   if (strcmp(name, PPB_BUFFER_DEV_INTERFACE) == 0)
     return ::ppapi::thunk::GetPPB_Buffer_Thunk();
+  if (strcmp(name, PPB_BUFFER_TRUSTED_INTERFACE) == 0)
+    return ::ppapi::thunk::GetPPB_BufferTrusted_Thunk();
   if (strcmp(name, PPB_CHAR_SET_DEV_INTERFACE) == 0)
     return ::ppapi::thunk::GetPPB_CharSet_Thunk();
   if (strcmp(name, PPB_CONSOLE_DEV_INTERFACE) == 0)

webkit/plugins/ppapi/ppb_buffer_impl.cc

@@ -15,6 +15,7 @@
 #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 
 using ::ppapi::thunk::PPB_Buffer_API;
+using ::ppapi::thunk::PPB_BufferTrusted_API;
 
 namespace webkit {
 namespace ppapi {
@@ -45,6 +46,10 @@ PPB_Buffer_API* PPB_Buffer_Impl::AsPPB_Buffer_API() {
   return this;
 }
 
+PPB_BufferTrusted_API* PPB_Buffer_Impl::AsPPB_BufferTrusted_API() {
+  return this;
+}
+
 bool PPB_Buffer_Impl::Init(uint32_t size) {
   if (size == 0 || !instance())
     return false;
@@ -75,6 +80,18 @@ void PPB_Buffer_Impl::Unmap() {
   shared_memory_->Unmap();
 }
 
+int32_t PPB_Buffer_Impl::GetSharedMemory(int* shm_handle) {
+#if defined(OS_POSIX)
+  *shm_handle = shared_memory_->handle().fd;
+#elif defined(OS_WIN)
+  *shm_handle = reinterpret_cast<int>(
+      shared_memory_->handle());
+#else
+#error "Platform not supported."
+#endif
+  return PP_OK;
+}
+
 BufferAutoMapper::BufferAutoMapper(PPB_Buffer_API* api) : api_(api) {
   needs_unmap_ = !PP_ToBool(api->IsMapped());
   data_ = api->Map();

webkit/plugins/ppapi/ppb_buffer_impl.h

@@ -9,6 +9,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/shared_memory.h"
 #include "ppapi/thunk/ppb_buffer_api.h"
+#include "ppapi/thunk/ppb_buffer_trusted_api.h"
 #include "webkit/plugins/ppapi/resource.h"
 
 struct PPB_Buffer_Dev;
@@ -19,7 +20,8 @@ namespace ppapi {
 class PluginInstance;
 
 class PPB_Buffer_Impl : public Resource,
-                        public ::ppapi::thunk::PPB_Buffer_API {
+                        public ::ppapi::thunk::PPB_Buffer_API,
+                        public ::ppapi::thunk::PPB_BufferTrusted_API {
  public:
   virtual ~PPB_Buffer_Impl();
 
@@ -30,8 +32,9 @@ class PPB_Buffer_Impl : public Resource,
   base::SharedMemory* shared_memory() const { return shared_memory_.get(); }
   uint32_t size() const { return size_; }
 
-  // ResourceObjectBase overries.
+  // ResourceObjectBase overrides.
   virtual ::ppapi::thunk::PPB_Buffer_API* AsPPB_Buffer_API() OVERRIDE;
+  virtual ::ppapi::thunk::PPB_BufferTrusted_API* AsPPB_BufferTrusted_API();
 
   // PPB_Buffer_API implementation.
   virtual PP_Bool Describe(uint32_t* size_in_bytes) OVERRIDE;
@@ -39,6 +42,9 @@ class PPB_Buffer_Impl : public Resource,
   virtual void* Map() OVERRIDE;
   virtual void Unmap() OVERRIDE;
 
+  // PPB_BufferTrusted_API implementation.
+  virtual int32_t GetSharedMemory(int* handle) OVERRIDE;
+
  private:
   explicit PPB_Buffer_Impl(PluginInstance* instance);
   bool Init(uint32_t size);
@@ -49,7 +55,7 @@ class PPB_Buffer_Impl : public Resource,
   DISALLOW_COPY_AND_ASSIGN(PPB_Buffer_Impl);
 };
 
-// Ensures that the given buffer is mapped, and retursn it to its previous
+// Ensures that the given buffer is mapped, and returns it to its previous
 // mapped state in the destructor.
 class BufferAutoMapper {
  public: