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 9098065b8632036f726f6e0c0c8b3dc02bf565e0..0988fff9c0fb51f26019948d152cd54f5fe0d5da 100644
--- a/webkit/media/crypto/ppapi/cdm_wrapper.cc
+++ b/webkit/media/crypto/ppapi/cdm_wrapper.cc
@@ -2,10 +2,18 @@
 // 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 <string>
 #include <vector>
 
-#include "base/compiler_specific.h"  // For OVERRIDE.
+#if defined _MSC_VER
+#include <memory>
+#else
+#include <tr1/memory>
+#endif
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_stdint.h"
 #include "ppapi/c/private/pp_content_decryptor.h"
@@ -55,6 +63,181 @@ void CallOnMain(pp::CompletionCallback cb) {
 
 namespace webkit_media {
 
+// Provides access to memory owned by a pp::Buffer_Dev created by
+// PpbBufferAllocator::Allocate(). This class holds a reference to the
+// Buffer_Dev throughout its lifetime.
+class PpbBuffer : public cdm::Buffer {
+ public:
+  // cdm::Buffer methods.
+  uint8_t* buffer() OVERRIDE { return static_cast<uint8_t*>(buffer_.data()); }
+  int32_t size() const OVERRIDE { return buffer_.size(); }
+
+  pp::Buffer_Dev buffer_dev() const { return buffer_; }
+
+ private:
+  explicit PpbBuffer(pp::Buffer_Dev buffer) : buffer_(buffer) {}
+  virtual ~PpbBuffer() {}
+
+  pp::Buffer_Dev buffer_;
+
+  friend class PpbBufferAllocator;
+
+  DISALLOW_COPY_AND_ASSIGN(PpbBuffer);
+};
+
+class PpbBufferAllocator : public cdm::Allocator {
+ public:
+  explicit PpbBufferAllocator(pp::Instance* instance);
+  virtual ~PpbBufferAllocator();
+
+  // cdm::Allocator methods.
+  // Allocates a pp::Buffer_Dev of the specified size and wraps it in a
+  // PpbBuffer, which it returns. The caller own the returned buffer and must
+  // free it by calling ReleaseBuffer(). Returns NULL on failure.
+  virtual cdm::Buffer* Allocate(int32_t size) OVERRIDE;
+
+  // Deletes the cdm::Buffer*.
+  virtual void Release(const cdm::Buffer* buffer) OVERRIDE;

[ddorwin, 2012/09/17 21:19:23]

I don't think the parameter should be const since it is being destroyed.

[Tom Finegan, 2012/09/18 01:08:02]

Done.

+
+ private:
+  pp::Instance* const instance_;
+
+  DISALLOW_COPY_AND_ASSIGN(PpbBufferAllocator);
+};
+
+class KeyMessageImpl : public cdm::KeyMessage {
+ public:
+  explicit KeyMessageImpl(PpbBufferAllocator* allocator);
+  virtual ~KeyMessageImpl();
+
+  // cdm::KeyMessage methods.
+  virtual bool set_session_id(const char* session_id, int32_t length) OVERRIDE;
+  virtual const char* session_id() const OVERRIDE;
+  virtual int32_t session_id_length() const OVERRIDE;
+
+  virtual bool set_message(cdm::Buffer* message) OVERRIDE;
+  virtual cdm::Buffer* message() const OVERRIDE;
+
+  virtual bool set_default_url(const char* default_url,
+                               int32_t length) OVERRIDE;
+  virtual const char* default_url() const OVERRIDE;
+  virtual int32_t default_url_length() const OVERRIDE;
+
+ private:
+  PpbBufferAllocator* const allocator_;
+  PpbBuffer* message_;
+  std::string session_id_;
+  std::string default_url_;
+
+  DISALLOW_COPY_AND_ASSIGN(KeyMessageImpl);
+};
+
+class OutputBufferImpl : public cdm::OutputBuffer {
+ public:
+  explicit OutputBufferImpl(PpbBufferAllocator* allocator);
+  virtual ~OutputBufferImpl();
+
+  virtual bool set_buffer(cdm::Buffer* buffer) OVERRIDE;
+  virtual cdm::Buffer* buffer() const OVERRIDE;
+
+  virtual void set_timestamp(int64_t timestamp) OVERRIDE;
+  virtual int64_t timestamp() const OVERRIDE;
+
+ private:
+  PpbBufferAllocator* const allocator_;
+  PpbBuffer* buffer_;
+  int64_t timestamp_;
+
+  DISALLOW_COPY_AND_ASSIGN(OutputBufferImpl);
+};
+
+KeyMessageImpl::KeyMessageImpl(PpbBufferAllocator* allocator)
+    : allocator_(allocator),
+      message_(NULL) {
+  PP_DCHECK(allocator_);
+}
+
+KeyMessageImpl::~KeyMessageImpl() {
+  if (message_)
+    allocator_->Release(message_);
+}
+
+bool KeyMessageImpl::set_session_id(const char* session_id, int32_t length) {

[ddorwin, 2012/09/17 21:19:23]

The return type for simple setters, such as set_var(), is void. If we need more
logic, it should be SetVar().
I suggest just DCHECKing. CDMs that pass invalid values will crash or have other
problems in production.

[Tom Finegan, 2012/09/18 01:08:02]

Done, but did not add DCHECKs. If the CDM wants to clear a field, maybe it
really needs that...?

+  if (!session_id || length < 0)

[ddorwin, 2012/09/17 21:19:23]

Is 0 okay?

[Tom Finegan, 2012/09/18 01:08:02]

Experience and some quick tests of std::string::assign in combination w/c_str
say yes.

+    return false;
+
+  session_id_.assign(session_id, length);
+  return true;
+}
+
+const char* KeyMessageImpl::session_id() const {
+  return session_id_.data();

[ddorwin, 2012/09/17 21:19:23]

c_str() would probably be safer (null-terminated).

[Tom Finegan, 2012/09/18 01:08:02]

Done.

+}
+
+int32_t KeyMessageImpl::session_id_length() const {
+  return session_id_.length();
+}
+
+bool KeyMessageImpl::set_message(cdm::Buffer* buffer) {
+  if (!buffer)
+    return false;
+
+  message_ = static_cast<PpbBuffer*>(buffer);
+  return true;
+}
+
+cdm::Buffer* KeyMessageImpl::message() const {
+  return message_;
+}
+
+bool KeyMessageImpl::set_default_url(const char* default_url, int32_t length) {
+  if (!default_url || length < 0)

[ddorwin, 2012/09/17 21:19:23]

0 okay?

[Tom Finegan, 2012/09/18 01:08:02]

Done.

+    return false;
+
+  default_url_.assign(default_url, length);
+  return true;
+}
+
+const char* KeyMessageImpl::default_url() const {
+  return default_url_.data();

[ddorwin, 2012/09/17 21:19:23]

c_str()

[Tom Finegan, 2012/09/18 01:08:02]

Done.

+}
+
+int32_t KeyMessageImpl::default_url_length() const {
+  return default_url_.length();
+}
+
+OutputBufferImpl::OutputBufferImpl(PpbBufferAllocator* allocator)
+    : allocator_(allocator),
+      buffer_(NULL),
+      timestamp_(0) {
+  PP_DCHECK(allocator_);
+}
+
+OutputBufferImpl::~OutputBufferImpl() {
+  if (buffer_)
+    allocator_->Release(buffer_);
+}
+
+bool OutputBufferImpl::set_buffer(cdm::Buffer* buffer) {
+  if (!buffer)
+    return false;
+
+  buffer_ = static_cast<PpbBuffer*>(buffer);
+  return true;
+}
+
+cdm::Buffer* OutputBufferImpl::buffer() const {
+  return buffer_;
+}
+
+void OutputBufferImpl::set_timestamp(int64_t timestamp) {
+  timestamp_ = timestamp;
+}
+
+int64_t OutputBufferImpl::timestamp() const {
+  return timestamp_;
+}
+
 // A wrapper class for abstracting away PPAPI interaction and threading for a
 // Content Decryption Module (CDM).
 class CdmWrapper : public pp::Instance,
@@ -62,7 +245,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;
   }
@@ -86,31 +268,51 @@ class CdmWrapper : public pp::Instance,
       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::Buffer_Dev MakeBufferResource(const uint8_t* data, uint32_t data_size);
+  typedef std::tr1::shared_ptr<KeyMessageImpl> SharedKeyMessage;
+  typedef std::tr1::shared_ptr<OutputBufferImpl> SharedOutputBuffer;
 
   // <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 KeyMessage(int32_t result, SharedKeyMessage& message);
   void KeyError(int32_t result, const std::string& session_id);
   void DeliverBlock(int32_t result,
                     const cdm::Status& status,
-                    cdm::OutputBuffer& output_buffer,
+                    SharedOutputBuffer& output_buffer,
                     const PP_DecryptTrackingInfo& tracking_info);
 
+  PpbBufferAllocator allocator_;
   pp::CompletionCallbackFactory<CdmWrapper> callback_factory_;
   cdm::ContentDecryptionModule* cdm_;
   std::string key_system_;
 };
 
+PpbBufferAllocator::PpbBufferAllocator(pp::Instance* instance)
+    : instance_(instance) {
+}
+
+PpbBufferAllocator::~PpbBufferAllocator() {
+}
+
+cdm::Buffer* PpbBufferAllocator::Allocate(int32_t size) {
+  PP_DCHECK(size > 0);
+
+  pp::Buffer_Dev buffer(instance_, size);
+  if (buffer.is_null())
+    return NULL;
+
+  return new PpbBuffer(buffer);
+}
+
+void PpbBufferAllocator::Release(const cdm::Buffer* buffer) {
+  delete buffer;
+}
+
 CdmWrapper::CdmWrapper(PP_Instance instance, pp::Module* module)
     : pp::Instance(instance),
       pp::ContentDecryptor_Private(this),
+      allocator_(this),
       cdm_(NULL) {
   callback_factory_.Initialize(this);
 }
@@ -125,20 +327,20 @@ void CdmWrapper::GenerateKeyRequest(const std::string& key_system,
   PP_DCHECK(!key_system.empty());
 
   if (!cdm_) {
-    cdm_ = CreateCdmInstance();
+    cdm_ = CreateCdmInstance(&allocator_);
     if (!cdm_)
       return;
   }
 
-  cdm::KeyMessage key_request;
+  SharedKeyMessage key_request(new KeyMessageImpl(&allocator_));
   cdm::Status status = cdm_->GenerateKeyRequest(
       reinterpret_cast<const uint8_t*>(init_data.Map()),
       init_data.ByteLength(),
-      &key_request);
+      key_request.get());
 
   if (status != cdm::kSuccess ||
-      !key_request.message ||
-      key_request.message_size == 0) {
+      !key_request->message() ||
+      key_request->message()->size() == 0) {
     CallOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyError,
                                              std::string()));
     return;
@@ -224,8 +426,9 @@ void CdmWrapper::Decrypt(pp::Buffer_Dev encrypted_buffer,
 
   input_buffer.timestamp = encrypted_block_info.tracking_info.timestamp;
 
-  cdm::OutputBuffer output_buffer;
-  cdm::Status status = cdm_->Decrypt(input_buffer, &output_buffer);
+  SharedOutputBuffer output_buffer(new OutputBufferImpl(&allocator_));
+  cdm::Status status = cdm_->Decrypt(input_buffer, output_buffer.get());
+  PP_DCHECK(status == cdm::kSuccess);

[ddorwin, 2012/09/17 21:19:23]

Handle errors including invalid buffer - similar to KeyMessage code.
(DeliverBlock() assumes it's valid.)

[Tom Finegan, 2012/09/18 01:08:02]

I added a PP_DCHECK(output_buffer->buffer()). Should I remove the two DCHECKs,
and send a KeyError instead?

[ddorwin, 2012/09/18 01:26:08]

Yes.

 
   CallOnMain(callback_factory_.NewCallback(
       &CdmWrapper::DeliverBlock,
@@ -239,41 +442,21 @@ void CdmWrapper::DecryptAndDecode(
     const PP_EncryptedBlockInfo& encrypted_block_info) {
 }
 
-pp::Buffer_Dev CdmWrapper::MakeBufferResource(const uint8_t* data,
-                                              uint32_t data_size) {
-  if (!data || !data_size)
-    return pp::Buffer_Dev();
-
-  pp::Buffer_Dev buffer(this, data_size);
-  if (!buffer.data())
-    return pp::Buffer_Dev();
-
-  memcpy(buffer.data(), data, data_size);
-  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));
+                            SharedKeyMessage& key_message) {
+  pp::Buffer_Dev message_buffer =
+      static_cast<const PpbBuffer*>(key_message->message())->buffer_dev();
   pp::ContentDecryptor_Private::KeyMessage(
       key_system_,
-      std::string(key_message.session_id, key_message.session_id_size),
+      std::string(key_message->session_id(),

[ddorwin, 2012/09/17 21:19:23]

SharedKeyMessage can use the Impl type. In that case, you can use the actual
string from the Impl instead of recreating here.

[Tom Finegan, 2012/09/18 01:08:02]

Done.

+                  key_message->session_id_length()),
       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;
+      std::string(key_message->default_url(),
+                  key_message->default_url_length()));
 }
 
 // TODO(xhwang): Support MediaKeyError (see spec: http://goo.gl/rbdnR) in CDM
@@ -287,13 +470,11 @@ void CdmWrapper::KeyError(int32_t result, const std::string& session_id) {
 
 void CdmWrapper::DeliverBlock(int32_t result,
                               const cdm::Status& status,
-                              cdm::OutputBuffer& output_buffer,
+                              SharedOutputBuffer& 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;
+  decrypted_block_info.tracking_info.timestamp = output_buffer->timestamp();
 
   switch (status) {
     case cdm::kSuccess:
@@ -306,21 +487,17 @@ void CdmWrapper::DeliverBlock(int32_t result,
       decrypted_block_info.result = PP_DECRYPTRESULT_DECRYPT_ERROR;
   }
 
-  pp::ContentDecryptor_Private::DeliverBlock(decrypted_buffer,
-                                             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;
+  pp::ContentDecryptor_Private::DeliverBlock(
+      static_cast<PpbBuffer*>(output_buffer->buffer())->buffer_dev(),
+      decrypted_block_info);
 }
 
 // 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);
@@ -333,7 +510,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