Encrypted Media: Handle empty init_data in AesDecryptor::GenerateKeyRequest().

media/crypto/aes_decryptor.cc

Index: media/crypto/aes_decryptor.cc
diff --git a/media/crypto/aes_decryptor.cc b/media/crypto/aes_decryptor.cc
index fb16486104441837edb0b635067279ae88cd952e..81ff92411081aed06cd2777d6a2dd3994eb51318 100644
--- a/media/crypto/aes_decryptor.cc
+++ b/media/crypto/aes_decryptor.cc
@@ -139,6 +139,12 @@ bool AesDecryptor::GenerateKeyRequest(const std::string& key_system,
                                       int init_data_length) {
   std::string session_id_string(base::UintToString(next_session_id_++));
 
+  if (!init_data || !init_data_length) {
+    DVLOG(1) << "Could not generate key request with empty init_data.";

[ddorwin, 2012/11/29 17:55:08]

nit: How about "init_data required to generate a key request." This tells the
reader what they need to do rather than that the code could not do something.

[xhwang, 2012/11/29 20:34:37]

Done.

+    client_->KeyError(key_system, "", Decryptor::kUnknownError, 0);
+    return false;

[ddorwin, 2012/11/29 17:55:08]

Is false going to cause some error to be reported synchronously? What about in
external Clear Key?

[xhwang, 2012/11/29 20:34:37]

We discussed offline that decryptors other than ProxyDecryptor should not return
"false" on GKR(). I looked the code more closely and found returning "false" IS
desired in some cases. For example, in PpapiDecryptor, after it returns false,
it'll assume AddKey() will never be called:
http://code.google.com/searchframe#OAMlx_jo-ck/src/webkit/media/crypto/ppapi_...
If we return true here, we'll have to check |plugin_cdm_delegate_| in a lot of
places (e.g. in AddKey()).

So my gut feeling is that for now we should return MediaKeyException as possible
as we can, and fire KeyError only if we can't return MediaKeyException. (Yes, we
shouldn't have both of then returned in one call.) WDYT?

Ultimately we need to solve this nicely. How about making
PPP::GenerateKeyRequest() a sync message?

[ddorwin, 2012/11/29 22:19:01]

No, I don't think we should do that. Let's file a bug to ensure behavior is the
same in internal and external Clear Key and fix it in the OO API.

Does the user of AesDecryptor ensure that an error is reported (cdm_wrapper in
the external case)?

+  }
+
   // For now, the AesDecryptor does not care about |key_system| and |type|;
   // just fire the event with the |init_data| as the request.
   int message_length = init_data_length;