EME: Update tests so 'audioCapabilities' always provided

EME: Update tests so 'audioCapabilities' always provided

EME spec now requires that one of audioCapabilities or
videoCapabilities be specified when calling
requestMediaKeySystemAccess, so update the tests to always
provide audioCapabilities (or both where required). Chrome
does not currently enforce this, but will have to in order
to be spec compliant.

BUG=442566
TEST=updated tests pass

Committed: https://crrev.com/51111720ce9dc84fce16319a239126b2c69ca6da
Cr-Commit-Position: refs/heads/master@{#406687}

[jrummell, 2016-06-21 17:34:13]

jrummell@chromium.org changed reviewers:
+ ddorwin@chromium.org, xhwang@chromium.org

[jrummell, 2016-06-21 17:34:13]

PTAL. The common function is in encrypted-media-utils.js, with the others simply
using the new function. I didn't modify the syntax tests (we can wait until
RMKSA actually changes to change them).

[ddorwin, 2016-06-21 22:41:00]

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html
(right):

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html:23:
return requestClearKeySystemAccessForInitType(initDataType);
Since these tests (in general, not necessarily this one) are intended to test an
API, it seems like a bad idea to abstract away the API call if possible. In
addition, the plan is that the web platform tests will support multiple key
systems (unless the test is explicitly for Clear Key).

I had envisioned a method replacing just the second parameter (`[{}]`). Will
that work? That should also make it easier to replace the keySystem in the WPT
tests and maybe parameterize the config if necessary in some tests.

[ddorwin, 2016-06-21 22:42:48]

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:81:
{ contentType: 'video/webm; codecs="vp8"' }
If we're going to pick one type, let's pick audio. That is more likely than
video-only in practice, and it would allow this tests to run on a audio-only
client. Thoughts?

[jrummell, 2016-06-23 21:54:02]

Description was changed from

==========
EME: Update tests so 'videoCapabilities' always provided

EME spec now requires that one of videoCapabilities or
audioCapabilities be specified when calling
requestMediaKeySystemAccess, so update the tests to provide
videoCapabilities. Chrome does not currently enforce this,
but will have to in order to be spec compliant.

BUG=442566
TEST=updated tests pass
==========

to

==========
EME: Update tests so 'audioCapabilities' always provided

EME spec now requires that one of audioCapabilities or
videoCapabilities be specified when calling
requestMediaKeySystemAccess, so update the tests to always
provide audioCapabilities (or both where required). Chrome
does not currently enforce this, but will have to in order
to be spec compliant.

BUG=442566
TEST=updated tests pass
==========

[jrummell, 2016-06-23 21:56:43]

Patchset #2 (id:20001) has been deleted

[jrummell, 2016-06-23 22:04:21]

Updated to use configuration functions to provide the second parameter.

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html
(right):

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html:23:
return requestClearKeySystemAccessForInitType(initDataType);
On 2016/06/21 22:41:00, ddorwin wrote:
> Since these tests (in general, not necessarily this one) are intended to test
an
> API, it seems like a bad idea to abstract away the API call if possible. In
> addition, the plan is that the web platform tests will support multiple key
> systems (unless the test is explicitly for Clear Key).
> 
> I had envisioned a method replacing just the second parameter (`[{}]`). Will
> that work? That should also make it easier to replace the keySystem in the WPT
> tests and maybe parameterize the config if necessary in some tests.

Done.

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:81:
{ contentType: 'video/webm; codecs="vp8"' }
On 2016/06/21 22:42:47, ddorwin wrote:
> If we're going to pick one type, let's pick audio. That is more likely than
> video-only in practice, and it would allow this tests to run on a audio-only
> client. Thoughts?

The encrypted-media-playback tests play a webm file that has both audio and
video tracks. Switched to 2 functions to get configuration.

[ddorwin, 2016-06-23 22:52:38]

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-setcert-with-gc.html
(right):

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-setcert-with-gc.html:18:
return navigator.requestMediaKeySystemAccess('org.w3.clearkey',
simpleConfigurationForInitDataType(initDataType));
As discussed, if we're not actually using initData in a test, we don't need to
include it in the configuration. That allows us to keep this simple like it was
before.

If these tests are to be used for spec tests where not all user agents support
WebM and/or Vorbis (your chosen simple configuration), such a promise-based
mechanism will be necessary (see also my comments in the .js file.). However,
since this is pretty isolated and does not address that, I think we should keep
it simple.

I think we should just call simpleConfiguration() her and the few other files
that don't need initDataType and aren't already WebM-specific.

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:77:
function simpleConfigurationForInitDataType(initDataType)
// NOTE: Supporting user agents that don't support WebM and/or Vorbis would
require passing in an audio content type. The initDataType is not actually
necessary for most tests.

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:87:
function webmConfiguration()
Just FTR, this would be avConfiguration, with types passed to it.

[jrummell, 2016-06-24 00:08:38]

Patchset #3 (id:60001) has been deleted

[jrummell, 2016-06-24 00:12:29]

Updated.

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-setcert-with-gc.html
(right):

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-setcert-with-gc.html:18:
return navigator.requestMediaKeySystemAccess('org.w3.clearkey',
simpleConfigurationForInitDataType(initDataType));
On 2016/06/23 22:52:38, ddorwin wrote:
> As discussed, if we're not actually using initData in a test, we don't need to
> include it in the configuration. That allows us to keep this simple like it
was
> before.
> 
> If these tests are to be used for spec tests where not all user agents support
> WebM and/or Vorbis (your chosen simple configuration), such a promise-based
> mechanism will be necessary (see also my comments in the .js file.). However,
> since this is pretty isolated and does not address that, I think we should
keep
> it simple.
> 
> I think we should just call simpleConfiguration() her and the few other files
> that don't need initDataType and aren't already WebM-specific.

Done.

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:77:
function simpleConfigurationForInitDataType(initDataType)
On 2016/06/23 22:52:38, ddorwin wrote:
> // NOTE: Supporting user agents that don't support WebM and/or Vorbis would
> require passing in an audio content type. The initDataType is not actually
> necessary for most tests.

Done.

https://codereview.chromium.org/2084053002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:87:
function webmConfiguration()
On 2016/06/23 22:52:38, ddorwin wrote:
> Just FTR, this would be avConfiguration, with types passed to it.

Done this now.

[ddorwin, 2016-06-24 03:05:23]

More thoughts. I'm not sure how generic we want to make the Blink tests. Let's
discuss.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html
(right):

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html:20:
getSupportedInitDataType().then(function(type) {
We don't actually need this. If we ask for all types in the
requestMediaKeySystemAccess() call below, we'll get a list of the supported ones
back in |access|. We can then grab access.initDataTypes[0] at line 25.

This pattern could be used to support all user agents, except for key system
variants, which may require a similar model.

What does that mean here? Well, we don't need this call, though someone might
want to replace it with getSupportedKeySystem(). Line 23 doesn't need take the
IDT and could simply be getSimpleConfigurationWithInitDataType(), which would be
different from getMinimalConfiguration(), though I'm not sure that matters - we
might just have getSimpleConfiguration().

Note: The IDTs are independent of the content types. See the note in
https://w3c.github.io/encrypted-media/#get-supported-configuration-and-consent.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html
(right):

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html:19:
return navigator.requestMediaKeySystemAccess('org.w3.clearkey',
simpleConfigurationForInitDataType(initDataType))
This one definitely needs to specify the IDT. We could getMinimalConfiguration()
then populate this field, though that requires some assumptions about the
implementation of that helper. Alternatively, continue to specify this
explicitly and just add audioCapabilities with a value from the helper function
I propose in the .js file.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:26:
function getSupportedInitDataType()
We may not actually need this anymore if we follow my suggestion. :)

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:30:
audioCapabilities: [ { contentType: 'audio/webm; codecs="vorbis"' } ]
We should add a note that one would need to include an entry for codecs that
cover all user agents.

Alternatively, we could add webm-opus, and mp4-aac (LC)? here and be done with
it. To allow reuse, define a helper that returns the array so we can use it
below too.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:85:
audioCapabilities: [ { contentType: 'audio/webm; codecs="vorbis"' } ]
Actually, that might work here too. We just need one to be supported! No need to
pass them in later.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:104:
function avConfiguration(initDataType)
Passing the IDT isn't strictly correct. Also, FTR, we don't need a type if we're
just trying to pick one. You could just return the configuration and note that
other types would need to be added.

[ddorwin, 2016-06-24 21:24:56]

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:107:
return [ {
(Especially for cases where more than one configurations could be returned) we
should consider adding the `label` member with a description. It doesn't hurt
and it might help debugging.

[jrummell, 2016-06-28 23:04:26]

Updated to automatically determine initDataType. PTAL.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html
(right):

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-async-creation-with-gc.html:20:
getSupportedInitDataType().then(function(type) {
On 2016/06/24 03:05:22, ddorwin wrote:
> We don't actually need this. If we ask for all types in the
> requestMediaKeySystemAccess() call below, we'll get a list of the supported
ones
> back in |access|. We can then grab access.initDataTypes[0] at line 25.
> 
> This pattern could be used to support all user agents, except for key system
> variants, which may require a similar model.
> 
> What does that mean here? Well, we don't need this call, though someone might
> want to replace it with getSupportedKeySystem(). Line 23 doesn't need take the
> IDT and could simply be getSimpleConfigurationWithInitDataType(), which would
be
> different from getMinimalConfiguration(), though I'm not sure that matters -
we
> might just have getSimpleConfiguration().
> 
> Note: The IDTs are independent of the content types. See the note in
>
https://w3c.github.io/encrypted-media/#get-supported-configuration-and-consent.

Done.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html
(right):

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html:19:
return navigator.requestMediaKeySystemAccess('org.w3.clearkey',
simpleConfigurationForInitDataType(initDataType))
On 2016/06/24 03:05:23, ddorwin wrote:
> This one definitely needs to specify the IDT. We could
getMinimalConfiguration()
> then populate this field, though that requires some assumptions about the
> implementation of that helper. Alternatively, continue to specify this
> explicitly and just add audioCapabilities with a value from the helper
function
> I propose in the .js file.

Acknowledged.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:26:
function getSupportedInitDataType()
On 2016/06/24 03:05:23, ddorwin wrote:
> We may not actually need this anymore if we follow my suggestion. :)

Done.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:30:
audioCapabilities: [ { contentType: 'audio/webm; codecs="vorbis"' } ]
On 2016/06/24 03:05:23, ddorwin wrote:
> We should add a note that one would need to include an entry for codecs that
> cover all user agents.
> 
> Alternatively, we could add webm-opus, and mp4-aac (LC)? here and be done with
> it. To allow reuse, define a helper that returns the array so we can use it
> below too.

Done.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:85:
audioCapabilities: [ { contentType: 'audio/webm; codecs="vorbis"' } ]
On 2016/06/24 03:05:23, ddorwin wrote:
> Actually, that might work here too. We just need one to be supported! No need
to
> pass them in later.

Done.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:104:
function avConfiguration(initDataType)
On 2016/06/24 03:05:23, ddorwin wrote:
> Passing the IDT isn't strictly correct. Also, FTR, we don't need a type if
we're
> just trying to pick one. You could just return the configuration and note that
> other types would need to be added.

Renamed the parameter since it is really the media format.

https://codereview.chromium.org/2084053002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:107:
return [ {
On 2016/06/24 21:24:56, ddorwin wrote:
> (Especially for cases where more than one configurations could be returned) we
> should consider adding the `label` member with a description. It doesn't hurt
> and it might help debugging.

Acknowledged. Doesn't look like we need to handle multiple configurations right
now.

[ddorwin, 2016-07-20 17:05:28]

Thank you. LG. Some minor suggestions, some of which we can address separately.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html:37:
return checkInitDataType('cenc');
What happens in Chromium?

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-clearkey-update-non-ascii-input.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-clearkey-update-non-ascii-input.html:39:
initData = getInitData(initDataType);
This could be done at line 44 and we wouldn't need 18. Unless this is a pattern
you want to do consistently.

I guess this is a pattern, though perhaps one we should fix in a separate CL.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html:35:
assert_equals(access.keySystem, 'org.w3.clearkey');
This existing assert seems unnecessary. Presumably we check the attribute
elsewhere.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html:35:
assert_equals(access.keySystem, 'org.w3.clearkey');
ditto

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin.html:39:
'           initDataTypes: [ \'keyids\' ],' +
Should we use the same types as in the .js file?
Ditto below.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:69:
// Returns a trivial MediaKeySystemConfiguration that should includes entries
Trying to make this a bit clearer. Maybe:
... that should be accepted, possibly as a subset of the specified capabilities,
by all user agents.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:71:
function simpleConfiguration()
These three methods are not named as actions like the other methods. Is that
common in these tests? Otherwise, we should probably add "get".

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:80:
// entries that should cover all user agents that support |initDataType|.
Similar text change.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:90:
// both audio and video capabilities.
When the change below is made, add "... for the specified file."

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:91:
function avConfiguration(mediaFormat)
Longer-term (for spec tests), it might make sense to change this to
getConfigurationForFile(file). For now, we could check if the last five
characters are ".webm", but eventually we might need to search for "vp9", etc.
in the filename.

This provides a lot more flexibility and avoids the need to rewrite tests. I
think we can do this separately. If not now, it might make sense to add a note
about this for spec test authors.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:101:
// NOTE: Supporting other mediaFormats not currently implemented as
nit: *is* not...

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:296:
function createMediaKeys(keyId, key)
In a separate CL, we should rename this. It does much more than just create the
MediaKeys.

[jrummell, 2016-07-20 19:07:49]

Updated. Includes rebase to pick up recent changes in
encrypted-media-keystatuses.html.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html:37:
return checkInitDataType('cenc');
On 2016/07/20 17:05:27, ddorwin wrote:
> What happens in Chromium?

isInitDataTypeSupported() returns false, so the test simply resolves the promise
with 'Not supported' (and thus the test passes).

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-clearkey-update-non-ascii-input.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-clearkey-update-non-ascii-input.html:39:
initData = getInitData(initDataType);
On 2016/07/20 17:05:27, ddorwin wrote:
> This could be done at line 44 and we wouldn't need 18. Unless this is a
pattern
> you want to do consistently.
> 
> I guess this is a pattern, though perhaps one we should fix in a separate CL.

Most of the other tests set |initDataType| and |initData| at the same time, so I
tried to use the same pattern everywhere.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html:35:
assert_equals(access.keySystem, 'org.w3.clearkey');
On 2016/07/20 17:05:27, ddorwin wrote:
> This existing assert seems unnecessary. Presumably we check the attribute
> elsewhere.

Removed.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html:35:
assert_equals(access.keySystem, 'org.w3.clearkey');
On 2016/07/20 17:05:27, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin.html:39:
'           initDataTypes: [ \'keyids\' ],' +
On 2016/07/20 17:05:27, ddorwin wrote:
> Should we use the same types as in the .js file?
> Ditto below.

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:69:
// Returns a trivial MediaKeySystemConfiguration that should includes entries
On 2016/07/20 17:05:28, ddorwin wrote:
> Trying to make this a bit clearer. Maybe:
> ... that should be accepted, possibly as a subset of the specified
capabilities,
> by all user agents.

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:71:
function simpleConfiguration()
On 2016/07/20 17:05:28, ddorwin wrote:
> These three methods are not named as actions like the other methods. Is that
> common in these tests? Otherwise, we should probably add "get".

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:80:
// entries that should cover all user agents that support |initDataType|.
On 2016/07/20 17:05:27, ddorwin wrote:
> Similar text change.

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:90:
// both audio and video capabilities.
On 2016/07/20 17:05:27, ddorwin wrote:
> When the change below is made, add "... for the specified file."

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:91:
function avConfiguration(mediaFormat)
On 2016/07/20 17:05:27, ddorwin wrote:
> Longer-term (for spec tests), it might make sense to change this to
> getConfigurationForFile(file). For now, we could check if the last five
> characters are ".webm", but eventually we might need to search for "vp9", etc.
> in the filename.
> 
> This provides a lot more flexibility and avoids the need to rewrite tests. I
> think we can do this separately. If not now, it might make sense to add a note
> about this for spec test authors.

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:101:
// NOTE: Supporting other mediaFormats not currently implemented as
On 2016/07/20 17:05:27, ddorwin wrote:
> nit: *is* not...

Done.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:296:
function createMediaKeys(keyId, key)
On 2016/07/20 17:05:27, ddorwin wrote:
> In a separate CL, we should rename this. It does much more than just create
the
> MediaKeys.

Acknowledged.

[ddorwin, 2016-07-20 19:34:21]

LGTM with some minor suggestions for future improvements.

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html
(right):

https://codereview.chromium.org/2084053002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-check-init-data-type.html:37:
return checkInitDataType('cenc');
On 2016/07/20 19:07:48, jrummell wrote:
> On 2016/07/20 17:05:27, ddorwin wrote:
> > What happens in Chromium?
> 
> isInitDataTypeSupported() returns false, so the test simply resolves the
promise
> with 'Not supported' (and thus the test passes).

Ah, okay. For the WPT, we would want to break this up into separate tests and
fail when not supported. Might even be a good idea for Chromium with some
expectation override

https://codereview.chromium.org/2084053002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-playback-encrypted-and-clear-sources.html
(right):

https://codereview.chromium.org/2084053002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-playback-encrypted-and-clear-sources.html:22:
var unencryptedContent = '../content/test-vp8-vorbis-webvtt.webm';
Any idea why we use this file? It seems we should use test.webm. (This was
changed in WPT.)

https://codereview.chromium.org/2084053002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-playback-encrypted-and-clear-sources.html:101:
// Unencrypted files don't require MediaKeys.
Since we start with clear, we should probably switch the order in the filename.

https://codereview.chromium.org/2084053002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
(right):

https://codereview.chromium.org/2084053002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js:94:
if (mediaFile.toLowerCase().endsWith('webm')) {
It would be slightly better if this was .webm.

[ddorwin, 2016-07-20 19:34:27]

The CQ bit was checked by ddorwin@chromium.org

[commit-bot: I haz the power, 2016-07-20 19:34:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 22:14:19]

Description was changed from

==========
EME: Update tests so 'audioCapabilities' always provided

EME spec now requires that one of audioCapabilities or
videoCapabilities be specified when calling
requestMediaKeySystemAccess, so update the tests to always
provide audioCapabilities (or both where required). Chrome
does not currently enforce this, but will have to in order
to be spec compliant.

BUG=442566
TEST=updated tests pass
==========

to

==========
EME: Update tests so 'audioCapabilities' always provided

EME spec now requires that one of audioCapabilities or
videoCapabilities be specified when calling
requestMediaKeySystemAccess, so update the tests to always
provide audioCapabilities (or both where required). Chrome
does not currently enforce this, but will have to in order
to be spec compliant.

BUG=442566
TEST=updated tests pass
==========

[commit-bot: I haz the power, 2016-07-20 22:14:20]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-07-20 22:15:45]

Description was changed from

==========
EME: Update tests so 'audioCapabilities' always provided

EME spec now requires that one of audioCapabilities or
videoCapabilities be specified when calling
requestMediaKeySystemAccess, so update the tests to always
provide audioCapabilities (or both where required). Chrome
does not currently enforce this, but will have to in order
to be spec compliant.

BUG=442566
TEST=updated tests pass
==========

to

==========
EME: Update tests so 'audioCapabilities' always provided

EME spec now requires that one of audioCapabilities or
videoCapabilities be specified when calling
requestMediaKeySystemAccess, so update the tests to always
provide audioCapabilities (or both where required). Chrome
does not currently enforce this, but will have to in order
to be spec compliant.

BUG=442566
TEST=updated tests pass

Committed: https://crrev.com/51111720ce9dc84fce16319a239126b2c69ca6da
Cr-Commit-Position: refs/heads/master@{#406687}
==========

[commit-bot: I haz the power, 2016-07-20 22:15:46]

Patchset 5 (id:??) landed as
https://crrev.com/51111720ce9dc84fce16319a239126b2c69ca6da
Cr-Commit-Position: refs/heads/master@{#406687}