Fix resource leaks in CDM implementation.

webkit/media/crypto/ppapi/cdm_wrapper.cc

 // Copyright (c) 2012 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 <cstring>  // For memcpy.
 #include <vector>
 
 #include "base/compiler_specific.h"  // For OVERRIDE.
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_stdint.h"
@@ skipping 72 matching lines @@
       const PP_EncryptedBlockInfo& encrypted_block_info) OVERRIDE;
   virtual bool DecryptAndDecode(
       pp::Buffer_Dev encrypted_buffer,
       const PP_EncryptedBlockInfo& encrypted_block_info) OVERRIDE;
 
  private:
   // Creates a PP_Resource containing a PPB_Buffer_Impl, copies |data| into the
   // 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::Resource 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
   // <code>PPP_ContentDecryptor_Private</code> are asynchronous.
   void KeyAdded(int32_t result, const std::string& session_id);
   void KeyMessage(int32_t result, cdm::KeyMessage& key_message);
   void KeyError(int32_t result, const std::string& session_id);
   void DeliverBlock(int32_t result,
                     const cdm::Status& status,
                     cdm::OutputBuffer& output_buffer,
@@ skipping 122 matching lines @@
   input_buffer.timestamp = encrypted_block_info.tracking_info.timestamp;
 
   cdm::OutputBuffer output_buffer;
   cdm::Status status = cdm_->Decrypt(input_buffer, &output_buffer);
 
   CallOnMain(callback_factory_.NewCallback(
       &CdmWrapper::DeliverBlock,
       status,
       output_buffer,
       encrypted_block_info.tracking_info));
-
   return true;
 }
 
 bool CdmWrapper::DecryptAndDecode(
     pp::Buffer_Dev encrypted_buffer,
     const PP_EncryptedBlockInfo& encrypted_block_info) {
   return false;
 }
 
-PP_Resource CdmWrapper::MakeBufferResource(const uint8_t* data,
-                                           uint32_t data_size) {
+pp::Resource CdmWrapper::MakeBufferResource(const uint8_t* data,
+                                            uint32_t data_size) {
   if (!data || !data_size)
-    return 0;
+    return pp::Resource();
 
   pp::Buffer_Dev buffer(this, data_size);
   if (!buffer.data())
-    return 0;
+    return pp::Resource();
 
   memcpy(buffer.data(), data, data_size);
-
-  return buffer.detach();
+  return buffer;
 }
 
 void CdmWrapper::KeyAdded(int32_t result, const std::string& session_id) {
   pp::ContentDecryptor_Private::KeyAdded(key_system_, session_id);
 }
 
 void CdmWrapper::KeyMessage(int32_t result,
                             cdm::KeyMessage& key_message) {
-  pp::Buffer_Dev message_buffer(MakeBufferResource(key_message.message,
-                                                   key_message.message_size));
+  pp::Buffer_Dev message_buffer(
+      MakeBufferResource(key_message.message,
+                         key_message.message_size).pp_resource());

[dmichael (off chromium), 2012/09/06 02:46:52]

Don't call pp_resource; just use the copy constructor. The C++ resource wrappers
are designed so that you can copy or pass them around safely. Going to the C
version unnecessarily just opens you up to mistakes.

[Tom Finegan, 2012/09/06 04:01:02]

Done.

   pp::ContentDecryptor_Private::KeyMessage(
       key_system_,
       std::string(key_message.session_id, key_message.session_id_size),
       message_buffer,
       std::string(key_message.default_url, key_message.default_url_size));
 
   // TODO(xhwang): Fix this. This is not always safe as the memory is allocated
   // in another shared object.
   delete [] key_message.session_id;
   key_message.session_id = NULL;
   delete [] key_message.message;
   key_message.message = NULL;
   delete [] key_message.default_url;
   key_message.default_url = NULL;
 }
 
 // TODO(xhwang): Support MediaKeyError (see spec: http://goo.gl/rbdnR) in CDM
 // interface and in this function.
 void CdmWrapper::KeyError(int32_t result, const std::string& session_id) {
   pp::ContentDecryptor_Private::KeyError(key_system_,
                                          session_id,
                                          kUnknownError,
                                          0);
 }
 
 void CdmWrapper::DeliverBlock(int32_t result,
                               const cdm::Status& status,
                               cdm::OutputBuffer& output_buffer,
                               const PP_DecryptTrackingInfo& tracking_info) {
-  pp::Buffer_Dev decrypted_buffer(MakeBufferResource(output_buffer.data,
-                                                     output_buffer.data_size));
+  pp::Buffer_Dev decrypted_buffer(
+      MakeBufferResource(output_buffer.data,
+                         output_buffer.data_size).pp_resource());

[dmichael (off chromium), 2012/09/06 02:46:52]

ditto

[Tom Finegan, 2012/09/06 04:01:02]

Done.

 
   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;
 
   switch (status) {
     case cdm::kSuccess:
       decrypted_block_info.result = PP_DECRYPTRESULT_SUCCESS;
       break;
     case cdm::kErrorNoKey:
@@ skipping 27 matching lines @@
 }  // namespace webkit_media
 
 namespace pp {
 
 // Factory function for your specialization of the Module object.
 Module* CreateModule() {
   return new webkit_media::MyModule();
 }
 
 }  // namespace pp