Fix resource leaks in CDM implementation.

Two fixes:

1) Add a PassRef constructor to pp::Buffer_Dev to deal with reference counting
   issues related to passing PP_Resources across the IPC proxy, and use it to
   avoid adding unnecessary references on PP_Resources.
2) Use the PassRef ScopedPPResource contructor in PluginInstance::Decrypt to 
   avoid added an unnecessary reference on the buffer resource before it's 
   passed to the plugin interface code.

BUG=146200
TEST=PPB_Testing_Dev::GetLiveObjectsForInstance shows a stable live
object count.


Review URL: https://chromiumcodereview.appspot.com/10909068

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

2 files changed, 10 insertions, 12 deletions

diff --git a/webkit/media/crypto/ppapi/cdm_wrapper.cc b/webkit/media/crypto/ppapi/cdm_wrapper.cc
index 88baa44..6abe3e9 100644
--- a/webkit/media/crypto/ppapi/cdm_wrapper.cc
+++ b/webkit/media/crypto/ppapi/cdm_wrapper.cc
@@ -90,7 +90,7 @@ class CdmWrapper : public pp::Instance,
   // buffer resource, and returns it. Returns a an invalid PP_Resource with an
   // ID of 0 on failure. Upon success, the returned Buffer resource has a
   // reference count of 1.
-  PP_Resource MakeBufferResource(const uint8_t* data, uint32_t data_size);
+  pp::Buffer_Dev MakeBufferResource(const uint8_t* data, uint32_t data_size);
 
   // <code>PPB_ContentDecryptor_Private</code> dispatchers. These are passed to
   // <code>callback_factory_</code> to ensure that calls into
@@ -233,7 +233,6 @@ bool CdmWrapper::Decrypt(pp::Buffer_Dev encrypted_buffer,
       status,
       output_buffer,
       encrypted_block_info.tracking_info));
-
   return true;
 }
 
@@ -243,18 +242,17 @@ bool CdmWrapper::DecryptAndDecode(
   return false;
 }
 
-PP_Resource CdmWrapper::MakeBufferResource(const uint8_t* data,
-                                           uint32_t data_size) {
+pp::Buffer_Dev CdmWrapper::MakeBufferResource(const uint8_t* data,
+                                              uint32_t data_size) {
   if (!data || !data_size)
-    return 0;
+    return pp::Buffer_Dev();
 
   pp::Buffer_Dev buffer(this, data_size);
   if (!buffer.data())
-    return 0;
+    return pp::Buffer_Dev();
 
   memcpy(buffer.data(), data, data_size);
-
-  return buffer.detach();
+  return buffer;
 }
 
 void CdmWrapper::KeyAdded(int32_t result, const std::string& session_id) {
@@ -296,7 +294,6 @@ void CdmWrapper::DeliverBlock(int32_t result,
                               const PP_DecryptTrackingInfo& tracking_info) {
   pp::Buffer_Dev decrypted_buffer(MakeBufferResource(output_buffer.data,
                                                      output_buffer.data_size));
-
   PP_DecryptedBlockInfo decrypted_block_info;
   decrypted_block_info.tracking_info.request_id = tracking_info.request_id;
   decrypted_block_info.tracking_info.timestamp = output_buffer.timestamp;
diff --git a/webkit/plugins/ppapi/ppapi_plugin_instance.cc b/webkit/plugins/ppapi/ppapi_plugin_instance.cc
index 30dc9379..b80f0ff 100644
--- a/webkit/plugins/ppapi/ppapi_plugin_instance.cc
+++ b/webkit/plugins/ppapi/ppapi_plugin_instance.cc
@@ -1506,6 +1506,7 @@ bool PluginInstance::Decrypt(
     return false;
 
   ScopedPPResource encrypted_resource(
+      ScopedPPResource::PassRef(),
       MakeBufferResource(pp_instance(),
                          encrypted_buffer->GetData(),
                          encrypted_buffer->GetDataSize()));
@@ -2235,10 +2236,8 @@ void PluginInstance::DeliverBlock(PP_Instance instance,
                                   PP_Resource decrypted_block,
                                   const PP_DecryptedBlockInfo* block_info) {
   DCHECK(block_info);
-
   DecryptionCBMap::iterator found = pending_decryption_cbs_.find(
       block_info->tracking_info.request_id);
-
   if (found == pending_decryption_cbs_.end())
     return;
   media::Decryptor::DecryptCB decrypt_cb = found->second;
@@ -2252,8 +2251,8 @@ void PluginInstance::DeliverBlock(PP_Instance instance,
     decrypt_cb.Run(media::Decryptor::kError, NULL);
     return;
   }
-  EnterResourceNoLock<PPB_Buffer_API> enter(decrypted_block, true);
 
+  EnterResourceNoLock<PPB_Buffer_API> enter(decrypted_block, true);
   if (!enter.succeeded()) {
     decrypt_cb.Run(media::Decryptor::kError, NULL);
     return;
@@ -2264,6 +2263,8 @@ void PluginInstance::DeliverBlock(PP_Instance instance,
     return;
   }
 
+  // TODO(tomfinegan): Find a way to take ownership of the shared memory
+  // managed by the PPB_Buffer_Dev, and avoid the extra copy.
   scoped_refptr<media::DecoderBuffer> decrypted_buffer(
       media::DecoderBuffer::CopyFrom(
           reinterpret_cast<const uint8*>(mapper.data()), mapper.size()));