Change EME WebIDL to use ArrayBuffer/ArrayBufferView.

Change EME WebIDL to use ArrayBuffer/ArrayBufferView.

Now that the spec specifies that data should be passed using
ArrayBuffer/ArrayBufferView, change the IDL to match.

BUG=358271
TEST=Updated EME layouts tests pass

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

[jrummell, 2014-07-16 21:40:47]

PTAL.

[ddorwin, 2014-07-16 22:34:58]

Thanks. C++ looking good overall. I'll have to re-review the test files after
the general comments are addressed.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
File LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html
(right):

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html:50: var
keyId = decodeMessageAttribute(event);
In the future, this will be JSON containing a base64url-encoded key ID, not a
key ID directly.

See also my comments in the .js file.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
File LayoutTests/media/encrypted-media/encrypted-media-utils.js (right):

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:73: function
decodeMessageAttribute(event)
I'm not sure this is worth hiding. Part of the purpose of the tests is
understanding how things work. Note that the decoding is only relevant for Clear
Key, so we don't always need it.

Where the output is necessary, this can probably be shortened to something like:
x = TextDecoder('utf-8').decode(Uint8Array(event.message))

I think not relying on this function might address my comment at line 95 too,
allowing us to just send the ArrayBufferView.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:75: var decoder = new
TextDecoder('utf-8');
FYI, the Intent to Ship email was just sent, so this requires the experimental
web features flag when run outside the layout tests.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:95: // keyId is
expected to be a string, key is ArrayBufferView.
nit: "key is an"

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:95: // keyId is
expected to be a string, key is ArrayBufferView.
Why are these types different? They contain the same thing - binary data to be
encoded.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:99: jwk +=
base64Encode(keyId);
FIXME: base64URLEncode()

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

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-v2-events.html:37: </html>
? This file probably doesn't belong in this CL.

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

https://codereview.chromium.org/397463005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:363:
m_pendingActions.append(PendingAction::CreatePendingUpdate(result, response));
Does PassRefPtr make the release implicit or are you just assuming the reference
gets deleted when we exit this function? Would it be better to be explicit about
the transfer of ownership? (We may not even need a RefPtr if it's not required
by other parts of Blink.

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

https://codereview.chromium.org/397463005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.idl:39: [CallWith=ScriptState] Promise
createSession(DOMString initDataType, ArrayBufferView initData, optional
SessionType sessionType = "temporary");
We need to accept both.

[jrummell, 2014-07-18 21:56:01]

Updated so createSession() takes ArrayBuffer as well.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
File LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html
(right):

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-lifetime-reload.html:50: var
keyId = decodeMessageAttribute(event);
On 2014/07/16 22:34:57, ddorwin wrote:
> In the future, this will be JSON containing a base64url-encoded key ID, not a
> key ID directly.
> 
> See also my comments in the .js file.

Acknowledged.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
File LayoutTests/media/encrypted-media/encrypted-media-utils.js (right):

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:73: function
decodeMessageAttribute(event)
On 2014/07/16 22:34:57, ddorwin wrote:
> I'm not sure this is worth hiding. Part of the purpose of the tests is
> understanding how things work. Note that the decoding is only relevant for
Clear
> Key, so we don't always need it.
> 
> Where the output is necessary, this can probably be shortened to something
like:
> x = TextDecoder('utf-8').decode(Uint8Array(event.message))
> 
> I think not relying on this function might address my comment at line 95 too,
> allowing us to just send the ArrayBufferView.

Removed. We can do the conversion when ClearKey is updated.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:75: var decoder = new
TextDecoder('utf-8');
On 2014/07/16 22:34:57, ddorwin wrote:
> FYI, the Intent to Ship email was just sent, so this requires the experimental
> web features flag when run outside the layout tests.

Acknowledged.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:95: // keyId is
expected to be a string, key is ArrayBufferView.
On 2014/07/16 22:34:57, ddorwin wrote:
> nit: "key is an"

Updated.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:95: // keyId is
expected to be a string, key is ArrayBufferView.
On 2014/07/16 22:34:57, ddorwin wrote:
> Why are these types different? They contain the same thing - binary data to be
> encoded.

They do now.

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-utils.js:99: jwk +=
base64Encode(keyId);
On 2014/07/16 22:34:57, ddorwin wrote:
> FIXME: base64URLEncode()

Done.

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

https://codereview.chromium.org/397463005/diff/1/LayoutTests/media/encrypted-...
LayoutTests/media/encrypted-media/encrypted-media-v2-events.html:37: </html>
On 2014/07/16 22:34:57, ddorwin wrote:
> ? This file probably doesn't belong in this CL.

Removed. The file is missing \n at the end, which is why it got included in this
cl.

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

https://codereview.chromium.org/397463005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:363:
m_pendingActions.append(PendingAction::CreatePendingUpdate(result, response));
On 2014/07/16 22:34:57, ddorwin wrote:
> Does PassRefPtr make the release implicit or are you just assuming the
reference
> gets deleted when we exit this function? Would it be better to be explicit
about
> the transfer of ownership? (We may not even need a RefPtr if it's not required
> by other parts of Blink.

ArrayBuffer::create() returns a RefPtr, so I don't think we can get rid of it.
updateInternal() gets passed the RefPtr and simply passes it on. It should get
released if CreatePendingUpdate() doesn't grab the object (which is does when
assigning to |m_data|).

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

https://codereview.chromium.org/397463005/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.idl:39: [CallWith=ScriptState] Promise
createSession(DOMString initDataType, ArrayBufferView initData, optional
SessionType sessionType = "temporary");
On 2014/07/16 22:34:58, ddorwin wrote:
> We need to accept both.

Done.

[ddorwin, 2014-07-24 22:20:44]

Thanks. The test changes are much more straightforward. LGTM

[jrummell, 2014-07-25 20:53:11]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-07-25 20:53:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jrummell@chromium.org/397463005/20001

[commit-bot: I haz the power, 2014-07-25 20:53:36]

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

[commit-bot: I haz the power, 2014-07-25 20:53:37]

Failed to apply patch for
Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp
  Hunk #1 FAILED at 26.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp.rej

Patch:       Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp
Index: Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp
diff --git a/Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp
b/Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp
index
3874a818071e89471d24651856a3fe0f55c3406b..15deed6c6ba15a8f301a68b56b5fe640aade6fc8
100644
--- a/Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp
+++ b/Source/modules/encryptedmedia/MediaKeyMessageEvent.cpp
@@ -26,8 +26,6 @@
 #include "config.h"
 #include "modules/encryptedmedia/MediaKeyMessageEvent.h"
 
-#include "wtf/Uint8Array.h"
-
 namespace WebCore {
 
 MediaKeyMessageEventInit::MediaKeyMessageEventInit()

[jrummell, 2014-07-25 22:37:20]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-07-25 22:37:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jrummell@chromium.org/397463005/40001

[commit-bot: I haz the power, 2014-07-25 23:54:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/17171)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/19031)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-26 00:32:51]

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

[commit-bot: I haz the power, 2014-07-26 00:32:52]

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

[jrummell, 2014-07-26 00:40:22]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-07-26 00:40:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jrummell@chromium.org/397463005/40001

[commit-bot: I haz the power, 2014-07-26 05:12:51]

Change committed as 178978