Add MediaKeySession.keystatuseschange event

Add MediaKeySession.keysstatuseschange event

Adds support so Chromium can send the event and the set of key
information over to blink.

BUG=432671
TEST=functionality not used yet (not called by Chromium)

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

[jrummell, 2015-01-17 00:12:33]

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

[jrummell, 2015-01-17 00:12:34]

PTAL.

[sandersd (OOO until July 31), 2015-01-17 00:29:01]

lgtm

[ddorwin, 2015-01-17 00:34:56]

LG

https://codereview.chromium.org/858693002/diff/1/Source/core/events/EventType...
File Source/core/events/EventTypeNames.in (right):

https://codereview.chromium.org/858693002/diff/1/Source/core/events/EventType...
Source/core/events/EventTypeNames.in:114: keyschange
Note that this name may change:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=27739
We'll have a final name Tuesday.

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

https://codereview.chromium.org/858693002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:861: WTF_LOG(Media,
"MediaKeySession(%p)::keysChange with %zu keys", this, keys.size());
nit: should log the bool too

https://codereview.chromium.org/858693002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:870: // FIXME: Copy |keys| or
whatever is necessary for keyStatuses.
nit: clarify that this is "the keyStatuses attribute."

https://codereview.chromium.org/858693002/diff/1/Source/platform/exported/Web...
File Source/platform/exported/WebEncryptedMediaKeyInformation.cpp (right):

https://codereview.chromium.org/858693002/diff/1/Source/platform/exported/Web...
Source/platform/exported/WebEncryptedMediaKeyInformation.cpp:25:
WebVector<unsigned char> newId(id, idLength);
Is there a reason you don't use assign() instead of create and swap?

https://codereview.chromium.org/858693002/diff/1/public/platform/WebContentDe...
File public/platform/WebContentDecryptionModuleSession.h (right):

https://codereview.chromium.org/858693002/diff/1/public/platform/WebContentDe...
public/platform/WebContentDecryptionModuleSession.h:65: // Called when the set
of keys for this session change or existing keys
nit: changes - the subject is "set"

https://codereview.chromium.org/858693002/diff/1/public/platform/WebContentDe...
public/platform/WebContentDecryptionModuleSession.h:68: // renewed, etc.) and
the browser should attempt to resume playback.
"... attempt to resume playback if necessary."

https://codereview.chromium.org/858693002/diff/1/public/platform/WebEncrypted...
File public/platform/WebEncryptedMediaKeyInformation.h (right):

https://codereview.chromium.org/858693002/diff/1/public/platform/WebEncrypted...
public/platform/WebEncryptedMediaKeyInformation.h:17: Error,
InternalError

Do you want to match the spec order?
https://w3c.github.io/encrypted-media/#idl-def-MediaKeyStatus

[jrummell, 2015-01-20 19:46:35]

Updated.

https://codereview.chromium.org/858693002/diff/1/Source/core/events/EventType...
File Source/core/events/EventTypeNames.in (right):

https://codereview.chromium.org/858693002/diff/1/Source/core/events/EventType...
Source/core/events/EventTypeNames.in:114: keyschange
On 2015/01/17 00:34:56, ddorwin wrote:
> Note that this name may change:
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=27739
> We'll have a final name Tuesday.

Looks like the new name is "keystatuseschange". Done.

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

https://codereview.chromium.org/858693002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:861: WTF_LOG(Media,
"MediaKeySession(%p)::keysChange with %zu keys", this, keys.size());
On 2015/01/17 00:34:56, ddorwin wrote:
> nit: should log the bool too

Done.

https://codereview.chromium.org/858693002/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:870: // FIXME: Copy |keys| or
whatever is necessary for keyStatuses.
On 2015/01/17 00:34:56, ddorwin wrote:
> nit: clarify that this is "the keyStatuses attribute."

Done.

https://codereview.chromium.org/858693002/diff/1/Source/platform/exported/Web...
File Source/platform/exported/WebEncryptedMediaKeyInformation.cpp (right):

https://codereview.chromium.org/858693002/diff/1/Source/platform/exported/Web...
Source/platform/exported/WebEncryptedMediaKeyInformation.cpp:25:
WebVector<unsigned char> newId(id, idLength);
On 2015/01/17 00:34:56, ddorwin wrote:
> Is there a reason you don't use assign() instead of create and swap?

Done.

https://codereview.chromium.org/858693002/diff/1/public/platform/WebContentDe...
File public/platform/WebContentDecryptionModuleSession.h (right):

https://codereview.chromium.org/858693002/diff/1/public/platform/WebContentDe...
public/platform/WebContentDecryptionModuleSession.h:65: // Called when the set
of keys for this session change or existing keys
On 2015/01/17 00:34:56, ddorwin wrote:
> nit: changes - the subject is "set"

Done.

https://codereview.chromium.org/858693002/diff/1/public/platform/WebContentDe...
public/platform/WebContentDecryptionModuleSession.h:68: // renewed, etc.) and
the browser should attempt to resume playback.
On 2015/01/17 00:34:56, ddorwin wrote:
> "... attempt to resume playback if necessary."

Done.

https://codereview.chromium.org/858693002/diff/1/public/platform/WebEncrypted...
File public/platform/WebEncryptedMediaKeyInformation.h (right):

https://codereview.chromium.org/858693002/diff/1/public/platform/WebEncrypted...
public/platform/WebEncryptedMediaKeyInformation.h:17: Error,
On 2015/01/17 00:34:56, ddorwin wrote:
> InternalError
> 
> Do you want to match the spec order?
> https://w3c.github.io/encrypted-media/#idl-def-MediaKeyStatus

Done.

[ddorwin, 2015-01-20 20:04:16]

LGTM % comments. Thanks.

https://codereview.chromium.org/858693002/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySession.h (right):

https://codereview.chromium.org/858693002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.h:104: virtual void
expirationChanged(double updatedExpiryTimeInMS) override;
"Changed" is the past tense here. Everything else appears to not be past tense,
so we should probably change this one. WDYT?

https://codereview.chromium.org/858693002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.h:105: virtual void
keysStatusesChange(bool hasAdditionalUsableKey, const
WebVector<WebEncryptedMediaKeyInformation>&) override;
nit: I think we should swap the parameter order. The bool is "extra"
information. WDYT?

[jrummell, 2015-01-20 22:06:44]

jrummell@chromium.org changed reviewers:
+ dcheng@chromium.org, haraken@chromium.org, oilpan-reviews@chromium.org

[jrummell, 2015-01-20 22:06:45]

Updated.
+dcheng@ for OWNERS review of public/platform and Source/core/events
+haraken@ for OWNERS review of Source/platform

https://codereview.chromium.org/858693002/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySession.h (right):

https://codereview.chromium.org/858693002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.h:104: virtual void
expirationChanged(double updatedExpiryTimeInMS) override;
On 2015/01/20 20:04:16, ddorwin wrote:
> "Changed" is the past tense here. Everything else appears to not be past
tense,
> so we should probably change this one. WDYT?

Sure. I'll do it as a separate CL since it is already being called.

https://codereview.chromium.org/858693002/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.h:105: virtual void
keysStatusesChange(bool hasAdditionalUsableKey, const
WebVector<WebEncryptedMediaKeyInformation>&) override;
On 2015/01/20 20:04:16, ddorwin wrote:
> nit: I think we should swap the parameter order. The bool is "extra"
> information. WDYT?

Done.

[dcheng, 2015-01-20 22:11:49]

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebConte...
File public/platform/WebContentDecryptionModuleSession.h (right):

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebConte...
public/platform/WebContentDecryptionModuleSession.h:70: virtual void
keysStatusesChange(const WebVector<WebEncryptedMediaKeyInformation>&, bool
hasAdditionalUsableKey) = 0;
Minor nit: it seems unfortunate to have expirationChange|d| and
keyStatusesChange right next to each other.

I don't have any real suggestions here though.

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebEncry...
File public/platform/WebEncryptedMediaKeyInformation.h (right):

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebEncry...
public/platform/WebEncryptedMediaKeyInformation.h:15: enum KeyStatus {
enum class? =)

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebEncry...
public/platform/WebEncryptedMediaKeyInformation.h:25: const WebVector<unsigned
char>& id() const;
WebData?

[jrummell, 2015-01-20 23:03:10]

Updated.

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebConte...
File public/platform/WebContentDecryptionModuleSession.h (right):

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebConte...
public/platform/WebContentDecryptionModuleSession.h:70: virtual void
keysStatusesChange(const WebVector<WebEncryptedMediaKeyInformation>&, bool
hasAdditionalUsableKey) = 0;
On 2015/01/20 22:11:48, dcheng wrote:
> Minor nit: it seems unfortunate to have expirationChange|d| and
> keyStatusesChange right next to each other.
> 
> I don't have any real suggestions here though.

ddorwin mentioned this in PS#2. I'll change it in a seperate CL.

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebEncry...
File public/platform/WebEncryptedMediaKeyInformation.h (right):

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebEncry...
public/platform/WebEncryptedMediaKeyInformation.h:15: enum KeyStatus {
On 2015/01/20 22:11:49, dcheng wrote:
> enum class? =)

Done.

https://codereview.chromium.org/858693002/diff/40001/public/platform/WebEncry...
public/platform/WebEncryptedMediaKeyInformation.h:25: const WebVector<unsigned
char>& id() const;
On 2015/01/20 22:11:49, dcheng wrote:
> WebData?

Done.

[dcheng, 2015-01-20 23:15:31]

lgtm

https://codereview.chromium.org/858693002/diff/60001/public/platform/WebEncry...
File public/platform/WebEncryptedMediaKeyInformation.h (right):

https://codereview.chromium.org/858693002/diff/60001/public/platform/WebEncry...
public/platform/WebEncryptedMediaKeyInformation.h:25: WebData id() const;
I think it's OK to inline all this stuff.

[haraken, 2015-01-21 01:07:46]

Source/platform/ LGTM

[jrummell, 2015-01-21 01:11:43]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2015-01-21 01:13:39]

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

[commit-bot: I haz the power, 2015-01-21 05:09:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-21 05:09:38]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[jrummell, 2015-01-21 17:26:58]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2015-01-21 17:27:14]

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

[commit-bot: I haz the power, 2015-01-21 18:32:27]

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