Filter supported session types in createSession().

Filter supported session types in createSession().

BUG=384152
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190606

[sandersd (OOO until July 31), 2015-02-20 03:20:05]

sandersd@chromium.org changed reviewers:
+ ddorwin@chromium.org

[ddorwin, 2015-02-20 03:42:00]

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeySystemAccess.cpp (right):

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:35:
NewCdmResultPromise(ScriptState* scriptState, const String& keySystem, const
WebMediaKeySystemConfiguration& configuration)
We should just pass the supportedSessionTypes vector.

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeys.cpp (right):

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.cpp:111: //    NotSupportedError.
Something like:
\n
(Chromium ensures the returned supported session types...)

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.cpp:116: bool valid = false;
nit: Replace valid with supported (or even found).

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.cpp:118: if
(m_configuration.sessionTypes[i] == webSessionType) {
Can we just do blink::WebString(sessionType) inline here? Even with an extra
line, this seems better.

[sandersd (OOO until July 31), 2015-02-20 03:47:17]

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeys.cpp (right):

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.cpp:111: //    NotSupportedError.
On 2015/02/20 03:42:00, ddorwin wrote:
> Something like:
> \n
> (Chromium ensures the returned supported session types...)

Done.

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.cpp:116: bool valid = false;
On 2015/02/20 03:42:00, ddorwin wrote:
> nit: Replace valid with supported (or even found).

Done.

https://codereview.chromium.org/946503002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.cpp:118: if
(m_configuration.sessionTypes[i] == webSessionType) {
On 2015/02/20 03:42:00, ddorwin wrote:
> Can we just do blink::WebString(sessionType) inline here? Even with an extra
> line, this seems better.

Done.

[ddorwin, 2015-02-20 04:01:50]

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySystemAccess.cpp (right):

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:128: // 2.5, 2.6 Set use
distinctive identifer from configuration's
identifier

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:129: //    
distinctiveIdentifier.
This seems to be paraphrasing.

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:131: // 2.7, 2.8 Set use
persistent state from configuration's persistentState.
ditto

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:131: // 2.7, 2.8 Set use
persistent state from configuration's persistentState.
(Represented by sessionTypes.)

[sandersd (OOO until July 31), 2015-02-20 04:04:25]

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySystemAccess.cpp (right):

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:128: // 2.5, 2.6 Set use
distinctive identifer from configuration's
On 2015/02/20 04:01:50, ddorwin wrote:
> identifier

Done.

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:129: //    
distinctiveIdentifier.
On 2015/02/20 04:01:50, ddorwin wrote:
> This seems to be paraphrasing.

Added square brackets.

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:131: // 2.7, 2.8 Set use
persistent state from configuration's persistentState.
On 2015/02/20 04:01:50, ddorwin wrote:
> (Represented by sessionTypes.)

Acknowledged.

https://codereview.chromium.org/946503002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySystemAccess.cpp:131: // 2.7, 2.8 Set use
persistent state from configuration's persistentState.
On 2015/02/20 04:01:50, ddorwin wrote:
> ditto

Acknowledged.

[ddorwin, 2015-02-20 20:59:03]

lgtm

[sandersd (OOO until July 31), 2015-02-20 21:25:02]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2015-02-20 21:26:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/946503002/40001

[sandersd (OOO until July 31), 2015-02-20 22:19:13]

New patchsets have been uploaded after l-g-t-m from ddorwin@chromium.org

[ddorwin, 2015-02-20 22:26:58]

https://codereview.chromium.org/946503002/diff/60001/LayoutTests/media/encryp...
File LayoutTests/media/encrypted-media/encrypted-media-syntax.html (right):

https://codereview.chromium.org/946503002/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-syntax.html:757:
assert_equals(error.name, 'InvalidAccessError');
We should at least have tests that we get NotSupportedError when trying to
create persistent-* sessions.

[sandersd (OOO until July 31), 2015-02-20 22:30:51]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2015-02-20 22:31:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/946503002/60001

[commit-bot: I haz the power, 2015-02-21 01:25:12]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190606