Rename onneedkey event to onencrypted.

Remove "needkey" event and add it's replacement, "encrypted".

This new event is of type MediaEncryptedEvent, which has initDataType instead of contentType (but this CL just renames one to the other, without translation), and changes the type of initData from Uint8Array to ArrayBuffer as per the WD EME spec.

Also in this CL:
  - Start renaming keyNeeded() to encrypted() by renaming the it on HTMLMediaElementEncryptedMedia.
  - Remove unused contentType argument from createWebkitNeedKeyEvent() (good thing it's unused or the contentType to initDataType change would be a lot harder).
  - Fix prefixed/encrypted-media-events.html test to test the prefixed needkey event instead of the unprefixed one.

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

[sandersd (OOO until July 31), 2014-09-26 21:39:00]

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

[jrummell, 2014-09-26 22:16:23]

lgtm w/nits.

https://codereview.chromium.org/600143005/diff/1/LayoutTests/media/encrypted-...
File LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html
(right):

https://codereview.chromium.org/600143005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html:17: var
keyNeededEvent = document.createEvent("MediaEncryptedEvent");
s/keyNeededEvent/encryptedEvent/ ?

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

https://codereview.chromium.org/600143005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:506:
WTF_LOG(Media, "HTMLMediaElementEncryptedMedia::mediaPlayerKeyNeeded:
initDataType=%s", initDataType.utf8().data());
Method name in message doesn't match.

https://codereview.chromium.org/600143005/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaEncryptedEvent.idl (right):

https://codereview.chromium.org/600143005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaEncryptedEvent.idl:2: * Copyright (C) 2012
Google Inc.  All rights reserved.
I'm not sure if these are considered new files, and thus need the simplified
copyright header (http://www.chromium.org/blink/coding-style#TOC-License). If
not, then you probably shouldn't change the copyright date in the other files.

https://codereview.chromium.org/600143005/diff/1/Source/modules/modules.gypi
File Source/modules/modules.gypi (right):

https://codereview.chromium.org/600143005/diff/1/Source/modules/modules.gypi#...
Source/modules/modules.gypi:33: 'encryptedmedia/MediaEncryptedEvent.idl',
Move up a line since I think they should be alphabetical.

[ddorwin, 2014-09-26 22:42:17]

As discussed, the Chromium browser tests need to be updated to handle both event
names during the transtion.

Please update the description with something like:
Rename "needkey" event to "encrypted".
All lower case and without the attribute prefix "on".

[sandersd (OOO until July 31), 2014-09-26 22:49:22]

https://codereview.chromium.org/600143005/diff/1/LayoutTests/media/encrypted-...
File LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html
(right):

https://codereview.chromium.org/600143005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html:17: var
keyNeededEvent = document.createEvent("MediaEncryptedEvent");
On 2014/09/26 22:16:23, jrummell wrote:
> s/keyNeededEvent/encryptedEvent/ ?

Done.

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

https://codereview.chromium.org/600143005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:506:
WTF_LOG(Media, "HTMLMediaElementEncryptedMedia::mediaPlayerKeyNeeded:
initDataType=%s", initDataType.utf8().data());
On 2014/09/26 22:16:23, jrummell wrote:
> Method name in message doesn't match.

Done.

https://codereview.chromium.org/600143005/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaEncryptedEvent.idl (right):

https://codereview.chromium.org/600143005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaEncryptedEvent.idl:2: * Copyright (C) 2012
Google Inc.  All rights reserved.
On 2014/09/26 22:16:23, jrummell wrote:
> I'm not sure if these are considered new files, and thus need the simplified
> copyright header (http://www.chromium.org/blink/coding-style#TOC-License). If
> not, then you probably shouldn't change the copyright date in the other files.

Done.

https://codereview.chromium.org/600143005/diff/1/Source/modules/modules.gypi
File Source/modules/modules.gypi (right):

https://codereview.chromium.org/600143005/diff/1/Source/modules/modules.gypi#...
Source/modules/modules.gypi:33: 'encryptedmedia/MediaEncryptedEvent.idl',
On 2014/09/26 22:16:23, jrummell wrote:
> Move up a line since I think they should be alphabetical.

Done.

[sandersd (OOO until July 31), 2014-09-29 23:03:18]

On 2014/09/26 22:42:17, ddorwin wrote:
> As discussed, the Chromium browser tests need to be updated to handle both
event
> names during the transtion.
> 
> Please update the description with something like:
> Rename "needkey" event to "encrypted".
> All lower case and without the attribute prefix "on".

Ping, tests should now support both.

[ddorwin, 2014-09-29 23:13:09]

ddorwin@chromium.org changed reviewers:
+ wolenetz@chromium.org
- ddorwin@chromium.org

[ddorwin, 2014-09-29 23:13:10]

Adding wolenetz and removing myself.

[ddorwin, 2014-09-29 23:26:41]

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

[ddorwin, 2014-09-29 23:26:42]

I reviewed Source/. modules/encryptedmedia LG % comment.

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

https://codereview.chromium.org/600143005/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:504: void
HTMLMediaElementEncryptedMedia::keyNeeded(HTMLMediaElement& element, const
String& initDataType, const unsigned char* initData, unsigned initDataLength)
We should rename this method. That involves the public API, which will involve
multiple steps. You might as well start the process now.

[sandersd (OOO until July 31), 2014-09-30 19:14:03]

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

https://codereview.chromium.org/600143005/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:504: void
HTMLMediaElementEncryptedMedia::keyNeeded(HTMLMediaElement& element, const
String& initDataType, const unsigned char* initData, unsigned initDataLength)
On 2014/09/29 23:26:41, ddorwin wrote:
> We should rename this method. That involves the public API, which will involve
> multiple steps. You might as well start the process now.

Done.

[Mike West, 2014-09-30 19:27:10]

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org

[Mike West, 2014-09-30 19:27:11]

One quick drive-by comment on public/.

https://codereview.chromium.org/600143005/diff/40001/public/platform/WebMedia...
File public/platform/WebMediaPlayerClient.h (right):

https://codereview.chromium.org/600143005/diff/40001/public/platform/WebMedia...
public/platform/WebMediaPlayerClient.h:87: virtual void keyNeeded(const
WebString& contentType, const unsigned char* initData, unsigned initDataLength)
OVERRIDE;
What is this overriding? WebMediaPlayerClient doesn't subclass anything.

[sandersd (OOO until July 31), 2014-09-30 19:39:13]

https://codereview.chromium.org/600143005/diff/40001/public/platform/WebMedia...
File public/platform/WebMediaPlayerClient.h (right):

https://codereview.chromium.org/600143005/diff/40001/public/platform/WebMedia...
public/platform/WebMediaPlayerClient.h:87: virtual void keyNeeded(const
WebString& contentType, const unsigned char* initData, unsigned initDataLength)
OVERRIDE;
On 2014/09/30 19:27:11, Mike West wrote:
> What is this overriding? WebMediaPlayerClient doesn't subclass anything.

Whoops, C&P error, thanks!

[wolenetz, 2014-10-02 00:15:21]

Looks pretty good: I mostly have just noob-to-eme questions (and nits that are
mostly confirming my understanding). I think the CL description could use an
update to describe a little better what this CL does beyond just a rename of the
event (other things are renamed + impl change to ArrayBuffer + prefixed API is
possibly impacted):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html:38:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
nit: this seems more than just a rename. Update CL description?

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

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:14: var
expectedInitData = stringToUint8Array('0123456789012345');
nit: test-encrypted.webm's expected metadata is used in more than one place.
Extract to encrypted-media-utils.js? (perhaps better in a separate CL)

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:28:
assert_equals(event.initDataType, 'webm');
eme noob question: why not also check contentType?

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:29:
assert_array_equals(new Uint8Array(event.initData), expectedInitData);
eme noob question: why now wrapping in a Uint8Array instance?

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:31: if
(--expectedEvents == 0)
nit: this affirms only that at least 2 events were dispatched. what if more are
incorrectly dispatched? (perhaps better in a separate CL, or maybe this is
already checked in some other layout test and this is a noob nit ;) )

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File
LayoutTests/media/encrypted-media/encrypted-media-playback-multiple-sessions.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-multiple-sessions.html:32:
var keyId = String.fromCharCode.apply(null, new Uint8Array(event.initData));
ditto why wrap?

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-multiple-sessions.html:36:
mediaKeySession.generateRequest(event.initDataType,
event.initData).then(function() {
ditto why change from contenttype to initdatatype (seems more than just rename
needkey->encrypted)

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-after-src.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-after-src.html:20:
var rawKey = new Uint8Array([0xeb, 0xdd, 0x62, 0xf1, 0x68, 0x14, 0xd2, 0x7b,
nit ditto: metadata definition copied in lots of places instead of in a common
include.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-after-src.html:37:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
ditto: more than a rename

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-before-src.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-before-src.html:20:
var rawKey = new Uint8Array([0xeb, 0xdd, 0x62, 0xf1, 0x68, 0x14, 0xd2, 0x7b,
ditto: metadata

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-before-src.html:36:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
ditto: more than a rename

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File LayoutTests/media/encrypted-media/encrypted-media-waiting-for-a-key.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-waiting-for-a-key.html:21: var
rawKey = new Uint8Array([0xeb, 0xdd, 0x62, 0xf1, 0x68, 0x14, 0xd2, 0x7b,
ditto: metadata

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-waiting-for-a-key.html:38:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
ditto: more than a rename

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html:17: var
keyNeededEvent = document.createEvent("MediaKeyEvent");
nit: this seems more than just a rename, and it appears this change impacts
prefixed API. Is this intended?

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html:143:
function needKey(event)
eme noob question: Is this correct across the rename, since the rename is only
for the unprefixed API?

https://codereview.chromium.org/600143005/diff/60001/Source/core/events/Event...
File Source/core/events/EventTypeNames.in (right):

https://codereview.chromium.org/600143005/diff/60001/Source/core/events/Event...
Source/core/events/EventTypeNames.in:226: webkitneedkey
this staying here is intended, right?

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp (right):

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:303: static
PassRefPtrWillBeRawPtr<Event> createWebkitNeedKeyEvent(const unsigned char*
initData, unsigned initDataLength)
is this a drive-by change? this seems mildly unrelated to needkey->encrypted
event rename (since this line is a prefixed API method change). Please update CL
description accordingly.

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.h (right):

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.h:47: static void
encrypted(HTMLMediaElement&, const String& initDataType, const unsigned char*
initData, unsigned initDataLength);
ditto: more than just a rename of needkey->encrypted (method name and parameters
here are also changed). Update CL description?

[sandersd (OOO until July 31), 2014-10-02 17:48:17]

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html:38:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
On 2014/10/02 00:15:20, wolenetz wrote:
> nit: this seems more than just a rename. Update CL description?

Done.

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

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:14: var
expectedInitData = stringToUint8Array('0123456789012345');
On 2014/10/02 00:15:20, wolenetz wrote:
> nit: test-encrypted.webm's expected metadata is used in more than one place.
> Extract to encrypted-media-utils.js? (perhaps better in a separate CL)

Ideally we wouldn't need the values in multiple tests, as it indicates poor test
scope. Perhaps a bigger refactor could correct that, but it should indeed be a
separate CL.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:28:
assert_equals(event.initDataType, 'webm');
On 2014/10/02 00:15:20, wolenetz wrote:
> eme noob question: why not also check contentType?

contentType is gone, initDataType replaced it.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:29:
assert_array_equals(new Uint8Array(event.initData), expectedInitData);
On 2014/10/02 00:15:20, wolenetz wrote:
> eme noob question: why now wrapping in a Uint8Array instance?

The return type is ArrayBuffer, which is not indexable. Wrapping in Uint8Array
makes it indexable as a list of bytes (which is what the assertion function
expects).

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:31: if
(--expectedEvents == 0)
On 2014/10/02 00:15:20, wolenetz wrote:
> nit: this affirms only that at least 2 events were dispatched. what if more
are
> incorrectly dispatched? (perhaps better in a separate CL, or maybe this is
> already checked in some other layout test and this is a noob nit ;) )

You are correct, but there is not a lot we can do other than setting a timer and
waiting to see if more events happen, which would be overly complicated.

There's a good chance we'd catch such a bug anyway, because this is a console
log diff test.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File
LayoutTests/media/encrypted-media/encrypted-media-playback-multiple-sessions.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-multiple-sessions.html:32:
var keyId = String.fromCharCode.apply(null, new Uint8Array(event.initData));
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto why wrap?

This wrap is necessary either way to access to the actual bytes in the
ArrayBuffer.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-multiple-sessions.html:36:
mediaKeySession.generateRequest(event.initDataType,
event.initData).then(function() {
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto why change from contenttype to initdatatype (seems more than just rename
> needkey->encrypted)

Done.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-after-src.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-after-src.html:20:
var rawKey = new Uint8Array([0xeb, 0xdd, 0x62, 0xf1, 0x68, 0x14, 0xd2, 0x7b,
On 2014/10/02 00:15:20, wolenetz wrote:
> nit ditto: metadata definition copied in lots of places instead of in a common
> include.

Acknowledged.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-after-src.html:37:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto: more than a rename

Done.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-before-src.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-before-src.html:20:
var rawKey = new Uint8Array([0xeb, 0xdd, 0x62, 0xf1, 0x68, 0x14, 0xd2, 0x7b,
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto: metadata

Acknowledged.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-playback-setmediakeys-before-src.html:36:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto: more than a rename

Done.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File LayoutTests/media/encrypted-media/encrypted-media-waiting-for-a-key.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-waiting-for-a-key.html:21: var
rawKey = new Uint8Array([0xeb, 0xdd, 0x62, 0xf1, 0x68, 0x14, 0xd2, 0x7b,
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto: metadata

Acknowledged.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/encrypted-media-waiting-for-a-key.html:38:
mediaKeySession.generateRequest(event.initDataType,
event.initData).catch(function(error) {
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto: more than a rename

Done.

https://codereview.chromium.org/600143005/diff/60001/Source/core/events/Event...
File Source/core/events/EventTypeNames.in (right):

https://codereview.chromium.org/600143005/diff/60001/Source/core/events/Event...
Source/core/events/EventTypeNames.in:226: webkitneedkey
On 2014/10/02 00:15:20, wolenetz wrote:
> this staying here is intended, right?

Yes, the prefixed API is frozen.

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp (right):

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:303: static
PassRefPtrWillBeRawPtr<Event> createWebkitNeedKeyEvent(const unsigned char*
initData, unsigned initDataLength)
On 2014/10/02 00:15:20, wolenetz wrote:
> is this a drive-by change? this seems mildly unrelated to needkey->encrypted
> event rename (since this line is a prefixed API method change). Please update
CL
> description accordingly.

Done.

I usually don't document very minor fixes like this one (affects only two lines
in this file, no behavior change), because I worry that raising the bar for
doing tiny fixes reduces the chances people will actually do them. A quick poll
in Kirkland suggests there are a variety of opinions, with no particular Chrome
culture to follow :-(.

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.h (right):

https://codereview.chromium.org/600143005/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.h:47: static void
encrypted(HTMLMediaElement&, const String& initDataType, const unsigned char*
initData, unsigned initDataLength);
On 2014/10/02 00:15:20, wolenetz wrote:
> ditto: more than just a rename of needkey->encrypted (method name and
parameters
> here are also changed). Update CL description?

Done.

Note that there are pending CLs to complete this particular rename throughout
Chromium, this specific instance thankfully doesn't cross between Blink and
Chromium.

[wolenetz, 2014-10-02 18:11:34]

Thank you for the clarifications. Verbosity can be the enemy of agility, though
sometimes it helps a noob reviewer to the area like me :)
lgtm % s/it's/its/ in CL description

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
File LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html
(right):

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html:17: var
keyNeededEvent = document.createEvent("MediaKeyEvent");
On 2014/10/02 00:15:20, wolenetz wrote:
> nit: this seems more than just a rename, and it appears this change impacts
> prefixed API. Is this intended?

I received clarification from sandersd@ that this test was previously wrong
(testing the unprefixed, not prefixed API), and this portion of the CL fixes
that to test the prefixed API. No visible API change to prefixed API is intended
to be included in this CL.

https://codereview.chromium.org/600143005/diff/60001/LayoutTests/media/encryp...
LayoutTests/media/encrypted-media/prefixed/encrypted-media-events.html:143:
function needKey(event)
On 2014/10/02 00:15:20, wolenetz wrote:
> eme noob question: Is this correct across the rename, since the rename is only
> for the unprefixed API?

I received clarification (see previous comment in this file.)

[sandersd (OOO until July 31), 2014-10-02 18:17:22]

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

[sandersd (OOO until July 31), 2014-10-02 18:17:23]

jamesr@chromium.org: Please review changes in Source/web and public/platform.

[ddorwin, 2014-10-02 18:39:22]

lgtm

[sandersd (OOO until July 31), 2014-10-07 19:38:33]

sandersd@chromium.org changed reviewers:
+ jochen@chromium.org
- jamesr@chromium.org

[sandersd (OOO until July 31), 2014-10-07 19:38:34]

jochen: Please review changes in Source/web, Source/core/events, and
public/platform.

[jochen (gone - plz use gerrit), 2014-10-08 14:46:02]

lgtm

[sandersd (OOO until July 31), 2014-10-08 17:32:56]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-10-08 17:33:39]

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

[commit-bot: I haz the power, 2014-10-08 17:33:55]

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

[commit-bot: I haz the power, 2014-10-08 17:33:57]

Failed to apply patch for
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
  Copied LayoutTests/media/encrypted-media/encrypted-media-needkey.html ->
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
  patching file
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
  Hunk #3 FAILED at 20.
  1 out of 3 hunks FAILED -- saving rejects to file
LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html.rej

Patch:   NR 
LayoutTests/media/encrypted-media/encrypted-media-needkey.html->LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
Index: LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
diff --git a/LayoutTests/media/encrypted-media/encrypted-media-needkey.html
b/LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
similarity index 60%
rename from LayoutTests/media/encrypted-media/encrypted-media-needkey.html
rename to LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
index
857b0685b9184edbe671be087f6652ce1c4b9a3e..67577ffa976fd79179dea20da1cf39dd401a3083
100644
--- a/LayoutTests/media/encrypted-media/encrypted-media-needkey.html
+++ b/LayoutTests/media/encrypted-media/encrypted-media-onencrypted.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <html>
     <head>
-        <title>NeedKey</title>
+        <title>onencrypted</title>
         <script src="encrypted-media-utils.js"></script>
         <script src="../../resources/testharness.js"></script>
         <script src="../../resources/testharnessreport.js"></script>
@@ -9,7 +9,7 @@
     <body>
         <video id="testVideo" controls></video>
         <div id="log"></div>
-        <p>Test that needkey event is fired on an encrypted media file.</p>
+        <p>Test that encrypted event is fired on an encrypted media file.</p>
         <script>
             var expectedInitData = stringToUint8Array('0123456789012345');
 
@@ -20,21 +20,21 @@
             {
                 var video = document.getElementById('testVideo');
 
-                var onNeedKey = function(event)
+                var onEncrypted = function(event)
                 {
                     assert_equals(event.target, video);
-                    assert_true(event instanceof window.MediaKeyNeededEvent);
-                    assert_equals(event.type, 'needkey');
-                    assert_equals(event.contentType, 'video/webm');
-                    assert_array_equals(event.initData, expectedInitData);
+                    assert_true(event instanceof window.MediaEncryptedEvent);
+                    assert_equals(event.type, 'encrypted');
+                    assert_equals(event.initDataType, 'webm');
+                    assert_array_equals(new Uint8Array(event.initData),
expectedInitData);
 
                     if (--expectedEvents == 0)
                         test.done();
                 };
 
-                waitForEventAndRunStep('needkey', video, onNeedKey, test);
+                waitForEventAndRunStep('encrypted', video, onEncrypted, test);
                 video.src = '../content/test-encrypted.webm';
-            }, 'Needkey fired on encrypted media file.');
+            }, 'encrypted fired on encrypted media file.');
         </script>
     </body>
 </html>

[sandersd (OOO until July 31), 2014-10-08 18:06:49]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-10-08 18:07:17]

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

[commit-bot: I haz the power, 2014-10-08 20:15:31]

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

[commit-bot: I haz the power, 2014-10-08 20:15:32]

Exceeded time limit waiting for builds to trigger.

[sandersd (OOO until July 31), 2014-10-08 20:40:26]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-10-08 20:41:32]

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

[commit-bot: I haz the power, 2014-10-09 05:19:54]

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

[commit-bot: I haz the power, 2014-10-09 05:19:55]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/26337)

[sandersd (OOO until July 31), 2014-10-09 22:32:47]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-10-09 22:34:03]

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

[commit-bot: I haz the power, 2014-10-10 01:21:48]

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

[commit-bot: I haz the power, 2014-10-10 01:21:49]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)

[sandersd (OOO until July 31), 2014-10-10 18:01:28]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-10-10 18:02:00]

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

[commit-bot: I haz the power, 2014-10-10 19:07:03]

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

[commit-bot: I haz the power, 2014-10-10 19:07:08]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)

[sandersd (OOO until July 31), 2014-10-10 20:07:30]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-10-10 20:07:55]

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

[wolenetz, 2014-10-10 20:41:39]

On 2014/10/10 20:07:55, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/600143005/100001

fyi - linux_blink_rel, at least, is consistently failing webkit tests with this
patch applied, specifically the "webexposed/global-constructors-listing.html"
test.
MediaKeyNeeded is referred to, still, in that test's expectations.

[sandersd (OOO until July 31), 2014-10-10 20:49:08]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-10-10 20:50:20]

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

[commit-bot: I haz the power, 2014-10-10 21:53:24]

Committed patchset #7 (id:400001) as 183561

[Stephen White, 2014-10-12 14:38:19]

On 2014/10/10 21:53:24, I haz the power (commit-bot) wrote:
> Committed patchset #7 (id:400001) as 183561

Looks like the Chrome side of this was reverted in
de394fa4553674e0aa522ce7c3483394f0f40d3d (r297538), which means this is now
causing browser_tests failures in the Blink canaries and blocking the Blink
roll. I'm going to try a speculative revert.