CdmAdapter: Allow parallel SendPlatformChallenge() calls.

media/cdm/ppapi/cdm_adapter.cc

 // Copyright 2013 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 "media/cdm/ppapi/cdm_adapter.h"
 
 #include "media/cdm/ppapi/cdm_file_io_impl.h"
 #include "media/cdm/ppapi/cdm_helpers.h"
 #include "media/cdm/ppapi/cdm_logging.h"
 #include "media/cdm/ppapi/supported_cdm_versions.h"
@@ skipping 237 matching lines @@
 }  // namespace
 
 namespace media {
 
 CdmAdapter::CdmAdapter(PP_Instance instance, pp::Module* module)
     : pp::Instance(instance),
       pp::ContentDecryptor_Private(this),
 #if defined(OS_CHROMEOS)
       output_protection_(this),
       platform_verification_(this),
-      challenge_in_progress_(false),
       output_link_mask_(0),
       output_protection_mask_(0),
       query_output_protection_in_progress_(false),
       uma_for_output_protection_query_reported_(false),
       uma_for_output_protection_positive_result_reported_(false),
 #endif
       allocator_(this),
       cdm_(NULL),
       deferred_initialize_audio_decoder_(false),
       deferred_audio_decoder_config_id_(0),
@@ skipping 729 matching lines @@
   const PPB_Console* console = reinterpret_cast<const PPB_Console*>(
       pp::Module::Get()->GetBrowserInterface(PPB_CONSOLE_INTERFACE));
   console->Log(pp_instance(), PP_LOGLEVEL_LOG, value.pp_var());
 }
 #endif  // !defined(NDEBUG)
 
 void CdmAdapter::SendPlatformChallenge(
     const char* service_id, uint32_t service_id_length,
     const char* challenge, uint32_t challenge_length) {
 #if defined(OS_CHROMEOS)
-  PP_DCHECK(!challenge_in_progress_);
-
-  // Ensure member variables set by the callback are in a clean state.
-  signed_data_output_ = pp::Var();
-  signed_data_signature_output_ = pp::Var();
-  platform_key_certificate_output_ = pp::Var();
-
   pp::VarArrayBuffer challenge_var(challenge_length);
   uint8_t* var_data = static_cast<uint8_t*>(challenge_var.Map());
   memcpy(var_data, challenge, challenge_length);
 
   std::string service_id_str(service_id, service_id_length);
+
+  linked_ptr<PepperPlatformChallengeResponse> response(
+      new PepperPlatformChallengeResponse());
+
   int32_t result = platform_verification_.ChallengePlatform(
-      pp::Var(service_id_str), challenge_var, &signed_data_output_,
-      &signed_data_signature_output_, &platform_key_certificate_output_,
-      callback_factory_.NewCallback(&CdmAdapter::SendPlatformChallengeDone));
+      pp::Var(service_id_str),
+      challenge_var,
+      &response->signed_data,
+      &response->signed_data_signature,
+      &response->platform_key_certificate,
+      callback_factory_.NewCallback(&CdmAdapter::SendPlatformChallengeDone,
+                                    response));
   challenge_var.Unmap();
-  if (result == PP_OK_COMPLETIONPENDING) {
-    challenge_in_progress_ = true;
+  if (result == PP_OK_COMPLETIONPENDING)
     return;
-  }
 
   // Fall through on error and issue an empty OnPlatformChallengeResponse().
   PP_DCHECK(result != PP_OK);
 #endif
 
-  cdm::PlatformChallengeResponse response = {};
-  cdm_->OnPlatformChallengeResponse(response);
+  cdm::PlatformChallengeResponse platform_challenge_response = {};

[xhwang, 2014/07/16 00:13:18]

rename because we have a different "response" on l.1013.

+  cdm_->OnPlatformChallengeResponse(platform_challenge_response);
 }
 
 void CdmAdapter::EnableOutputProtection(uint32_t desired_protection_mask) {
 #if defined(OS_CHROMEOS)
   int32_t result = output_protection_.EnableProtection(
       desired_protection_mask, callback_factory_.NewCallback(
           &CdmAdapter::EnableProtectionDone));
 
   // Errors are ignored since clients must call QueryOutputProtectionStatus() to
   // inspect the protection status on a regular basis.
@@ skipping 97 matching lines @@
         OUTPUT_PROTECTION_ALL_EXTERNAL_LINKS_PROTECTED);
     uma_for_output_protection_positive_result_reported_ = true;
     return;
   }
 
   // Do not report a negative result because it could be a false negative.
   // Instead, we will calculate number of negatives using the total number of
   // queries and success results.
 }
 
-void CdmAdapter::SendPlatformChallengeDone(int32_t result) {
-  challenge_in_progress_ = false;
-
+void CdmAdapter::SendPlatformChallengeDone(
+    int32_t result,
+    const linked_ptr<PepperPlatformChallengeResponse>& response) {
   if (result != PP_OK) {
     CDM_DLOG() << __FUNCTION__ << ": Platform challenge failed!";
-    cdm::PlatformChallengeResponse response = {};
-    cdm_->OnPlatformChallengeResponse(response);
+    cdm::PlatformChallengeResponse platform_challenge_response = {};
+    cdm_->OnPlatformChallengeResponse(platform_challenge_response);
     return;
   }
 
-  pp::VarArrayBuffer signed_data_var(signed_data_output_);
-  pp::VarArrayBuffer signed_data_signature_var(signed_data_signature_output_);
+  pp::VarArrayBuffer signed_data_var(response->signed_data);
+  pp::VarArrayBuffer signed_data_signature_var(response->signed_data_signature);
   std::string platform_key_certificate_string =
-      platform_key_certificate_output_.AsString();
+      response->platform_key_certificate.AsString();
 
-  cdm::PlatformChallengeResponse response = {
+  cdm::PlatformChallengeResponse platform_challenge_response = {
       static_cast<uint8_t*>(signed_data_var.Map()),
       signed_data_var.ByteLength(),
       static_cast<uint8_t*>(signed_data_signature_var.Map()),
       signed_data_signature_var.ByteLength(),
       reinterpret_cast<const uint8_t*>(platform_key_certificate_string.data()),
       static_cast<uint32_t>(platform_key_certificate_string.length())};
-  cdm_->OnPlatformChallengeResponse(response);
+  cdm_->OnPlatformChallengeResponse(platform_challenge_response);

[dmichael (off chromium), 2014/07/16 15:21:32]

One thing I just noticed...  OnPlatformChallengeResponse here has to make use of
those buffers immediately, because they will be deleted when this function
(SendPlatformChallengeDone) goes out of scope. That seems correct & appropriate
from an API standpoint, but I wanted to point it out, because it looks like a
behavior change from prior to this CL, so I want to make sure it works with your
CDM code.

[xhwang, 2014/07/16 16:14:31]

Thanks for pointing this out. In all CDM interface calls, the pointer is only
guaranteed to be valid in the scope of that call. CDM has to use it immediately
or copy it for future use. So this should be fine.

 
   signed_data_var.Unmap();
   signed_data_signature_var.Unmap();
 }
 
 void CdmAdapter::EnableProtectionDone(int32_t result) {
   // Does nothing since clients must call QueryOutputProtectionStatus() to
   // inspect the protection status on a regular basis.
   CDM_DLOG() << __FUNCTION__ << " : " << result;
 }
@@ skipping 82 matching lines @@
 }  // namespace media
 
 namespace pp {
 
 // Factory function for your specialization of the Module object.
 Module* CreateModule() {
   return new media::CdmAdapterModule();
 }
 
 }  // namespace pp