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);
   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));

[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);
 #endif
 
   cdm::PlatformChallengeResponse response = {};
   cdm_->OnPlatformChallengeResponse(response);
 }
 
 void CdmAdapter::EnableOutputProtection(uint32_t desired_protection_mask) {
@@ skipping 105 matching lines @@
     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;
-
   if (result != PP_OK) {
     CDM_DLOG() << __FUNCTION__ << ": Platform challenge failed!";
     cdm::PlatformChallengeResponse response = {};
     cdm_->OnPlatformChallengeResponse(response);
     return;
   }
 
   pp::VarArrayBuffer signed_data_var(signed_data_output_);
   pp::VarArrayBuffer signed_data_signature_var(signed_data_signature_output_);
   std::string platform_key_certificate_string =
       platform_key_certificate_output_.AsString();
 
   cdm::PlatformChallengeResponse 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);
 
   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) {
   // Does nothing since clients must call QueryOutputProtectionStatus() to
   // inspect the protection status on a regular basis.
   CDM_DLOG() << __FUNCTION__ << " : " << result;
 }
 
 void CdmAdapter::QueryOutputProtectionStatusDone(int32_t result) {
   PP_DCHECK(query_output_protection_in_progress_);
@@ skipping 79 matching lines @@
 }  // namespace media
 
 namespace pp {
 
 // Factory function for your specialization of the Module object.
 Module* CreateModule() {
   return new media::CdmAdapterModule();
 }
 
 }  // namespace pp