EME: Clear Key CDM now generates keystatuseschange event on close()

EME: Clear Key CDM now generates keystatuseschange event on close()

EME spec requires a CDM to generate a keystatuseschange event when
close() is called (as any available keys for that session are now
gone). This updates the Clear Key CDM to generate the required event.

BUG=622956
TEST=updated EME tests passes

Committed: https://crrev.com/7166975fdb70c379be724825394c20c01e606606
Cr-Commit-Position: refs/heads/master@{#403595}

[jrummell, 2016-06-29 00:13:56]

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

[jrummell, 2016-06-29 00:13:57]

PTAL.

[ddorwin, 2016-06-30 06:51:24]

LGTM with comments. Please make the order match the spec.

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

https://codereview.chromium.org/2108033002/diff/1/media/cdm/aes_decryptor.cc#...
media/cdm/aes_decryptor.cc:413: promise->resolve();
This is supposed to be last.
https://w3c.github.io/encrypted-media/#session-closed

Also, everything here and below is supposed to be run in a task to ensure it
happens in the correct order. That's probably a larger issue, though.

For now, move the resolve to the end.

https://codereview.chromium.org/2108033002/diff/1/media/cdm/aes_decryptor.cc#...
media/cdm/aes_decryptor.cc:415: // Update key statuses. All keys are gone, so
it's an empty set.
nit: "gone" sounds informal. I suggest either copy the spec line or say all keys
have been destroyed, so ...

[jrummell, 2016-07-01 00:01:22]

Updated, and a question on switching to PostTask.

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

https://codereview.chromium.org/2108033002/diff/1/media/cdm/aes_decryptor.cc#...
media/cdm/aes_decryptor.cc:413: promise->resolve();
On 2016/06/30 06:51:24, ddorwin wrote:
> This is supposed to be last.
> https://w3c.github.io/encrypted-media/#session-closed
> 
> Also, everything here and below is supposed to be run in a task to ensure it
> happens in the correct order. That's probably a larger issue, though.
> 
> For now, move the resolve to the end.

Agreed. However, since we're not currently using a task, calling the cb's after
resolving the promise is the way it's been done in this file before (see
UpdateSession() directly above).

This could be converted to call PostTask (plus the other calls above). WDYT?

https://codereview.chromium.org/2108033002/diff/1/media/cdm/aes_decryptor.cc#...
media/cdm/aes_decryptor.cc:415: // Update key statuses. All keys are gone, so
it's an empty set.
On 2016/06/30 06:51:24, ddorwin wrote:
> nit: "gone" sounds informal. I suggest either copy the spec line or say all
keys
> have been destroyed, so ...

Done.

[ddorwin, 2016-07-02 00:24:33]

LGTM.

I'm not sure that Chrome tasks guarantee the same behavior as Blink tasks. We'll
need to address the larger issues separately.

[ddorwin, 2016-07-02 00:24:48]

The CQ bit was checked by ddorwin@chromium.org

[commit-bot: I haz the power, 2016-07-02 00:25:03]

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-02 02:39:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-02 02:39:12]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-02 02:41:18]

Description was changed from

==========
EME: Clear Key CDM now generates keystatuseschange event on close()

EME spec requires a CDM to generate a keystatuseschange event when
close() is called (as any available keys for that session are now
gone). This updates the Clear Key CDM to generate the required event.

BUG=622956
TEST=updated EME tests passes
==========

to

==========
EME: Clear Key CDM now generates keystatuseschange event on close()

EME spec requires a CDM to generate a keystatuseschange event when
close() is called (as any available keys for that session are now
gone). This updates the Clear Key CDM to generate the required event.

BUG=622956
TEST=updated EME tests passes

Committed: https://crrev.com/7166975fdb70c379be724825394c20c01e606606
Cr-Commit-Position: refs/heads/master@{#403595}
==========

[commit-bot: I haz the power, 2016-07-02 02:41:18]

Patchset 2 (id:??) landed as
https://crrev.com/7166975fdb70c379be724825394c20c01e606606
Cr-Commit-Position: refs/heads/master@{#403595}