Implement ClearKey message format as JSON.

Implement ClearKey message format as JSON.

The EME-WD specification now describes the format of the license
request provided to the application via the message attribute of
the message event as being JSON formatted. This change updates
the ClearKey implmentation to return the JSON formatted attribute.

For backwards compatibility with the prefixed EME implementation,
the old format (single key id as Uint8Array) is still used for
any prefixed calls.

BUG=358271
TEST=Updated EME layout tests pass

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290669

[jrummell, 2014-07-29 23:25:31]

PTAL. Changes in https://codereview.chromium.org/429033003/ need to be submitted
first so the tests continue to succeed.

[xhwang, 2014-07-30 05:56:08]

https://codereview.chromium.org/427993002/diff/1/media/cdm/aes_decryptor.cc
File media/cdm/aes_decryptor.cc (right):

https://codereview.chromium.org/427993002/diff/1/media/cdm/aes_decryptor.cc#n...
media/cdm/aes_decryptor.cc:244: // with the |init_data| as the license request.
Is "with the |init_data| as the license request" still accurate? Now it seems
you are creating a license request from the init_data.

https://codereview.chromium.org/427993002/diff/1/media/cdm/aes_decryptor.cc#n...
media/cdm/aes_decryptor.cc:279: SessionType session_type;
initialize |session_type|?

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h
File media/cdm/json_web_key.h (right):

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:19: // following members:
provide ref to the eme spec

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:73: std::vector<uint8>& result);
We prefer to using pointer for output parameters:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Refere...

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:75: // Extract the first key from the license request
message.
From the documentation on l.18-24, a license request message contains info about
key IDs and the type. So what do you mean by "the first key from the license
request message"? From the implementation, it seems that we are extracting the
first key ID in the "kids". If so, how about renaming this to something like
ExtractFirstKeyIdFromLicenseRequest?

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:76: MEDIA_EXPORT bool ExtractLicenseKey(const
std::vector<uint8>& message,
what is the return value?

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:77: std::vector<uint8>& first_key);
ditto about passing pointer instead of ref

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
File media/cdm/json_web_key_unittest.cc (right):

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:45: std::string expected_result) {
pass by const ref

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:52: std::string HexString(const uint8* data,
int length) {
Can you use HexEncode? 

https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:60: void ExtractKeyFromLicenseAndExpect(const
std::string license,
pass by const ref

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:64: std::vector<uint8>
expanded_license(license.begin(), license.end());
what do you mean by "expanded"? If we just convert string to vector, how about
license_vector?

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:69: HexString(key.data(), key.size()));
Why do you need to HexString() and then compare? Can't you just compare two
vectors?

https://codereview.chromium.org/427993002/diff/1/media/test/data/eme_player_j...
File media/test/data/eme_player_js/utils.js (right):

https://codereview.chromium.org/427993002/diff/1/media/test/data/eme_player_j...
media/test/data/eme_player_js/utils.js:98: } catch (o) {
replace "o" with something more readable?

[jrummell, 2014-07-30 18:44:16]

Updated.

https://codereview.chromium.org/427993002/diff/1/media/cdm/aes_decryptor.cc
File media/cdm/aes_decryptor.cc (right):

https://codereview.chromium.org/427993002/diff/1/media/cdm/aes_decryptor.cc#n...
media/cdm/aes_decryptor.cc:244: // with the |init_data| as the license request.
On 2014/07/30 05:56:07, xhwang wrote:
> Is "with the |init_data| as the license request" still accurate? Now it seems
> you are creating a license request from the init_data.

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/aes_decryptor.cc#n...
media/cdm/aes_decryptor.cc:279: SessionType session_type;
On 2014/07/30 05:56:07, xhwang wrote:
> initialize |session_type|?

Not sure it's needed, but done anyway.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h
File media/cdm/json_web_key.h (right):

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:19: // following members:
On 2014/07/30 05:56:07, xhwang wrote:
> provide ref to the eme spec

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:73: std::vector<uint8>& result);
On 2014/07/30 05:56:07, xhwang wrote:
> We prefer to using pointer for output parameters:
> 
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Refere...

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:75: // Extract the first key from the license request
message.
On 2014/07/30 05:56:07, xhwang wrote:
> From the documentation on l.18-24, a license request message contains info
about
> key IDs and the type. So what do you mean by "the first key from the license
> request message"? From the implementation, it seems that we are extracting the
> first key ID in the "kids". If so, how about renaming this to something like
> ExtractFirstKeyIdFromLicenseRequest?

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:76: MEDIA_EXPORT bool ExtractLicenseKey(const
std::vector<uint8>& message,
On 2014/07/30 05:56:07, xhwang wrote:
> what is the return value?

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key.h#new...
media/cdm/json_web_key.h:77: std::vector<uint8>& first_key);
On 2014/07/30 05:56:07, xhwang wrote:
> ditto about passing pointer instead of ref

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
File media/cdm/json_web_key_unittest.cc (right):

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:45: std::string expected_result) {
On 2014/07/30 05:56:08, xhwang wrote:
> pass by const ref

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:52: std::string HexString(const uint8* data,
int length) {
On 2014/07/30 05:56:08, xhwang wrote:
> Can you use HexEncode? 
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...

No longer needed.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:60: void ExtractKeyFromLicenseAndExpect(const
std::string license,
On 2014/07/30 05:56:08, xhwang wrote:
> pass by const ref

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:64: std::vector<uint8>
expanded_license(license.begin(), license.end());
On 2014/07/30 05:56:08, xhwang wrote:
> what do you mean by "expanded"? If we just convert string to vector, how about
> license_vector?

Done.

https://codereview.chromium.org/427993002/diff/1/media/cdm/json_web_key_unitt...
media/cdm/json_web_key_unittest.cc:69: HexString(key.data(), key.size()));
On 2014/07/30 05:56:08, xhwang wrote:
> Why do you need to HexString() and then compare? Can't you just compare two
> vectors?

Didn't realize that EXPECT_EQ displays the vector contents on failure. Done.

https://codereview.chromium.org/427993002/diff/1/media/test/data/eme_player_j...
File media/test/data/eme_player_js/utils.js (right):

https://codereview.chromium.org/427993002/diff/1/media/test/data/eme_player_j...
media/test/data/eme_player_js/utils.js:98: } catch (o) {
On 2014/07/30 05:56:08, xhwang wrote:
> replace "o" with something more readable?

Done.

[xhwang, 2014-08-08 20:31:12]

lgtm % nits

https://codereview.chromium.org/427993002/diff/20001/media/cdm/aes_decryptor.cc
File media/cdm/aes_decryptor.cc (right):

https://codereview.chromium.org/427993002/diff/20001/media/cdm/aes_decryptor....
media/cdm/aes_decryptor.cc:244: // using the |init_data| as the key id in the
license request.
nit: s/id/ID

https://codereview.chromium.org/427993002/diff/20001/media/cdm/json_web_key.cc
File media/cdm/json_web_key.cc (right):

https://codereview.chromium.org/427993002/diff/20001/media/cdm/json_web_key.c...
media/cdm/json_web_key.cc:222: reinterpret_cast<const char*>(license.data()),
license.size());
vector::data() is a c++11 feature which we don't support officially

http://www.cplusplus.com/reference/vector/vector/data/

http://www.chromium.org/developers/coding-style#TOC-C-11

Use the ugly &license[0] instead. Note that when you do this, you need to check
whether the vector is empty.

[jrummell, 2014-08-09 00:14:17]

Updated.

https://codereview.chromium.org/427993002/diff/20001/media/cdm/aes_decryptor.cc
File media/cdm/aes_decryptor.cc (right):

https://codereview.chromium.org/427993002/diff/20001/media/cdm/aes_decryptor....
media/cdm/aes_decryptor.cc:244: // using the |init_data| as the key id in the
license request.
On 2014/08/08 20:31:12, xhwang wrote:
> nit: s/id/ID

Done.

https://codereview.chromium.org/427993002/diff/20001/media/cdm/json_web_key.cc
File media/cdm/json_web_key.cc (right):

https://codereview.chromium.org/427993002/diff/20001/media/cdm/json_web_key.c...
media/cdm/json_web_key.cc:222: reinterpret_cast<const char*>(license.data()),
license.size());
On 2014/08/08 20:31:12, xhwang wrote:
> vector::data() is a c++11 feature which we don't support officially
> 
> http://www.cplusplus.com/reference/vector/vector/data/
> 
> http://www.chromium.org/developers/coding-style#TOC-C-11
> 
> Use the ugly &license[0] instead. Note that when you do this, you need to
check
> whether the vector is empty.

Done.

[jrummell, 2014-08-19 18:39:23]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-08-19 18:40:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jrummell@chromium.org/427993002/80001

[commit-bot: I haz the power, 2014-08-19 20:10:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-19 21:43:44]

Committed patchset #4 (80001) as 290669