From 32523380fc0111a95540f5a860184d8917270375 Mon Sep 17 00:00:00 2001 From: "fischman@chromium.org" Date: Fri, 10 Jun 2011 02:30:00 +0000 Subject: 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 --- ppapi/c/trusted/ppb_buffer_trusted.h | 24 +++++++++++ ppapi/cpp/dev/buffer_dev.cc | 7 +++- ppapi/cpp/dev/buffer_dev.h | 11 ++--- ppapi/ppapi_cpp.gypi | 3 +- ppapi/ppapi_shared.gypi | 2 + ppapi/proxy/ppapi_messages.h | 3 +- ppapi/proxy/ppb_buffer_proxy.cc | 71 ++++++++++++++++++++++---------- ppapi/proxy/ppb_buffer_proxy.h | 5 ++- ppapi/shared_impl/resource_object_base.h | 1 + ppapi/tests/test_buffer.cc | 28 ++++++++++++- ppapi/tests/test_buffer.h | 3 +- ppapi/thunk/ppb_buffer_trusted_api.h | 22 ++++++++++ ppapi/thunk/ppb_buffer_trusted_thunk.cc | 34 +++++++++++++++ ppapi/thunk/resource_creation_api.h | 2 +- ppapi/thunk/thunk.h | 4 +- 15 files changed, 182 insertions(+), 38 deletions(-) create mode 100644 ppapi/c/trusted/ppb_buffer_trusted.h create mode 100644 ppapi/thunk/ppb_buffer_trusted_api.h create mode 100644 ppapi/thunk/ppb_buffer_trusted_thunk.cc (limited to 'ppapi') diff --git a/ppapi/c/trusted/ppb_buffer_trusted.h b/ppapi/c/trusted/ppb_buffer_trusted.h new file mode 100644 index 0000000..d8568c5 --- /dev/null +++ b/ppapi/c/trusted/ppb_buffer_trusted.h @@ -0,0 +1,24 @@ +/* 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_ diff --git a/ppapi/cpp/dev/buffer_dev.cc b/ppapi/cpp/dev/buffer_dev.cc index 8a41ab4..46b34bb 100644 --- a/ppapi/cpp/dev/buffer_dev.cc +++ b/ppapi/cpp/dev/buffer_dev.cc @@ -1,4 +1,4 @@ -// 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()->Unmap(pp_resource()); +} +} // namespace pp diff --git a/ppapi/cpp/dev/buffer_dev.h b/ppapi/cpp/dev/buffer_dev.h index 83c265c2..b34a2f8 100644 --- a/ppapi/cpp/dev/buffer_dev.h +++ b/ppapi/cpp/dev/buffer_dev.h @@ -1,4 +1,4 @@ -// 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_ - diff --git a/ppapi/ppapi_cpp.gypi b/ppapi/ppapi_cpp.gypi index 6b04efc..74dc1f6 100644 --- a/ppapi/ppapi_cpp.gypi +++ b/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', ], diff --git a/ppapi/ppapi_shared.gypi b/ppapi/ppapi_shared.gypi index bfcd435..3fc24be 100644 --- a/ppapi/ppapi_shared.gypi +++ b/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', diff --git a/ppapi/proxy/ppapi_messages.h b/ppapi/proxy/ppapi_messages.h index 49f0045..7ceb936 100644 --- a/ppapi/proxy/ppapi_messages.h +++ b/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 */) - diff --git a/ppapi/proxy/ppb_buffer_proxy.cc b/ppapi/proxy/ppb_buffer_proxy.cc index 5c2fd8a..a358de7 100644 --- a/ppapi/proxy/ppb_buffer_proxy.cc +++ b/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 object(new Buffer(result, - static_cast(shm_handle), size)); + linked_ptr 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(static_cast(local_fd)); +#elif defined(OS_POSIX) + local_fd; +#else + #error Not implemented. +#endif + *result_shm_handle = dispatcher->ShareHandleWithRemote(platform_file, false); } } // namespace proxy diff --git a/ppapi/proxy/ppb_buffer_proxy.h b/ppapi/proxy/ppb_buffer_proxy.h index 752ab8a..6002950 100644 --- a/ppapi/proxy/ppb_buffer_proxy.h +++ b/ppapi/proxy/ppb_buffer_proxy.h @@ -1,10 +1,11 @@ -// 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 diff --git a/ppapi/shared_impl/resource_object_base.h b/ppapi/shared_impl/resource_object_base.h index 00ac498..464d477 100644 --- a/ppapi/shared_impl/resource_object_base.h +++ b/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) \ diff --git a/ppapi/tests/test_buffer.cc b/ppapi/tests/test_buffer.cc index fddc573..109b8a6 100644 --- a/ppapi/tests/test_buffer.cc +++ b/ppapi/tests/test_buffer.cc @@ -1,4 +1,4 @@ -// 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(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(); +} diff --git a/ppapi/tests/test_buffer.h b/ppapi/tests/test_buffer.h index 4c78d9d..ede075c 100644 --- a/ppapi/tests/test_buffer.h +++ b/ppapi/tests/test_buffer.h @@ -1,4 +1,4 @@ -// 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_; diff --git a/ppapi/thunk/ppb_buffer_trusted_api.h b/ppapi/thunk/ppb_buffer_trusted_api.h new file mode 100644 index 0000000..9aba99a --- /dev/null +++ b/ppapi/thunk/ppb_buffer_trusted_api.h @@ -0,0 +1,22 @@ +// 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_ diff --git a/ppapi/thunk/ppb_buffer_trusted_thunk.cc b/ppapi/thunk/ppb_buffer_trusted_thunk.cc new file mode 100644 index 0000000..f20d6d5 --- /dev/null +++ b/ppapi/thunk/ppb_buffer_trusted_thunk.cc @@ -0,0 +1,34 @@ +// 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 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 diff --git a/ppapi/thunk/resource_creation_api.h b/ppapi/thunk/resource_creation_api.h index 8a8cefc..efb0150 100644 --- a/ppapi/thunk/resource_creation_api.h +++ b/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: diff --git a/ppapi/thunk/thunk.h b/ppapi/thunk/thunk.h index 1738edd..4d0dcf7 100644 --- a/ppapi/thunk/thunk.h +++ b/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_ - - -- cgit v1.1