CdmAdapter: Allow parallel SendPlatformChallenge() calls.

media/cdm/ppapi/cdm_adapter.cc

Index: media/cdm/ppapi/cdm_adapter.cc
diff --git a/media/cdm/ppapi/cdm_adapter.cc b/media/cdm/ppapi/cdm_adapter.cc
index 8388f449880cf28d0d41486e821d63101546cee4..970318551164c520138d64c785b742932bbaca3e 100644
--- a/media/cdm/ppapi/cdm_adapter.cc
+++ b/media/cdm/ppapi/cdm_adapter.cc
@@ -255,7 +255,6 @@ CdmAdapter::CdmAdapter(PP_Instance instance, pp::Module* module)
 #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),
@@ -1005,13 +1004,6 @@ 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);
@@ -1022,10 +1014,8 @@ void CdmAdapter::SendPlatformChallenge(
       &signed_data_signature_output_, &platform_key_certificate_output_,
       callback_factory_.NewCallback(&CdmAdapter::SendPlatformChallengeDone));

[dmichael (off chromium), 2014/07/15 21:59:52]

Do you know you can bind parameters with the callback factory?

E.g., I think you could in an unnamed namespace define something like:
struct PlatformChallengeResponse {
  pp::Var signed_data_output;
  pp::Var signed_data_signature_output;
  pp::Var key_signature_output;
};
Then say:
 PlatformChallengeResponse* response = new PlatformChallengeResponse();
 callback_factory_.NewCallback(&CdmAdapter::SendPlatformChallengeDone,
                               response);

Then you might have:
void CdmAdapter::SendPlatformChallengeDone(
    int32_t result, PlatformChallengeResponse* response) {
  // Do stuff with response
  delete response;
}

You could eliminate those member variables, and I think it would just look more
obviously correct and more similar to how we would do this in Chromium.

WDYT?

[DaleCurtis, 2014/07/15 22:04:31]

That would be awesome if it works now.  IIRC, at the time of writing I wasn't
able to do this for some reason that I can't remember.

   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);
@@ -1151,8 +1141,6 @@ void CdmAdapter::ReportOutputProtectionQueryResult() {
 }
 
 void CdmAdapter::SendPlatformChallengeDone(int32_t result) {
-  challenge_in_progress_ = false;
-
   if (result != PP_OK) {
     CDM_DLOG() << __FUNCTION__ << ": Platform challenge failed!";
     cdm::PlatformChallengeResponse response = {};
@@ -1176,6 +1164,11 @@ void CdmAdapter::SendPlatformChallengeDone(int32_t result) {
 
   signed_data_var.Unmap();
   signed_data_signature_var.Unmap();
+
+  // Ensure member variables are in a clean state.
+  signed_data_output_ = pp::Var();
+  signed_data_signature_output_ = pp::Var();
+  platform_key_certificate_output_ = pp::Var();

[xhwang, 2014/07/15 01:35:50]

Move this block here so that we keep these variable clean most of the time,
except when we are in the stack of the challenge reply.

 }
 
 void CdmAdapter::EnableProtectionDone(int32_t result) {