Fix resource leaks in CDM implementation.

ppapi/proxy/ppp_content_decryptor_private_proxy.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 "ppapi/proxy/ppp_content_decryptor_private_proxy.h"
 
 #include "base/platform_file.h"
 #include "ppapi/c/pp_bool.h"
 #include "ppapi/c/ppb_core.h"
 #include "ppapi/proxy/content_decryptor_private_serializer.h"
@@ skipping 57 matching lines @@
 // SerializedVarReceiveInput will decrement the reference count, but we want
 // to give the recipient a reference. This utility function takes care of that
 // work for the message handlers defined below.
 PP_Var ExtractReceivedVarAndAddRef(Dispatcher* dispatcher,
                                    SerializedVarReceiveInput* serialized_var) {
   PP_Var var = serialized_var->Get(dispatcher);
   PpapiGlobals::Get()->GetVarTracker()->AddRefVar(var);
   return var;
 }
 
+// Increments the reference count on |resource| to ensure that it remains valid
+// until the plugin receives the resource within the asynchronous message sent
+// from the proxy.  The plugin side takes ownership of that reference. Returns
+// PP_TRUE when the reference is successfully added, PP_FALSE otherwise.
+PP_Bool AddRefResourceForPlugin(HostDispatcher* dispatcher,
+                                PP_Resource resource) {
+  const PPB_Core* core = static_cast<const PPB_Core*>(
+      dispatcher->local_get_interface()(PPB_CORE_INTERFACE));
+  if (!core) {
+    NOTREACHED();
+    return PP_FALSE;
+  }
+  core->AddRefResource(resource);
+  return PP_TRUE;
+}
+
 PP_Bool GenerateKeyRequest(PP_Instance instance,
                            PP_Var key_system,
                            PP_Var init_data) {
   HostDispatcher* dispatcher = HostDispatcher::GetForInstance(instance);
   if (!dispatcher) {
     NOTREACHED();
     return PP_FALSE;
   }
 
   return PP_FromBool(dispatcher->Send(
@@ skipping 38 matching lines @@
 }
 
 PP_Bool Decrypt(PP_Instance instance,
                 PP_Resource encrypted_block,
                 const PP_EncryptedBlockInfo* encrypted_block_info) {
   HostDispatcher* dispatcher = HostDispatcher::GetForInstance(instance);
   if (!dispatcher) {
     NOTREACHED();
     return PP_FALSE;
   }
-  const PPB_Core* core = static_cast<const PPB_Core*>(
-      dispatcher->local_get_interface()(PPB_CORE_INTERFACE));
-  if (!core) {
-    NOTREACHED();
-    return PP_FALSE;
-  }
 
-  // We need to take a ref on the resource now. The browser may drop
-  // references once we return from here, but we're sending an asynchronous
-  // message. The plugin side takes ownership of that reference.
-  core->AddRefResource(encrypted_block);
+  if (AddRefResourceForPlugin(dispatcher, encrypted_block) != PP_TRUE) {

[dmichael (off chromium), 2012/09/05 19:46:19]

You could just do if (!AddRefResource...) {
right?

[Tom Finegan, 2012/09/05 20:15:26]

Done.

+    NOTREACHED();
+    return PP_FALSE;
+  }
 
   HostResource host_resource;
   host_resource.SetHostResource(instance, encrypted_block);
 
   uint32_t size = 0;
   if (DescribeHostBufferResource(encrypted_block, &size) == PP_FALSE)
     return PP_FALSE;
 
   base::SharedMemoryHandle handle;
   if (ShareHostBufferResourceToPlugin(dispatcher,
@@ skipping 20 matching lines @@
 
 PP_Bool DecryptAndDecode(PP_Instance instance,
                          PP_Resource encrypted_block,
                          const PP_EncryptedBlockInfo* encrypted_block_info) {
   HostDispatcher* dispatcher = HostDispatcher::GetForInstance(instance);
   if (!dispatcher) {
     NOTREACHED();
     return PP_FALSE;
   }
 
+  if (AddRefResourceForPlugin(dispatcher, encrypted_block) != PP_TRUE) {
+    NOTREACHED();
+    return PP_FALSE;
+  }
+
   HostResource host_resource;
   host_resource.SetHostResource(instance, encrypted_block);
 
   uint32_t size = 0;
   if (DescribeHostBufferResource(encrypted_block, &size) == PP_FALSE)
     return PP_FALSE;
 
   base::SharedMemoryHandle handle;
   if (ShareHostBufferResourceToPlugin(dispatcher,
                                       encrypted_block,
@@ skipping 117 matching lines @@
                                            encrypted_buffer.handle,
                                            encrypted_buffer.size);
     PP_EncryptedBlockInfo block_info;
     if (!DeserializeBlockInfo(serialized_block_info, &block_info))
       return;
     CallWhileUnlocked(ppp_decryptor_impl_->Decrypt,
                       instance,
                       plugin_resource,
                       const_cast<const PP_EncryptedBlockInfo*>(&block_info));
   }
+
+  // Tell the host to release its reference on the resource. The plugin has
+  // taken ownership.
+  dispatcher()->Send(new PpapiHostMsg_PPBCore_ReleaseResource(
+      API_ID_PPB_CORE, encrypted_buffer.resource));
 }
 
 void PPP_ContentDecryptor_Private_Proxy::OnMsgDecryptAndDecode(
     PP_Instance instance,
     const PPPDecryptor_Buffer& encrypted_buffer,
     const std::string& serialized_block_info) {
   if (ppp_decryptor_impl_) {
     PP_Resource plugin_resource =
         PPB_Buffer_Proxy::AddProxyResource(encrypted_buffer.resource,
                                            encrypted_buffer.handle,
                                            encrypted_buffer.size);
     PP_EncryptedBlockInfo block_info;
     if (!DeserializeBlockInfo(serialized_block_info, &block_info))
       return;
     CallWhileUnlocked(ppp_decryptor_impl_->DecryptAndDecode,
                       instance,
                       plugin_resource,
                       const_cast<const PP_EncryptedBlockInfo*>(&block_info));
   }
+
+  // Tell the host to release its reference on the resource. The plugin has
+  // taken ownership.
+  dispatcher()->Send(new PpapiHostMsg_PPBCore_ReleaseResource(
+      API_ID_PPB_CORE, encrypted_buffer.resource));

[dmichael (off chromium), 2012/09/05 19:46:19]

Sorry, as I mentioned in a more recent comment, I was wrong about this part. The
plugin assumes it gets 1 ref count, and it will release it later when the
plugin-side refcount goes to 0. (same in Decrypt())

[Tom Finegan, 2012/09/05 20:15:26]

Removed.

 }
 
 }  // namespace proxy
 }  // namespace ppapi