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 63078563da7af8d630fa46df8bb90e08ee8679c2..196d8fb406b788187547191ba846aef42bdfcee3 100644
--- a/webkit/media/crypto/ppapi/cdm_wrapper.cc
+++ b/webkit/media/crypto/ppapi/cdm_wrapper.cc
@@ -2,10 +2,12 @@
 // 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 <cstring>
 #include <vector>
 
-#include "base/compiler_specific.h"  // For OVERRIDE.
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/hash_tables.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_stdint.h"
 #include "ppapi/c/private/pp_content_decryptor.h"
@@ -55,6 +57,53 @@ void CallOnMain(pp::CompletionCallback cb) {
 
 namespace webkit_media {
 
+class PpbCdmBuffer : public cdm::Buffer {
+ public:
+  // Constructs an empty buffer.
+  PpbCdmBuffer() : buffer_(NULL), size_(0), id_(0) {}
+
+  // Constructs a buffer object containing all information necessary for a

[ddorwin, 2012/09/12 23:53:01]

This wording is a bit odd, especially such it doesn't allocate any buffer.
Does it make sense for the atual PPAPI calls to occur in here? I guess that
depends on whether we copy these buffers around.

[Tom Finegan, 2012/09/13 10:33:40]

Removed. There's only one constructor now, and hopefully the class comment
suffices.

+  // writable buffer.
+  PpbCdmBuffer(uint8_t* buffer, int32_t size, PP_Resource id)
+      : buffer_(buffer), size_(size), id_(id) {}
+
+  virtual ~PpbCdmBuffer() {}
+
+  uint8_t* buffer() const OVERRIDE { return buffer_; }
+  int32_t size() const OVERRIDE { return size_; }
+  PP_Resource id() const { return id_; }

[ddorwin, 2012/09/12 23:53:01]

nit: Space to separate overrides.

[Tom Finegan, 2012/09/13 10:33:40]

Done.

+
+ private:
+  uint8_t* const buffer_;
+  const int32_t size_;
+  const PP_Resource id_;
+
+  DISALLOW_COPY_AND_ASSIGN(PpbCdmBuffer);
+};
+
+class CdmAllocatorImpl : public cdm::Allocator {
+ public:
+  explicit CdmAllocatorImpl(pp::Instance* instance);
+  virtual ~CdmAllocatorImpl();
+
+  // CdmAllocator methods.
+  // Creates a pp::Buffer_Dev, stores it in |buffer_map_|, and returns the
+  // buffer identifier, pointer, and size wrapped in a cdm::Buffer*. Returns
+  // NULL on failure.

[ddorwin, 2012/09/12 23:53:01]

Caller takes ownership?

[Tom Finegan, 2012/09/13 10:33:40]

Done.

+  virtual cdm::Buffer* Allocate(int32_t size) OVERRIDE;
+
+  // Relinquishes buffer ownership by removing it from |buffer_map_|. The buffer
+  // memory will become invalid after this call returns when it results in a
+  // zero reference count on the pp::Buffer_Dev resource.
+  virtual void ReleaseBuffer(const cdm::Buffer& buffer) OVERRIDE;
+
+ private:
+  typedef base::hash_map<PP_Resource, pp::Buffer_Dev> BufferMap;
+
+  pp::Instance* instance_;
+  BufferMap buffer_map_;

[ddorwin, 2012/09/12 23:53:01]

What do we need this map for? We only insert and remove from it but never look
it up. If PpbCdmBuffer held the Buffer_Dev as a member, then I don't think we'd
need it or the ReleaseBuffer() method.

[Tom Finegan, 2012/09/13 10:33:40]

Removed.

+};
+
 // A wrapper class for abstracting away PPAPI interaction and threading for a
 // Content Decryption Module (CDM).
 class CdmWrapper : public pp::Instance,
@@ -62,7 +111,6 @@ class CdmWrapper : public pp::Instance,
  public:
   CdmWrapper(PP_Instance instance, pp::Module* module);
   virtual ~CdmWrapper();
-
   virtual bool Init(uint32_t argc, const char* argn[], const char* argv[]) {
     return true;
   }
@@ -103,14 +151,43 @@ class CdmWrapper : public pp::Instance,
                     cdm::OutputBuffer& output_buffer,
                     const PP_DecryptTrackingInfo& tracking_info);
 
+  CdmAllocatorImpl allocator_;
   pp::CompletionCallbackFactory<CdmWrapper> callback_factory_;
   cdm::ContentDecryptionModule* cdm_;
   std::string key_system_;
 };
 
+CdmAllocatorImpl::CdmAllocatorImpl(pp::Instance* instance)
+    : instance_(instance) {
+}
+
+CdmAllocatorImpl::~CdmAllocatorImpl() {
+}
+
+cdm::Buffer* CdmAllocatorImpl::Allocate(int32_t size) {
+  PP_DCHECK(size > 0);
+
+  pp::Buffer_Dev buffer(instance_, size);
+  if (buffer.is_null())
+    return NULL;
+
+  if (!buffer_map_.insert(std::make_pair(buffer.pp_resource(), buffer)).second)
+    return NULL;
+
+  return new PpbCdmBuffer(reinterpret_cast<uint8_t*>(buffer.data()),
+                          buffer.size(),
+                          buffer.pp_resource());
+}
+
+void CdmAllocatorImpl::ReleaseBuffer(const cdm::Buffer& buffer) {
+  const PpbCdmBuffer* cdm_buffer = static_cast<const PpbCdmBuffer*>(&buffer);
+  buffer_map_.erase(cdm_buffer->id());
+}
+
 CdmWrapper::CdmWrapper(PP_Instance instance, pp::Module* module)
     : pp::Instance(instance),
       pp::ContentDecryptor_Private(this),
+      allocator_(this),
       cdm_(NULL) {
   callback_factory_.Initialize(this);
 }
@@ -125,7 +202,7 @@ bool CdmWrapper::GenerateKeyRequest(const std::string& key_system,
   PP_DCHECK(!key_system.empty());
 
   if (!cdm_) {
-    cdm_ = CreateCdmInstance();
+    cdm_ = CreateCdmInstance(&allocator_);
     if (!cdm_)
       return false;
   }
@@ -227,6 +304,7 @@ bool CdmWrapper::Decrypt(pp::Buffer_Dev encrypted_buffer,
 
   cdm::OutputBuffer output_buffer;
   cdm::Status status = cdm_->Decrypt(input_buffer, &output_buffer);
+  PP_DCHECK(status == cdm::kSuccess);
 
   CallOnMain(callback_factory_.NewCallback(
       &CdmWrapper::DeliverBlock,
@@ -292,8 +370,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;
@@ -309,21 +385,23 @@ void CdmWrapper::DeliverBlock(int32_t result,
       decrypted_block_info.result = PP_DECRYPTRESULT_DECRYPT_ERROR;
   }
 
-  pp::ContentDecryptor_Private::DeliverBlock(decrypted_buffer,
-                                             decrypted_block_info);
+  const PpbCdmBuffer* const cdm_buffer =
+      static_cast<PpbCdmBuffer*>(output_buffer.buffer);
+  PP_DCHECK(cdm_buffer);
 
-  // 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;
+  pp::ContentDecryptor_Private::DeliverBlock(
+      pp::Buffer_Dev(cdm_buffer->id()),
+      decrypted_block_info);
+  allocator_.ReleaseBuffer(*cdm_buffer);
+  delete cdm_buffer;
 }
 
 // This object is the global object representing this plugin library as long
 // as it is loaded.
-class MyModule : public pp::Module {
+class CdmWrapperModule : public pp::Module {
  public:
-  MyModule() : pp::Module() {}
-  virtual ~MyModule() {}
+  CdmWrapperModule() : pp::Module() {}
+  virtual ~CdmWrapperModule() {}
 
   virtual pp::Instance* CreateInstance(PP_Instance instance) {
     return new CdmWrapper(instance, this);
@@ -336,7 +414,7 @@ namespace pp {
 
 // Factory function for your specialization of the Module object.
 Module* CreateModule() {
-  return new webkit_media::MyModule();
+  return new webkit_media::CdmWrapperModule();
 }
 
 }  // namespace pp