EME: Implement Session Closed algorithm

EME: Implement Session Closed algorithm

Update MediaKeySession to implement the Session Closed algorithm
as outlined in the EME spec. This replaces a previous change that
implemented this as part of Clear Key key system only.

BUG=622956, 624192
TEST=updated test passes

Committed: https://crrev.com/beab77818bad0932063375097110e6048632a193
Cr-Commit-Position: refs/heads/master@{#424524}

[jrummell, 2016-10-11 00:49:15]

Description was changed from

==========
EME: Implement Session Closed algorithm

Update MediaKeySession to implement the Session Closed algorithm
as outlined in the EME spec. This replaces a previous change that
implemented this as part of Clear Key key system only.

BUG=622956
TEST=updated test passes
==========

to

==========
EME: Implement Session Closed algorithm

Update MediaKeySession to implement the Session Closed algorithm
as outlined in the EME spec. This replaces a previous change that
implemented this as part of Clear Key key system only.

BUG=622956,624192
TEST=updated test passes
==========

[jrummell, 2016-10-11 00:50:16]

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

[jrummell, 2016-10-11 00:50:16]

PTAL.

[ddorwin, 2016-10-11 01:41:09]

Thanks. LGTM. Let's fix naming at some point.

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-keystatuses.html
(right):

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-keystatuses.html:148:
assert_true(isNaN(mediaKeySession.expiration));
We might need to update the spec test.

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp
(right):

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:843: void
MediaKeySession::close() {
This method is overloaded with the implementation of the close() in the spec. It
should really be closed() or something.

That leaves message() as an outlier. See also the naming comment below. onFoo
style would make this much clearer, but I guess that's not used here.

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:860:
expirationChanged(std::numeric_limits<double>::quiet_NaN());
Unrelated to this CL, it's odd that we have ChangeD here and Change above.

[xhwang, 2016-10-11 17:32:56]

LGTM % ddorwin's comments

[jrummell, 2016-10-11 18:47:35]

Thanks for the reviews.

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-keystatuses.html
(right):

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-keystatuses.html:148:
assert_true(isNaN(mediaKeySession.expiration));
On 2016/10/11 01:41:07, ddorwin wrote:
> We might need to update the spec test.

I wanted to make sure that we verified that this was NaN after the close. But
since clearkey doesn't set expiration, it really doesn't make a difference (it's
always NaN). But in the future we may, so this will catch any regression.

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp
(right):

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:843: void
MediaKeySession::close() {
On 2016/10/11 01:41:07, ddorwin wrote:
> This method is overloaded with the implementation of the close() in the spec.
It
> should really be closed() or something.
> 
> That leaves message() as an outlier. See also the naming comment below. onFoo
> style would make this much clearer, but I guess that's not used here.

Since we're switching blink to Chromium style, we could switch to onXxx. Opened
http://crbug.com/654834.

https://codereview.chromium.org/2406073002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:860:
expirationChanged(std::numeric_limits<double>::quiet_NaN());
On 2016/10/11 01:41:07, ddorwin wrote:
> Unrelated to this CL, it's odd that we have ChangeD here and Change above.

Agreed.

[jrummell, 2016-10-11 18:47:42]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2016-10-11 18:48:16]

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-10-11 20:17:46]

Description was changed from

==========
EME: Implement Session Closed algorithm

Update MediaKeySession to implement the Session Closed algorithm
as outlined in the EME spec. This replaces a previous change that
implemented this as part of Clear Key key system only.

BUG=622956,624192
TEST=updated test passes
==========

to

==========
EME: Implement Session Closed algorithm

Update MediaKeySession to implement the Session Closed algorithm
as outlined in the EME spec. This replaces a previous change that
implemented this as part of Clear Key key system only.

BUG=622956,624192
TEST=updated test passes
==========

[commit-bot: I haz the power, 2016-10-11 20:17:46]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-11 20:21:44]

Description was changed from

==========
EME: Implement Session Closed algorithm

Update MediaKeySession to implement the Session Closed algorithm
as outlined in the EME spec. This replaces a previous change that
implemented this as part of Clear Key key system only.

BUG=622956,624192
TEST=updated test passes
==========

to

==========
EME: Implement Session Closed algorithm

Update MediaKeySession to implement the Session Closed algorithm
as outlined in the EME spec. This replaces a previous change that
implemented this as part of Clear Key key system only.

BUG=622956,624192
TEST=updated test passes

Committed: https://crrev.com/beab77818bad0932063375097110e6048632a193
Cr-Commit-Position: refs/heads/master@{#424524}
==========

[commit-bot: I haz the power, 2016-10-11 20:21:45]

Patchset 1 (id:??) landed as
https://crrev.com/beab77818bad0932063375097110e6048632a193
Cr-Commit-Position: refs/heads/master@{#424524}