Hook up CDM calls in CdmWrapper.

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 std::memcpy.
+#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"
 #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/pass_ref.h"
 #include "ppapi/cpp/resource.h"
 #include "ppapi/cpp/var.h"
 #include "ppapi/cpp/var_array_buffer.h"
 #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/content_decryption_module.h"
 
 namespace {
 
-struct DecryptorMessage {
-  DecryptorMessage() : media_error(0), system_code(0) {}
-  std::string key_system;
-  std::string session_id;
-  std::string default_url;
-  std::string message_data;
-  int32_t media_error;
-  int32_t system_code;
-};
-
-struct DecryptedBlock {
-  DecryptedBlock() {
-    std::memset(reinterpret_cast<void*>(&decrypted_block_info),
-                0,
-                sizeof(decrypted_block_info));
-  }
-  std::string decrypted_data;
-  PP_DecryptedBlockInfo decrypted_block_info;
+// This needs to be consistent with media::Decryptor::KeyError!

[Tom Finegan, 2012/08/22 16:54:31]

s/needs to/must/

[dmichael (off chromium), 2012/08/23 18:11:29]

Should these be defined in the API? If they were, you could do a compile assert
somewhere to make sure they stay in sync.

[xhwang, 2012/08/24 22:01:59]

Done.

[xhwang, 2012/08/24 22:01:59]

Good point, added a TODO to add a PP_MediaKeyError enum to avoid the static_cast
here:
http://codereview.chromium.org/10871006/diff/9007/webkit/plugins/ppapi/ppapi_...

+enum KeyError {

[dmichael (off chromium), 2012/08/23 18:11:29]

How about a different name, like KeyErrorCode, to distinguish from the function
of the same name?

[xhwang, 2012/08/24 22:01:59]

Renamed to MediaKeyError to be consistent to the spec.

+  kUnknownError = 1,
+  kClientError,
+  kServiceError,
+  kOutputError,
+  kHardwareChangeError,
+  kDomainError
 };
 
 bool IsMainThread() {
   return pp::Module::Get()->core()->IsMainThread();
 }
 
 void CallOnMain(pp::CompletionCallback cb) {
   // TODO(tomfinegan): This is only necessary because PPAPI doesn't allow calls
   // off the main thread yet. Remove this once the change lands.
   if (IsMainThread())
     cb.Run(PP_OK);
   else
     pp::Module::Get()->core()->CallOnMainThread(0, cb, PP_OK);
 }
 
 }  // namespace
 
+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 pp::Instance,
                    public pp::ContentDecryptor_Private {
  public:
-  CDMWrapper(PP_Instance instance, pp::Module* module);
-  virtual ~CDMWrapper() {}
+  CdmWrapper(PP_Instance instance, pp::Module* module);
+  virtual ~CdmWrapper();
+
+  virtual bool Init(uint32_t argc, const char* argn[], const char* argv[]) {
+    return true;
+  }
 
   // PPP_ContentDecryptor_Private methods
   virtual bool GenerateKeyRequest(const std::string& key_system,
                                   pp::VarArrayBuffer init_data) OVERRIDE;
   virtual bool AddKey(const std::string& session_id,
                       pp::VarArrayBuffer key,
                       pp::VarArrayBuffer init_data) OVERRIDE;
   virtual bool CancelKeyRequest(const std::string& session_id) OVERRIDE;
   virtual bool Decrypt(
       pp::Buffer_Dev encrypted_buffer,
       const PP_EncryptedBlockInfo& encrypted_block_info) OVERRIDE;
-
   virtual bool DecryptAndDecode(
       pp::Buffer_Dev encrypted_buffer,
-      const PP_EncryptedBlockInfo& encrypted_block_info) OVERRIDE {
-    return false;
-  }
-
-  virtual bool Init(uint32_t argc, const char* argn[], const char* argv[]) {
-    return true;
-  }
+      const PP_EncryptedBlockInfo& encrypted_block_info) OVERRIDE;
 
  private:
-  PP_Resource StringToBufferResource(const std::string& str);
+  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 NeedKey(int32_t result, const DecryptorMessage& decryptor_message);
-  void KeyAdded(int32_t result, const DecryptorMessage& decryptor_message);
-  void KeyMessage(int32_t result, const DecryptorMessage& decryptor_message);
-  void KeyError(int32_t result, const DecryptorMessage& decryptor_message);
-  void DeliverBlock(int32_t result, const DecryptedBlock& decrypted_block);
-
-  pp::CompletionCallbackFactory<CDMWrapper> callback_factory_;
+  void KeyAdded(int32_t result, const std::string& session_id);

[ddorwin, 2012/08/22 18:25:54]

result is never used. Do we need it?

[xhwang, 2012/08/24 22:01:59]

Even if we don't use the result, I think this is required if we want to use
CallOnMainThread/PP_CompletionCallback.

+  void KeyMessage(int32_t result, const cdm::KeyMessage& key_message);
+  void KeyError(int32_t result, const std::string& session_id);

[ddorwin, 2012/08/22 18:25:54]

Should have the two error values as parameters too.

[xhwang, 2012/08/24 22:01:59]

For now we really don't need it. If we need it in the future, we can add it any
time.

+  void DeliverBlock(int32_t result,
+                    const cdm::Status& status,
+                    const cdm::OutputBuffer& output_buffer,
+                    const PP_DecryptTrackingInfo& tracking_info);
+
+  pp::CompletionCallbackFactory<CdmWrapper> callback_factory_;
+  cdm::ContentDecryptionModule* cdm_;
+  std::string key_system_;
 };
 
-CDMWrapper::CDMWrapper(PP_Instance instance,
-                       pp::Module* module)
+CdmWrapper::CdmWrapper(PP_Instance instance, pp::Module* module)
     : pp::Instance(instance),
-      pp::ContentDecryptor_Private(this) {
+      pp::ContentDecryptor_Private(this),
+      cdm_(NULL) {
   callback_factory_.Initialize(this);
 }
 
-bool CDMWrapper::GenerateKeyRequest(const std::string& key_system,
+CdmWrapper::~CdmWrapper() {
+  if (cdm_)
+    DestroyCdmInstance(cdm_);
+}
+
+bool CdmWrapper::GenerateKeyRequest(const std::string& key_system,
                                     pp::VarArrayBuffer init_data) {
-  PP_DCHECK(!key_system.empty() && init_data.ByteLength());
-
-  DecryptorMessage decryptor_message;
-  decryptor_message.key_system = key_system;
-  decryptor_message.session_id = "0";
-  decryptor_message.default_url = "http://www.google.com";
-  decryptor_message.message_data = "GenerateKeyRequest";
-
-  CallOnMain(callback_factory_.NewCallback(&CDMWrapper::KeyMessage,
-                                           decryptor_message));
-  return true;
-}
-
-bool CDMWrapper::AddKey(const std::string& session_id,
+  PP_DCHECK(!key_system.empty());
+
+  if (!cdm_) {
+    cdm_ = CreateCdmInstance();
+    if (!cdm_) {
+      CallOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyError,
+                                               std::string()));

[dmichael (off chromium), 2012/08/23 18:11:29]

You don't have to CallOnMainThread for this, do you? You know you're on the main
thread, and all your PPB calls will behave asynchronously.

[xhwang, 2012/08/24 22:01:59]

See reply below.

+      return true;
+    }
+  }
+
+  cdm::KeyMessage key_request;
+  cdm::Status status = cdm_->GenerateKeyRequest(
+      reinterpret_cast<const uint8_t*>(init_data.Map()),
+      init_data.ByteLength(),
+      &key_request);
+
+  if (status != cdm::kSuccess ||
+      !key_request.message ||
+      key_request.message_size == 0) {
+    CallOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyError,

[ddorwin, 2012/08/22 18:25:54]

We need to pass cdm::Status to the error. Eventually, cdm::Status should have
the same values as the EME API.

[dmichael (off chromium), 2012/08/23 18:11:29]

CallOnMain isn't necessary here either (it's probably not necessary anywhere
yet, since you're not calling from background threads yet).

And...  we're probably about a month away from you being able to make PPB calls
off the main thread. Though you'll still have to worry about the thread safety
of your own class, of course.

[xhwang, 2012/08/24 22:01:59]

As per offline discussion with dmichael@:

If running out-of-process, PPB calls will always behave asynchronously since IPC
is involved. In that case, if we are already on main thread, we don't need to
use CallOnMain to help us call PPB call on main thread, or to help call PPB
asynchronously.

For now, we still want to keep in-process working (for easy debugging). So I'll
keep the CallOnMain here and anywhere else to make sure we call PPB
asynchronously.

In the near future, we will support out-of-process only (either because we will
start using SimpleThread to forward Decrypt() calls to another thread and
in-process will stop working, or because pepper team stop supporting
in-process). Then we can drop CallOnMain at least for
GenerateKeyRequest/AddKey/CancelKeyQuest.

Added TODO about the above.

[xhwang, 2012/08/24 22:01:59]

Agreed, added a TODO.

+                                             std::string()));

[Tom Finegan, 2012/08/22 16:54:31]

Seems kind of unfriendly to pass an empty string back up-- even something like
"internal error" might be better.

[ddorwin, 2012/08/22 18:25:54]

it's the session ID, which doesn't exist yet.

+    return true;
+  }
+
+  key_system_ = key_system;
+  CallOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyMessage,
+                                           key_request));
+
+  return true;
+}
+
+bool CdmWrapper::AddKey(const std::string& session_id,
                         pp::VarArrayBuffer key,
                         pp::VarArrayBuffer init_data) {
-  const std::string key_string(reinterpret_cast<char*>(key.Map()),
-                               key.ByteLength());
-  const std::string init_data_string(reinterpret_cast<char*>(init_data.Map()),
-                                     init_data.ByteLength());
-
-  PP_DCHECK(!session_id.empty() && !key_string.empty());
-
-  DecryptorMessage decryptor_message;
-  decryptor_message.key_system = "AddKey";
-  decryptor_message.session_id = "0";
-  decryptor_message.default_url = "http://www.google.com";
-  decryptor_message.message_data = "AddKey";
-  CallOnMain(callback_factory_.NewCallback(&CDMWrapper::KeyAdded,
-                                           decryptor_message));
-  return true;
-}
-
-bool CDMWrapper::CancelKeyRequest(const std::string& session_id) {
-  // TODO(tomfinegan): cancel pending key request in CDM.
-
-  PP_DCHECK(!session_id.empty());
-
-  DecryptorMessage decryptor_message;
-  decryptor_message.key_system = "CancelKeyRequest";
-  decryptor_message.session_id = "0";
-  decryptor_message.default_url = "http://www.google.com";
-  decryptor_message.message_data = "CancelKeyRequest";
-
-  CallOnMain(callback_factory_.NewCallback(&CDMWrapper::KeyMessage,
-                                           decryptor_message));
-  return true;
-}
-
-bool CDMWrapper::Decrypt(pp::Buffer_Dev encrypted_buffer,
+  PP_DCHECK(key.Map() && key.ByteLength() > 0);
+  PP_DCHECK(init_data.Map() && init_data.ByteLength() > 0);

[dmichael (off chromium), 2012/08/23 18:11:29]

Please don't do Map() inside of a DCHECK. It can have non-trivial behavior, and
you don't want it to work differently in debug vs release.

+  PP_DCHECK(cdm_);
+
+  cdm::Status status = cdm_->AddKey(
+      session_id.data(),
+      session_id.size(),

[Tom Finegan, 2012/08/22 16:54:31]

I still think we should get rid of the size param for session ID's, since we can
pretty much guarantee NUL termination. :)

Add a TODO?

[ddorwin, 2012/08/22 18:25:54]

This is consistent with the other parameters, and I think it's better to be
specific, especially with external code.

[dmichael (off chromium), 2012/08/23 18:11:29]

It also makes it easier to use stuff like memcpy or memset without having to
measure first.

+      reinterpret_cast<const uint8_t*>(key.Map()),
+      key.ByteLength(),
+      reinterpret_cast<const uint8_t*>(init_data.Map()),
+      init_data.ByteLength());
+
+  if (status != cdm::kSuccess) {
+    CallOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyError,
+                                             session_id));
+    return true;
+  }
+
+  CallOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyAdded, session_id));
+  return true;
+}
+
+bool CdmWrapper::CancelKeyRequest(const std::string& session_id) {
+  PP_DCHECK(cdm_);
+
+  cdm::Status status = cdm_->CancelKeyRequest(session_id.data(),
+                                              session_id.size());

[dmichael (off chromium), 2012/08/23 18:11:29]

Could maybe be one line?

+
+  if (status != cdm::kSuccess) {
+    CallOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyError,

[dmichael (off chromium), 2012/08/23 18:11:29]

could be one line? (esp. since probably doesn't need to be CallOnMain)

+                                             session_id));
+    return false;
+  }
+
+  // TODO(xhwang): Need to do anything here? Spec says TBD.

[ddorwin, 2012/08/22 18:25:54]

I think this TBD belongs in the CDMs, not here. There is not an event to report.

[xhwang, 2012/08/24 22:01:59]

Done.

+  return true;
+}
+
+bool CdmWrapper::Decrypt(pp::Buffer_Dev encrypted_buffer,
                          const PP_EncryptedBlockInfo& encrypted_block_info) {
   PP_DCHECK(!encrypted_buffer.is_null());
-
-  DecryptedBlock decrypted_block;
-  decrypted_block.decrypted_data = "Pretend I'm decrypted data!";
-  decrypted_block.decrypted_block_info.result = PP_DECRYPTRESULT_SUCCESS;
-  decrypted_block.decrypted_block_info.tracking_info =
-      encrypted_block_info.tracking_info;
-
-  // TODO(tomfinegan): This would end up copying a lot of data in the real
-  // implementation if we continue passing std::strings around. It *might* not
-  // be such a big deal w/a real CDM. We may be able to simply pass a pointer
-  // into the CDM. Otherwise we could look into using std::tr1::shared_ptr
-  // instead of passing a giant std::string filled with encrypted data.
-  CallOnMain(callback_factory_.NewCallback(&CDMWrapper::DeliverBlock,
-                                           decrypted_block));
-  return true;
-}
-
-PP_Resource CDMWrapper::StringToBufferResource(const std::string& str) {
-  if (str.empty())
+  PP_DCHECK(cdm_);
+
+  // TODO(xhwang): Simplify the following data conversion.
+  cdm::InputBuffer input_buffer;
+  input_buffer.data = reinterpret_cast<uint8_t*>(encrypted_buffer.data());
+  input_buffer.data_size = encrypted_buffer.size();
+  input_buffer.data_offset = encrypted_block_info.data_offset;
+  input_buffer.key_id = encrypted_block_info.key_id;
+  input_buffer.key_id_size = encrypted_block_info.key_id_size;
+  input_buffer.iv = encrypted_block_info.iv;
+  input_buffer.iv_size = encrypted_block_info.iv_size;
+  input_buffer.checksum = encrypted_block_info.checksum;
+  input_buffer.checksum_size = encrypted_block_info.checksum_size;
+  input_buffer.num_subsamples = encrypted_block_info.num_subsamples;
+  std::vector<cdm::SubsampleEntry> subsamples;
+  for (uint32_t i = 0; i < encrypted_block_info.num_subsamples; ++i) {
+    subsamples.push_back(cdm::SubsampleEntry(
+        encrypted_block_info.subsamples[i].clear_bytes,
+        encrypted_block_info.subsamples[i].cipher_bytes));
+  }
+  input_buffer.subsamples = &subsamples[0];
+  input_buffer.timestamp = encrypted_block_info.tracking_info.timestamp;
+
+  cdm::OutputBuffer output_buffer;
+  cdm::Status status = cdm_->Decrypt(input_buffer, &output_buffer);

[Tom Finegan, 2012/08/22 16:54:31]

I think we should check this status here, and return false if we already know
there was an error. It doesn't mesh with your current comments in the IDL, but I
think it's better to notify the caller instead hiding the information.

[ddorwin, 2012/08/22 18:25:54]

I've changed my mind (since our conversation) about error reporting after
looking at this. We are hiding CDM errors (i.e. license expired, etc.) because
we end up converting this return value to one of three decrypt results. Right
now, the only way these can be reported in the steady state is via Decrypt
calls. (That might be a problem we should solve with a permanent callback - i.e.
in case playback is paused. Until then we should handle it here.)

What do others think?

[xhwang, 2012/08/24 22:01:59]

The reason I add the comments in the IDL is to avoid double firing of callbacks.
For example, if I return false here, the caller would assume the call didn't
reach the CDM at all and will fire the callback with error. But the DeliverBlock
below will also trigger the callback to be fired. So for this particular case we
can't return false.

[xhwang, 2012/08/24 22:01:59]

As per the spec: 5.1. Encrypted Block Encountered:
- KeyError is only fired "if handler fails to load or initialize".
- For all other errors, we report "MEDIA_ERR_ENCRYPTED".

So even if licence expired and decryption error happened, we should report
MEDIA_ERR_ENCRYPTED by converting the cdm status to a decrypt error code.

We can talk about this more offline.

+
+  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;
+}
+

[Tom Finegan, 2012/08/22 16:54:31]

A comment here that this returns a resource with a ref count of one would be
good-- you can copy/paste the one in ppapi_plugin_instance.cc.

[xhwang, 2012/08/24 22:01:59]

Done.

[ddorwin, 2012/08/27 17:33:54]

Where is it?

[xhwang, 2012/08/27 18:07:26]

http://codereview.chromium.org/10876014/diff/1006/webkit/media/crypto/ppapi/c...

+PP_Resource CdmWrapper::MakeBufferResource(const uint8_t* data,
+                                           uint32_t data_size) {

[dmichael (off chromium), 2012/08/23 18:11:29]

nit: looks like data_size could fit on the previous line, maybe?

[xhwang, 2012/08/24 22:01:59]

Unfortunately it doesn't fit :(

+  if (!data || data_size == 0)
     return 0;
 
-  pp::Buffer_Dev buffer(this, str.size());
+  pp::Buffer_Dev buffer(this, data_size);
   if (!buffer.data())
     return 0;
 
-  std::memcpy(buffer.data(), str.data(), str.size());
+  memcpy(buffer.data(), data, data_size);
+
   return buffer.detach();
 }
 
-void CDMWrapper::NeedKey(int32_t result,
-                         const DecryptorMessage& decryptor_message) {
-  const std::string& message_data = decryptor_message.message_data;
-  pp::VarArrayBuffer init_data(message_data.size());
-  std::memcpy(init_data.Map(), message_data.data(), message_data.size());
-  pp::ContentDecryptor_Private::NeedKey(decryptor_message.key_system,
-                                        decryptor_message.session_id,
-                                        init_data);
-}
-
-void CDMWrapper::KeyAdded(int32_t result,
-                          const DecryptorMessage& decryptor_message) {
-  pp::ContentDecryptor_Private::KeyAdded(decryptor_message.key_system,
-                                         decryptor_message.session_id);
-}
-
-void CDMWrapper::KeyMessage(int32_t result,
-                            const DecryptorMessage& decryptor_message) {
-  pp::Buffer_Dev message_buffer(
-      StringToBufferResource(decryptor_message.message_data));
-  pp::ContentDecryptor_Private::KeyMessage(decryptor_message.key_system,
-                                           decryptor_message.session_id,
-                                           message_buffer,
-                                           decryptor_message.default_url);
-}
-
-void CDMWrapper::KeyError(int32_t result,
-                          const DecryptorMessage& decryptor_message) {
-  pp::ContentDecryptor_Private::KeyError(decryptor_message.key_system,
-                                         decryptor_message.session_id,
-                                         decryptor_message.media_error,
-                                         decryptor_message.system_code);
-}
-
-void CDMWrapper::DeliverBlock(int32_t result,
-                              const DecryptedBlock& decrypted_block) {
-  pp::Buffer_Dev decrypted_buffer(
-      StringToBufferResource(decrypted_block.decrypted_data));
-  pp::ContentDecryptor_Private::DeliverBlock(
-      decrypted_buffer,
-      decrypted_block.decrypted_block_info);
+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,
+                            const cdm::KeyMessage& key_message) {
+  pp::Buffer_Dev message_buffer(MakeBufferResource(key_message.message,
+                                                   key_message.message_size));
+  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 safe as the memory is allocated in

[ddorwin, 2012/08/22 18:25:54]

"not _always_ safe"
It works for now...

[xhwang, 2012/08/24 22:01:59]

Done.

+  // another object.

[Tom Finegan, 2012/08/22 16:54:31]

s/object/object that runs in a different thread/ ?

Just a TODO, but maybe clearer this way...

[ddorwin, 2012/08/22 18:25:54]

It's really a different shared object. I'm sure there is a better name for the
real issue.

[xhwang, 2012/08/24 22:01:59]

Done.

+  delete [] key_message.session_id;
+  delete [] key_message.message;
+  delete [] key_message.default_url;

[dmichael (off chromium), 2012/08/23 18:11:29]

Can you make cdm::KeyMessage smarter or something? This looks ugly and easy to
forget.

[xhwang, 2012/08/24 22:01:59]

We have a plan which is not trivial. The above TODO covers this task. Will fix
this pretty soon.

+}
+
+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,
+                              const cdm::OutputBuffer& output_buffer,

[ddorwin, 2012/08/22 18:25:54]

can't be const if we're deleting a member.

[xhwang, 2012/08/24 22:01:59]

Done.

+                              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;
+
+  if (status == cdm::kSuccess)

[ddorwin, 2012/08/22 18:25:54]

switch statement? esp. useful when we have more errors.
Thought it's TBD whether this conversion should be here or at the caller (see
earlier comment about reporting errors).

[xhwang, 2012/08/24 22:01:59]

Done.

+    decrypted_block_info.result = PP_DECRYPTRESULT_SUCCESS;
+  else if (status == cdm::kErrorNoKey)
+    decrypted_block_info.result = PP_DECRYPTRESULT_DECRYPT_NOKEY;
+  else  // if (status == cdm::kErrorUnknown)
+    decrypted_block_info.result = PP_DECRYPTRESULT_DECRYPT_ERROR;
+
+  pp::ContentDecryptor_Private::DeliverBlock(decrypted_buffer,
+                                             decrypted_block_info);
+
+  // TODO(xhwang): Fix this. This is not safe as the memory is allocated in
+  // another object.
+  delete [] output_buffer.data;

[ddorwin, 2012/08/22 18:25:54]

How do we guarantee output_buffer is not being used by another thread?
.data = NULL since it's a parameter and could be accessed elsewhere.

[xhwang, 2012/08/24 22:01:59]

I am not sure what's the problem? The output_buffer is created in the Decrypt()
call and will only be accessed here afterwards.
Done.

[ddorwin, 2012/08/27 17:33:54]

That answers my question. Would be nice if we could make that clear, but I'm not
sure how.

 }
 
 // This object is the global object representing this plugin library as long
 // as it is loaded.
 class MyModule : public pp::Module {
  public:
   MyModule() : pp::Module() {}
   virtual ~MyModule() {}
 
   virtual pp::Instance* CreateInstance(PP_Instance instance) {
-    return new CDMWrapper(instance, this);
+    return new CdmWrapper(instance, this);
   }
 };
 
+}  // namespace webkit_media
+
 namespace pp {
 
 // Factory function for your specialization of the Module object.
 Module* CreateModule() {
-  return new MyModule();
+  return new webkit_media::MyModule();
 }
 
 }  // namespace pp