Add proper support for copy-constructed pp::Buffer_Dev's.

This requires refcounting Map() to make sure that Unmap() is called once per
underlying SharedMemory.

BUG=85629
TEST=trybots, ui_tests:*PPAPI.Buffer

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

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

5 files changed, 30 insertions, 11 deletions

diff --git a/ppapi/cpp/dev/buffer_dev.cc b/ppapi/cpp/dev/buffer_dev.cc
index 46b34bb..38a1b1f 100644
--- a/ppapi/cpp/dev/buffer_dev.cc
+++ b/ppapi/cpp/dev/buffer_dev.cc
@@ -23,9 +23,11 @@ Buffer_Dev::Buffer_Dev() : data_(NULL), size_(0) {
 }
 
 Buffer_Dev::Buffer_Dev(const Buffer_Dev& other)
-    : Resource(other),
-      data_(other.data_),
-      size_(other.size_) {
+    : Resource(other) {
+  if (!get_interface<PPB_Buffer_Dev>()->Describe(pp_resource(), &size_) ||
+      !(data_ = get_interface<PPB_Buffer_Dev>()->Map(pp_resource()))) {
+    *this = Buffer_Dev();
+  }
 }
 
 Buffer_Dev::Buffer_Dev(Instance* instance, uint32_t size)
@@ -37,8 +39,9 @@ Buffer_Dev::Buffer_Dev(Instance* instance, uint32_t size)
   PassRefFromConstructor(get_interface<PPB_Buffer_Dev>()->Create(
       instance->pp_instance(), size));
   if (!get_interface<PPB_Buffer_Dev>()->Describe(pp_resource(), &size_) ||
-      !(data_ = get_interface<PPB_Buffer_Dev>()->Map(pp_resource())))
+      !(data_ = get_interface<PPB_Buffer_Dev>()->Map(pp_resource()))) {
     *this = Buffer_Dev();
+  }
 }
 
 Buffer_Dev::~Buffer_Dev() {
diff --git a/ppapi/proxy/ppb_buffer_proxy.cc b/ppapi/proxy/ppb_buffer_proxy.cc
index a358de7..ec4b1ea 100644
--- a/ppapi/proxy/ppb_buffer_proxy.cc
+++ b/ppapi/proxy/ppb_buffer_proxy.cc
@@ -57,6 +57,7 @@ class Buffer : public ppapi::thunk::PPB_Buffer_API,
   base::SharedMemory shm_;
   uint32_t size_;
   void* mapped_data_;
+  int map_count_;
 
   DISALLOW_COPY_AND_ASSIGN(Buffer);
 };
@@ -67,7 +68,8 @@ Buffer::Buffer(const HostResource& resource,
     : PluginResource(resource),
       shm_(shm_handle, false),
       size_(size),
-      mapped_data_(NULL) {
+      mapped_data_(NULL),
+      map_count_(0) {
 }
 
 Buffer::~Buffer() {
@@ -92,13 +94,14 @@ PP_Bool Buffer::IsMapped() {
 }
 
 void* Buffer::Map() {
-  if (!shm_.memory())
+  if (map_count_++ == 0)
     shm_.Map(size_);
   return shm_.memory();
 }
 
 void Buffer::Unmap() {
-  shm_.Unmap();
+  if (--map_count_ == 0)
+    shm_.Unmap();
 }
 
 PPB_Buffer_Proxy::PPB_Buffer_Proxy(Dispatcher* dispatcher,
diff --git a/ppapi/tests/test_buffer.cc b/ppapi/tests/test_buffer.cc
index 109b8a6..b44047b 100644
--- a/ppapi/tests/test_buffer.cc
+++ b/ppapi/tests/test_buffer.cc
@@ -95,9 +95,20 @@ std::string TestBuffer::TestBasicLifeCycle() {
   for (int i = 0; i < kBufferSize; ++i)
     data[i] = 'X';
 
+  // Implicitly test that the copy constructor doesn't cause a double-unmap on
+  // delete.
+  pp::Buffer_Dev* copy = new pp::Buffer_Dev(*buffer);
+
   // Implicitly test that destroying the buffer doesn't encounter a fatal error
   // in Unmap.
   delete buffer;
 
+  // Test that we can still write to copy's copy of the data.
+  char* copy_data = static_cast<char*>(copy->data());
+  for (int i = 0; i < kBufferSize; ++i)
+    copy_data[i] = 'Y';
+
+  delete copy;
+
   PASS();
 }
diff --git a/webkit/plugins/ppapi/ppb_buffer_impl.cc b/webkit/plugins/ppapi/ppb_buffer_impl.cc
index 5c248d6..b32c616 100644
--- a/webkit/plugins/ppapi/ppb_buffer_impl.cc
+++ b/webkit/plugins/ppapi/ppb_buffer_impl.cc
@@ -21,7 +21,7 @@ namespace webkit {
 namespace ppapi {
 
 PPB_Buffer_Impl::PPB_Buffer_Impl(PluginInstance* instance)
-    : Resource(instance), size_(0) {
+    : Resource(instance), size_(0), map_count_(0) {
 }
 
 PPB_Buffer_Impl::~PPB_Buffer_Impl() {
@@ -71,13 +71,14 @@ PP_Bool PPB_Buffer_Impl::IsMapped() {
 void* PPB_Buffer_Impl::Map() {
   DCHECK(size_);
   DCHECK(shared_memory_.get());
-  if (!shared_memory_->Map(size_))
-    return NULL;
+  if (map_count_++ == 0)
+    shared_memory_->Map(size_);
   return shared_memory_->memory();
 }
 
 void PPB_Buffer_Impl::Unmap() {
-  shared_memory_->Unmap();
+  if (--map_count_ == 0)
+    shared_memory_->Unmap();
 }
 
 int32_t PPB_Buffer_Impl::GetSharedMemory(int* shm_handle) {
diff --git a/webkit/plugins/ppapi/ppb_buffer_impl.h b/webkit/plugins/ppapi/ppb_buffer_impl.h
index 2cba90d..86dbec9 100644
--- a/webkit/plugins/ppapi/ppb_buffer_impl.h
+++ b/webkit/plugins/ppapi/ppb_buffer_impl.h
@@ -51,6 +51,7 @@ class PPB_Buffer_Impl : public Resource,
 
   scoped_ptr<base::SharedMemory> shared_memory_;
   uint32_t size_;
+  int map_count_;
 
   DISALLOW_COPY_AND_ASSIGN(PPB_Buffer_Impl);
 };