Add CDM allocator interface.

webkit/media/crypto/ppapi/cdm_wrapper.cc

Index: webkit/media/crypto/ppapi/cdm_wrapper.cc
diff --git a/webkit/media/crypto/ppapi/cdm_wrapper.cc b/webkit/media/crypto/ppapi/cdm_wrapper.cc
index 88baa44062b74920ed0ee272db53ec8c29824e7b..941360910a6aee688ff9b96e0b9a3a578f803c71 100644
--- a/webkit/media/crypto/ppapi/cdm_wrapper.cc
+++ b/webkit/media/crypto/ppapi/cdm_wrapper.cc
@@ -8,12 +8,14 @@
 #include "base/compiler_specific.h"  // For OVERRIDE.
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_stdint.h"
+#include "ppapi/c/dev/ppb_buffer_dev.h"
 #include "ppapi/c/private/pp_content_decryptor.h"
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/core.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/logging.h"
 #include "ppapi/cpp/module.h"
+#include "ppapi/cpp/module_impl.h"

[Tom Finegan, 2012/09/01 05:00:09]

I might not need this-- was trying to use pp::get_interface<PPB_Buffer_Dev>() to
get at the PPB_Buffer_Dev C interface, but that wasn't working. Maybe I should
have been passing PPB_BUFFER_DEV_INTERFACE. Using
pp::Module::Get()->GetBrowserInterface solved the problem, anyway.

 #include "ppapi/cpp/pass_ref.h"
 #include "ppapi/cpp/resource.h"
 #include "ppapi/cpp/var.h"
@@ -21,6 +23,7 @@
 #include "ppapi/cpp/dev/buffer_dev.h"
 #include "ppapi/cpp/private/content_decryptor_private.h"
 #include "ppapi/utility/completion_callback_factory.h"
+#include "webkit/media/crypto/ppapi/cdm_allocator.h"
 #include "webkit/media/crypto/ppapi/content_decryption_module.h"
 
 namespace {
@@ -57,7 +60,8 @@ namespace webkit_media {
 
 // A wrapper class for abstracting away PPAPI interaction and threading for a
 // Content Decryption Module (CDM).
-class CdmWrapper : public pp::Instance,
+class CdmWrapper : public CdmAllocatorInterface,

[xhwang, 2012/09/01 13:49:09]

Instead of making the CdmWrapper a CdmAllocator, how about making a new class in
this file named PpbBufferCdmAllocator?

In this CL, CdmWrapper::Allocate() doesn't use anything in CdmWrapper and does
not need to be a class method of CdmWrapper.

[Tom Finegan, 2012/09/01 21:32:49]

It uses pp_instance(), and buffer_iface_ is now a CdmWrapper member.

[ddorwin, 2012/09/04 09:53:15]

Pass it into the constructor. Also, see my comment in the CdmAllocator file.
It could just as easily be a member of the new class.

+                   public pp::Instance,
                    public pp::ContentDecryptor_Private {
  public:
   CdmWrapper(PP_Instance instance, pp::Module* module);
@@ -85,6 +89,8 @@ class CdmWrapper : public pp::Instance,
       pp::Buffer_Dev encrypted_buffer,
       const PP_EncryptedBlockInfo& encrypted_block_info) OVERRIDE;
 
+  uint8_t* Allocate(int32_t size, int32_t* id) OVERRIDE;

[xhwang, 2012/09/01 13:49:09]

Add void Deallocate(uint8_t*, int32_t id)

[Tom Finegan, 2012/09/01 21:32:49]

See comments in cdm_allocator.h

+
  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
@@ -128,6 +134,8 @@ bool CdmWrapper::GenerateKeyRequest(const std::string& key_system,
     cdm_ = CreateCdmInstance();
     if (!cdm_)
       return false;
+    if (!cdm_->Initialize(this))

[xhwang, 2012/09/01 13:49:09]

Merge these two if's to: if (!cdm_ || !cdm_->Initialize(...))

[Tom Finegan, 2012/09/01 21:32:49]

Done.

+      return false;
   }
 
   cdm::KeyMessage key_request;
@@ -243,6 +251,26 @@ bool CdmWrapper::DecryptAndDecode(
   return false;
 }
 
+// This method uses the C buffer interface (PPB_Buffer_Dev) because callers
+// require that the buffer is mapped.
+uint8_t* CdmWrapper::Allocate(int32_t size, int32_t* id) {
+  PP_DCHECK(size > 0);
+  PP_DCHECK(id);
+
+  const PPB_Buffer_Dev* buffer_iface = static_cast<const PPB_Buffer_Dev*>(
+      pp::Module::Get()->GetBrowserInterface(PPB_BUFFER_DEV_INTERFACE));
+  PP_DCHECK(buffer_iface);

[Tom Finegan, 2012/09/01 05:00:09]

I'll move this to Init and make buffer_iface a member; doing this every time we
allocate a buffer is unnecessary.

[Tom Finegan, 2012/09/01 21:32:49]

Done.

+
+  PP_Resource buffer_resource = buffer_iface->Create(pp_instance(), size);
+  PP_DCHECK(buffer_resource);
+  PP_DCHECK(id);
+
+  void* buffer = buffer_iface->Map(buffer_resource);
+  *id = buffer_resource;
+
+  return reinterpret_cast<uint8_t*>(buffer);
+}
+
 PP_Resource CdmWrapper::MakeBufferResource(const uint8_t* data,
                                            uint32_t data_size) {
   if (!data || !data_size)
@@ -294,9 +322,6 @@ 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_DecryptedBlockInfo decrypted_block_info;
   decrypted_block_info.tracking_info.request_id = tracking_info.request_id;
   decrypted_block_info.tracking_info.timestamp = output_buffer.timestamp;
@@ -312,12 +337,8 @@ void CdmWrapper::DeliverBlock(int32_t result,
       decrypted_block_info.result = PP_DECRYPTRESULT_DECRYPT_ERROR;
   }
 
-  pp::ContentDecryptor_Private::DeliverBlock(decrypted_buffer,
+  pp::ContentDecryptor_Private::DeliverBlock(output_buffer.buffer_id,
                                              decrypted_block_info);
-
-  // TODO(xhwang): Fix this. This is not always safe as the memory is allocated
-  // in another shared object.
-  delete [] output_buffer.data;
   output_buffer.data = NULL;
 }