Ensure keySystem is always provided in keyerror events from Pepper CDMs.

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>
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
@@ skipping 504 matching lines @@
   virtual void SendKeyMessage(
       const char* session_id, int32_t session_id_length,
       const char* message, int32_t message_length,
       const char* default_url, int32_t default_url_length) OVERRIDE;
   virtual void SendKeyError(const char* session_id,
                             int32_t session_id_length,
                             cdm::MediaKeyError error_code,
                             uint32_t system_code) OVERRIDE;
 
  private:
+  struct SessionInfo {

[xhwang, 2013/01/10 07:16:25]

OOC, did you ask pepper team why we can't add support to 4 parameters?

http://code.google.com/searchframe#OAMlx_jo-ck/src/ppapi/utility/completion_c...

[ddorwin, 2013/01/10 07:39:55]

No. I realize we could add another override, but you have to draw a line
somewhere and putting values in a struct is pretty common for callbacks. (Plus,
this information should eventually be part of a session object anyway.)

+    SessionInfo(const std::string& key_system_in,
+                const std::string& session_id_in)
+        : key_system(key_system_in),
+          session_id(session_id_in) {}
+    const std::string key_system;
+    const std::string session_id;
+  };
+
   typedef linked_ptr<DecryptedBlockImpl> LinkedDecryptedBlock;
   typedef linked_ptr<VideoFrameImpl> LinkedVideoFrame;
   typedef linked_ptr<AudioFramesImpl> LinkedAudioFrames;
 
-  void SendUnknownKeyError(const std::string& session_id);
+  void SendUnknownKeyError(const std::string& key_system,
+                           const std::string& session_id);
 
-  void SendKeyAdded(const std::string& session_id);
+  void SendKeyAdded(const std::string& key_system,
+                    const std::string& session_id);
+
+  void SendKeyErrorInternal(const std::string& key_system,
+                            const std::string& session_id,
+                            cdm::MediaKeyError error_code,
+                            uint32_t system_code);
 
   // <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 KeyAdded(int32_t result, const SessionInfo& session_info);
   void KeyMessage(int32_t result,
-                  const std::string& session_id,
+                  const SessionInfo& session_info,
                   const std::string& message,
                   const std::string& default_url);
   void KeyError(int32_t result,
-                const std::string& session_id,
+                const SessionInfo& session_info,
                 cdm::MediaKeyError error_code,
                 uint32_t system_code);
   void DeliverBlock(int32_t result,
                     const cdm::Status& status,
                     const LinkedDecryptedBlock& decrypted_block,
                     const PP_DecryptTrackingInfo& tracking_info);
   void DecoderInitializeDone(int32_t result,
                              PP_DecryptorStreamType decoder_type,
                              uint32_t request_id,
                              bool success);
@@ skipping 34 matching lines @@
 }
 
 CdmWrapper::~CdmWrapper() {
   if (cdm_)
     DestroyCdmInstance(cdm_);
 }
 
 void CdmWrapper::GenerateKeyRequest(const std::string& key_system,
                                     const std::string& type,
                                     pp::VarArrayBuffer init_data) {
+  PP_DCHECK(!key_system.empty());
   PP_DCHECK(key_system_.empty() || key_system_ == key_system);
 
   if (!cdm_) {
     cdm_ = CreateCdmInstance(key_system.data(), key_system.size(),
                              &allocator_, this);
     PP_DCHECK(cdm_);
     if (!cdm_) {
-      SendUnknownKeyError("");
+      SendUnknownKeyError(key_system, "");
       return;
     }
   }
 
+  // Must be set here in case the CDM synchronously calls a cdm::Host method.
+  // Clear below on error.
+  // TODO(ddorwin): Remove this when key_system is added to cdm::Host methods.
+  key_system_ = key_system;
   cdm::Status status = cdm_->GenerateKeyRequest(
       type.data(), type.size(),
       static_cast<const uint8_t*>(init_data.Map()),
       init_data.ByteLength());
   PP_DCHECK(status == cdm::kSuccess || status == cdm::kSessionError);
-  if (status != cdm::kSuccess)
+  if (status != cdm::kSuccess) {
+    key_system_.clear();  // See comment above.
     return;
+  }
 
   key_system_ = key_system;

[xhwang, 2013/01/10 07:16:25]

do we still need this line?

[ddorwin, 2013/01/10 07:39:55]

No. My intent was to leave the code in working order so that it would still work
when the workaround is removed. Doesn't seem harmful. WDYT?

[xhwang, 2013/01/10 18:21:30]

sg since we already have a TODO to remove line 629.

 }
 
 void CdmWrapper::AddKey(const std::string& session_id,
                         pp::VarArrayBuffer key,
                         pp::VarArrayBuffer init_data) {
   PP_DCHECK(cdm_);  // GenerateKeyRequest() should have succeeded.
   if (!cdm_) {
-    SendUnknownKeyError(session_id);
+    SendUnknownKeyError(key_system_, session_id);
     return;
   }
 
   const uint8_t* key_ptr = static_cast<const uint8_t*>(key.Map());
   int key_size = key.ByteLength();
   const uint8_t* init_data_ptr = static_cast<const uint8_t*>(init_data.Map());
   int init_data_size = init_data.ByteLength();
 
   if (!key_ptr || key_size <= 0 || !init_data_ptr || init_data_size <= 0) {
-    SendUnknownKeyError(session_id);
+    SendUnknownKeyError(key_system_, session_id);
     return;
   }
 
   cdm::Status status = cdm_->AddKey(session_id.data(), session_id.size(),
                                     key_ptr, key_size,
                                     init_data_ptr, init_data_size);
   PP_DCHECK(status == cdm::kSuccess || status == cdm::kSessionError);
   if (status != cdm::kSuccess) {
-    SendUnknownKeyError(session_id);
+    SendUnknownKeyError(key_system_, session_id);
     return;
   }
 
-  SendKeyAdded(session_id);
+  SendKeyAdded(key_system_, session_id);
 }
 
 void CdmWrapper::CancelKeyRequest(const std::string& session_id) {
   PP_DCHECK(cdm_);  // GenerateKeyRequest() should have succeeded.
   if (!cdm_) {
-    SendUnknownKeyError(session_id);
+    SendUnknownKeyError(key_system_, session_id);
     return;
   }
 
   cdm::Status status = cdm_->CancelKeyRequest(session_id.data(),
                                               session_id.size());
   PP_DCHECK(status == cdm::kSuccess || status == cdm::kSessionError);
   if (status != cdm::kSuccess)
-    SendUnknownKeyError(session_id);
+    SendUnknownKeyError(key_system_, session_id);
 }
 
 // Note: In the following decryption/decoding related functions, errors are NOT
 // reported via KeyError, but are reported via corresponding PPB calls.
 
 void CdmWrapper::Decrypt(pp::Buffer_Dev encrypted_buffer,
                          const PP_EncryptedBlockInfo& encrypted_block_info) {
   PP_DCHECK(cdm_);  // GenerateKeyRequest() should have succeeded.
   PP_DCHECK(!encrypted_buffer.is_null());
 
@@ skipping 172 matching lines @@
 }
 
 double CdmWrapper::GetCurrentWallTimeInSeconds() {
   return pp::Module::Get()->core()->GetTime();
 }
 
 void CdmWrapper::SendKeyMessage(
     const char* session_id, int32_t session_id_length,
     const char* message, int32_t message_length,
     const char* default_url, int32_t default_url_length) {
+  PP_DCHECK(!key_system_.empty());
   PostOnMain(callback_factory_.NewCallback(
       &CdmWrapper::KeyMessage,
-      std::string(session_id, session_id_length),
+      SessionInfo(key_system_,
+                  std::string(session_id, session_id_length)),
       std::string(message, message_length),
       std::string(default_url, default_url_length)));
 }
 
 void CdmWrapper::SendKeyError(const char* session_id,
                               int32_t session_id_length,
                               cdm::MediaKeyError error_code,
                               uint32_t system_code) {
-  PostOnMain(callback_factory_.NewCallback(
-      &CdmWrapper::KeyError,
-      std::string(session_id, session_id_length),
-      error_code,
-      system_code));
+  SendKeyErrorInternal(key_system_,
+                       std::string(session_id, session_id_length),
+                       error_code,
+                       system_code);
 }
 
-void CdmWrapper::SendUnknownKeyError(const std::string& session_id) {
-  SendKeyError(session_id.data(), session_id.size(), cdm::kUnknownError, 0);
+void CdmWrapper::SendUnknownKeyError(const std::string& key_system,
+                                     const std::string& session_id) {
+  SendKeyErrorInternal(key_system, session_id, cdm::kUnknownError, 0);
 }
 
-void CdmWrapper::SendKeyAdded(const std::string& session_id) {
-  PostOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyAdded,session_id));
+
+void CdmWrapper::SendKeyAdded(const std::string& key_system,
+                              const std::string& session_id) {
+  PostOnMain(callback_factory_.NewCallback(
+      &CdmWrapper::KeyAdded,
+      SessionInfo(key_system_, session_id)));
 }
 
-void CdmWrapper::KeyAdded(int32_t result, const std::string& session_id) {
+void CdmWrapper::SendKeyErrorInternal(const std::string& key_system,
+                                      const std::string& session_id,
+                                      cdm::MediaKeyError error_code,
+                                      uint32_t system_code) {
+  PP_DCHECK(!key_system.empty());
+  PostOnMain(callback_factory_.NewCallback(&CdmWrapper::KeyError,
+                                           SessionInfo(key_system_, session_id),
+                                           error_code,
+                                           system_code));
+

[xhwang, 2013/01/10 07:16:25]

remove extra line?

[ddorwin, 2013/01/10 07:39:55]

Will do.

[ddorwin, 2013/01/10 18:13:28]

Done.

+}
+
+void CdmWrapper::KeyAdded(int32_t result, const SessionInfo& session_info) {
   PP_DCHECK(result == PP_OK);
-  PP_DCHECK(!key_system_.empty());
-  pp::ContentDecryptor_Private::KeyAdded(key_system_, session_id);
+  PP_DCHECK(!session_info.key_system.empty());
+  pp::ContentDecryptor_Private::KeyAdded(session_info.key_system,
+                                         session_info.session_id);
 }
 
 void CdmWrapper::KeyMessage(int32_t result,
-                            const std::string& session_id,
+                            const SessionInfo& session_info,
                             const std::string& message,
                             const std::string& default_url) {
   PP_DCHECK(result == PP_OK);
+  PP_DCHECK(!session_info.key_system.empty());
 
   pp::VarArrayBuffer message_array_buffer(message.size());
   if (message.size() > 0) {
     memcpy(message_array_buffer.Map(), message.data(), message.size());
   }
 
-  PP_DCHECK(!key_system_.empty());
   pp::ContentDecryptor_Private::KeyMessage(
-      key_system_, session_id, message_array_buffer, default_url);
+      session_info.key_system, session_info.session_id,
+      message_array_buffer, default_url);
 }
 
 void CdmWrapper::KeyError(int32_t result,
-                          const std::string& session_id,
+                          const SessionInfo& session_info,
                           cdm::MediaKeyError error_code,
                           uint32_t system_code) {
   PP_DCHECK(result == PP_OK);
-  PP_DCHECK(!key_system_.empty());
+  PP_DCHECK(!session_info.key_system.empty());
   pp::ContentDecryptor_Private::KeyError(
-      key_system_, session_id, error_code, system_code);
+      session_info.key_system, session_info.session_id,
+      error_code, system_code);
 }
 
 void CdmWrapper::DeliverBlock(int32_t result,
                               const cdm::Status& status,
                               const LinkedDecryptedBlock& decrypted_block,
                               const PP_DecryptTrackingInfo& tracking_info) {
   PP_DCHECK(result == PP_OK);
   PP_DecryptedBlockInfo decrypted_block_info;
   decrypted_block_info.tracking_info = tracking_info;
   decrypted_block_info.tracking_info.timestamp = decrypted_block->Timestamp();
@@ skipping 166 matching lines @@
 }  // namespace webkit_media
 
 namespace pp {
 
 // Factory function for your specialization of the Module object.
 Module* CreateModule() {
   return new webkit_media::CdmWrapperModule();
 }
 
 }  // namespace pp