Output silence if the MediaElementAudioSourceNode has a different origin

Output silence if the MediaElementAudioSourceNode has a different origin

See http://webaudio.github.io/web-audio-api/#security-with-mediaelementaudiosourcenode-and-cross-origin-resources

Two new tests added for the same origin and a cross origin source.

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

[Raymond Toy, 2014-08-28 22:27:46]

rtoy@chromium.org changed reviewers:
+ kbr@chromium.org

[Raymond Toy, 2014-08-28 22:27:46]

PTAL.

[Ken Russell (switch to Gerrit), 2014-08-29 00:51:47]

This is testable; see for example
LayoutTests/http/tests/security/webgl-remote-read-*. Could you please add a test
for this?

[Raymond Toy, 2014-09-09 21:12:51]

On 2014/08/29 00:51:47, Ken Russell wrote:
> This is testable; see for example
> LayoutTests/http/tests/security/webgl-remote-read-*. Could you please add a
test
> for this?

Done. PTAL. Not sure I implemented the cross-origin test correctly, though, but
it does work and it correctly fails when run in Chrome beta.

[Ken Russell (switch to Gerrit), 2014-09-10 04:16:43]

This test doesn't look correct. I think you need to add a test under
LayoutTests/http/tests/security/ in order to be able to set up cross-origin
fetches. I don't know why the data: URL is being treated as coming from a
different origin than the test's page, but that seems wrong.

https://codereview.chromium.org/520433002/diff/100001/LayoutTests/webaudio/me...
File LayoutTests/webaudio/mediaelementaudiosourcenode-cross-origin.html (right):

https://codereview.chromium.org/520433002/diff/100001/LayoutTests/webaudio/me...
LayoutTests/webaudio/mediaelementaudiosourcenode-cross-origin.html:19: var src =
"data:audio/wav;base64,UklGRmisAABXQVZFZm10IBAAAAABAAEARKwAAIhYAQACABAAZGF0YUSsAAAAAGoGzwwkE2oZkR+gJX8rPjG/NhQ8JUH5RYRKxU63UlZWnFmMXBpfTmEaY4dkjGUuZmZmOWajZapkSWOHYV9f21z1WbtWJFM8TwVLfUa6QaQ8ZTfZMS4sSSZFIB4a2xOJDSMHvABQ+uvzk+1L5yDhENso1WnP3cmExGy/j7r9tbOxuK0OqsCmw6Mtoe6eE52gm4ma45mZmb+ZQ5o5m4WcRZ5coNais6XiqG+sUbB5tP64t73EwgPIfs0w0wTZD98r5W7rwPEf+I3+8wReC7MRABgtHkMkLiryL4Q14zoDQOhEgEnWTddRjFXlWOpbjF7YYLhiQGRWZRZmXmZQZstl7mShY/Zh5V91Xadaflf8UydQ/0uOR9JC1T2XOCQzei2kJ6chhBtMFfgOmggvAsf7XPUE77boheJt3H3WstAcy7XFkMCjuwO3orKbrtqqeqdopLuhaJ93neubwJr/maGZrZkdmveaMpzWndmfPqIFpSConKtjr4Wz7LeivJfBzMY9zN3RsNer3cTjAupM8K/2E/2CA+YJRhCUFscc5SLXKKguRTSvOd8+0EN7SOFM9lC6VCxYQFv9XVpgVGLuYyRl7mVcZldm92UnZfVjYWJlYAxeU1s7WNJUDFH5TJdI6kP+Ps05ZjTHLv0oBCPuHLgWZxARCqIDPf3R9nPwJOrq487d0dcB0lrM7sazwb68B7ics36vrqs1qBWlTKLpn96dPZz6miSarJmhmfyZuZrmm2qdXZ+soVikZ6fFqoSui7Lotoi7ccCYxfrKlNBX1k7cXeKW6NvuOvWh+wkCdgjSDigVYBuEIYEnWS0DM3k4tj23QnJH5ksRUOZTa1eVWmZd2F/rYZlj5mTJZU1mX2YZZltlSGTCYuNgm177W/hYnlXwUetNnkkBRSBAAzuiNRUwUCpjJFQeIhjaEYELGwWv/kf45PGR61TlLN8r2VDTnc0nyNvC3L0PuZy0YLCKrPSow6XpomagUp6NnEGbRZrDmZiZ4JmEmpmbCZ3jnh6hs6Otpvypn62esd+1eLpMv2jEu8lKzwXV7tr74Cjnbu3G8yv6lQAAB2INuBP4GSQgJSYLLLwxQTeLPJlBZUbpSiZPDVOoVuJZzVxQX3thQmOgZKJlNGZpZi1mk2WNZCRjWmEmX55crVlqVs1S3U6dShZGP0E1PN02XzGgK8Qlsx+QGUgT8wyRBiIAwPlT8wLtuuaQ4Ifan9Tnzl3JC8T4viO6lbVTsV+tvql2poWj8aDBnu6cgJt6mtKZnpnCmQ==";
Are you sure that data: URLs are supposed to be treated as coming from a
different origin? That doesn't sound correct. In the Canvas APIs, if you can get
a data: URL for an image, then you can create an image from it and draw it to a
canvas, and the canvas won't be tainted.

[Raymond Toy, 2014-09-10 17:32:26]

On 2014/09/10 04:16:43, Ken Russell wrote:
> This test doesn't look correct. I think you need to add a test under
> LayoutTests/http/tests/security/ in order to be able to set up cross-origin
> fetches. I don't know why the data: URL is being treated as coming from a
> different origin than the test's page, but that seems wrong.
> 
>
https://codereview.chromium.org/520433002/diff/100001/LayoutTests/webaudio/me...
> File LayoutTests/webaudio/mediaelementaudiosourcenode-cross-origin.html
(right):
> 
>
https://codereview.chromium.org/520433002/diff/100001/LayoutTests/webaudio/me...
> LayoutTests/webaudio/mediaelementaudiosourcenode-cross-origin.html:19: var src
=
>
"data:audio/wav;base64,UklGRmisAABXQVZFZm10IBAAAAABAAEARKwAAIhYAQACABAAZGF0YUSsAAAAAGoGzwwkE2oZkR+gJX8rPjG/NhQ8JUH5RYRKxU63UlZWnFmMXBpfTmEaY4dkjGUuZmZmOWajZapkSWOHYV9f21z1WbtWJFM8TwVLfUa6QaQ8ZTfZMS4sSSZFIB4a2xOJDSMHvABQ+uvzk+1L5yDhENso1WnP3cmExGy/j7r9tbOxuK0OqsCmw6Mtoe6eE52gm4ma45mZmb+ZQ5o5m4WcRZ5coNais6XiqG+sUbB5tP64t73EwgPIfs0w0wTZD98r5W7rwPEf+I3+8wReC7MRABgtHkMkLiryL4Q14zoDQOhEgEnWTddRjFXlWOpbjF7YYLhiQGRWZRZmXmZQZstl7mShY/Zh5V91Xadaflf8UydQ/0uOR9JC1T2XOCQzei2kJ6chhBtMFfgOmggvAsf7XPUE77boheJt3H3WstAcy7XFkMCjuwO3orKbrtqqeqdopLuhaJ93neubwJr/maGZrZkdmveaMpzWndmfPqIFpSConKtjr4Wz7LeivJfBzMY9zN3RsNer3cTjAupM8K/2E/2CA+YJRhCUFscc5SLXKKguRTSvOd8+0EN7SOFM9lC6VCxYQFv9XVpgVGLuYyRl7mVcZldm92UnZfVjYWJlYAxeU1s7WNJUDFH5TJdI6kP+Ps05ZjTHLv0oBCPuHLgWZxARCqIDPf3R9nPwJOrq487d0dcB0lrM7sazwb68B7ics36vrqs1qBWlTKLpn96dPZz6miSarJmhmfyZuZrmm2qdXZ+soVikZ6fFqoSui7Lotoi7ccCYxfrKlNBX1k7cXeKW6NvuOvWh+wkCdgjSDigVYBuEIYEnWS0DM3k4tj23QnJH5ksRUOZTa1eVWmZd2F/rYZlj5mTJZU1mX2YZZltlSGTCYuNgm177W/hYnlXwUetNnkkBRSBAAzuiNRUwUCpjJFQeIhjaEYELGwWv/kf45PGR61TlLN8r2VDTnc0nyNvC3L0PuZy0YLCKrPSow6XpomagUp6NnEGbRZrDmZiZ4JmEmpmbCZ3jnh6hs6Otpvypn62esd+1eLpMv2jEu8lKzwXV7tr74Cjnbu3G8yv6lQAAB2INuBP4GSQgJSYLLLwxQTeLPJlBZUbpSiZPDVOoVuJZzVxQX3thQmOgZKJlNGZpZi1mk2WNZCRjWmEmX55crVlqVs1S3U6dShZGP0E1PN02XzGgK8Qlsx+QGUgT8wyRBiIAwPlT8wLtuuaQ4Ifan9Tnzl3JC8T4viO6lbVTsV+tvql2poWj8aDBnu6cgJt6mtKZnpnCmQ==";
> Are you sure that data: URLs are supposed to be treated as coming from a
> different origin? That doesn't sound correct. In the Canvas APIs, if you can
get
> a data: URL for an image, then you can create an image from it and draw it to
a
> canvas, and the canvas won't be tainted.

Yes, the data: URL surprised me.  Perhaps I'm not checking the origins
correctly? I just created a SecurityOrigin for the audio context based on the
executionContext()->url(). And I use canRequest to determine if the media
element source can be used.

Perhaps this is wrong?

[Ken Russell (switch to Gerrit), 2014-09-10 21:14:49]

On 2014/09/10 17:32:26, Raymond Toy wrote:
> On 2014/09/10 04:16:43, Ken Russell wrote:
> > This test doesn't look correct. I think you need to add a test under
> > LayoutTests/http/tests/security/ in order to be able to set up cross-origin
> > fetches. I don't know why the data: URL is being treated as coming from a
> > different origin than the test's page, but that seems wrong.
> > 
> >
>
https://codereview.chromium.org/520433002/diff/100001/LayoutTests/webaudio/me...
> > File LayoutTests/webaudio/mediaelementaudiosourcenode-cross-origin.html
> (right):
> > 
> >
>
https://codereview.chromium.org/520433002/diff/100001/LayoutTests/webaudio/me...
> > LayoutTests/webaudio/mediaelementaudiosourcenode-cross-origin.html:19: var
src
> =
> >
>
"data:audio/wav;base64,UklGRmisAABXQVZFZm10IBAAAAABAAEARKwAAIhYAQACABAAZGF0YUSsAAAAAGoGzwwkE2oZkR+gJX8rPjG/NhQ8JUH5RYRKxU63UlZWnFmMXBpfTmEaY4dkjGUuZmZmOWajZapkSWOHYV9f21z1WbtWJFM8TwVLfUa6QaQ8ZTfZMS4sSSZFIB4a2xOJDSMHvABQ+uvzk+1L5yDhENso1WnP3cmExGy/j7r9tbOxuK0OqsCmw6Mtoe6eE52gm4ma45mZmb+ZQ5o5m4WcRZ5coNais6XiqG+sUbB5tP64t73EwgPIfs0w0wTZD98r5W7rwPEf+I3+8wReC7MRABgtHkMkLiryL4Q14zoDQOhEgEnWTddRjFXlWOpbjF7YYLhiQGRWZRZmXmZQZstl7mShY/Zh5V91Xadaflf8UydQ/0uOR9JC1T2XOCQzei2kJ6chhBtMFfgOmggvAsf7XPUE77boheJt3H3WstAcy7XFkMCjuwO3orKbrtqqeqdopLuhaJ93neubwJr/maGZrZkdmveaMpzWndmfPqIFpSConKtjr4Wz7LeivJfBzMY9zN3RsNer3cTjAupM8K/2E/2CA+YJRhCUFscc5SLXKKguRTSvOd8+0EN7SOFM9lC6VCxYQFv9XVpgVGLuYyRl7mVcZldm92UnZfVjYWJlYAxeU1s7WNJUDFH5TJdI6kP+Ps05ZjTHLv0oBCPuHLgWZxARCqIDPf3R9nPwJOrq487d0dcB0lrM7sazwb68B7ics36vrqs1qBWlTKLpn96dPZz6miSarJmhmfyZuZrmm2qdXZ+soVikZ6fFqoSui7Lotoi7ccCYxfrKlNBX1k7cXeKW6NvuOvWh+wkCdgjSDigVYBuEIYEnWS0DM3k4tj23QnJH5ksRUOZTa1eVWmZd2F/rYZlj5mTJZU1mX2YZZltlSGTCYuNgm177W/hYnlXwUetNnkkBRSBAAzuiNRUwUCpjJFQeIhjaEYELGwWv/kf45PGR61TlLN8r2VDTnc0nyNvC3L0PuZy0YLCKrPSow6XpomagUp6NnEGbRZrDmZiZ4JmEmpmbCZ3jnh6hs6Otpvypn62esd+1eLpMv2jEu8lKzwXV7tr74Cjnbu3G8yv6lQAAB2INuBP4GSQgJSYLLLwxQTeLPJlBZUbpSiZPDVOoVuJZzVxQX3thQmOgZKJlNGZpZi1mk2WNZCRjWmEmX55crVlqVs1S3U6dShZGP0E1PN02XzGgK8Qlsx+QGUgT8wyRBiIAwPlT8wLtuuaQ4Ifan9Tnzl3JC8T4viO6lbVTsV+tvql2poWj8aDBnu6cgJt6mtKZnpnCmQ==";
> > Are you sure that data: URLs are supposed to be treated as coming from a
> > different origin? That doesn't sound correct. In the Canvas APIs, if you can
> get
> > a data: URL for an image, then you can create an image from it and draw it
to
> a
> > canvas, and the canvas won't be tainted.
> 
> Yes, the data: URL surprised me.  Perhaps I'm not checking the origins
> correctly? I just created a SecurityOrigin for the audio context based on the
> executionContext()->url(). And I use canRequest to determine if the media
> element source can be used.
> 
> Perhaps this is wrong?

I haven't looked at the associated code in a while, but can you please study the
CanvasRenderingContext2D implementation? It contains checks like this for
tainting the canvas.

[Raymond Toy, 2014-09-22 20:05:42]

PTAL. Tests have been added and are working as expected.

[Ken Russell (switch to Gerrit), 2014-09-22 21:43:24]

This looks better. Please add at least one more test which sets the crossOrigin
property on the media element to "anonymous" and verifies that the samples can
be read. This is important for apps which load resources from other domains,
when those domains allow the sound files to be fetched without credentials. See
webgl-remote-read-remote-image-allowed.html for an example. For extra credit,
mirror webgl-remote-read-remote-image-allowed-with-credentials.html too.

[Raymond Toy, 2015-02-03 21:20:25]

On 2014/09/22 21:43:24, Ken Russell wrote:
> This looks better. Please add at least one more test which sets the
crossOrigin
> property on the media element to "anonymous" and verifies that the samples can
> be read. This is important for apps which load resources from other domains,
> when those domains allow the sound files to be fetched without credentials.
See
> webgl-remote-read-remote-image-allowed.html for an example. For extra credit,
> mirror webgl-remote-read-remote-image-allowed-with-credentials.html too.

PTAL.  I've added test for the cross-origin cases with anonymous crossOrigin and
with a test with credentials.

[Ken Russell (switch to Gerrit), 2015-02-04 01:28:19]

Very nice. LGTM overall. Couple minor points.

https://codereview.chromium.org/520433002/diff/280001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/security/media-element-audio-source-node-cross-origin-allowed.html
(right):

https://codereview.chromium.org/520433002/diff/280001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/media-element-audio-source-node-cross-origin-allowed.html:23:
// should be zero.
This comment is wrong for this test.

https://codereview.chromium.org/520433002/diff/280001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/security/media-element-audio-source-node-cross-origin-with-credentials.html
(right):

https://codereview.chromium.org/520433002/diff/280001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/media-element-audio-source-node-cross-origin-with-credentials.html:23:
// should be zero.
This comment is wrong for this test.

https://codereview.chromium.org/520433002/diff/280001/Source/modules/webaudio...
File Source/modules/webaudio/MediaElementAudioSourceNode.cpp (right):

https://codereview.chromium.org/520433002/diff/280001/Source/modules/webaudio...
Source/modules/webaudio/MediaElementAudioSourceNode.cpp:134: ||
context()->securityOrigin()->canRequest(mediaElement()->currentSrc()))) {
The indentation looks confusing here. I'd indent the || a couple more spaces to
be on the right hand side of the inner open-paren.
http://www.chromium.org/blink/coding-style#TOC-Line-breaking (not really covered
there)

[Raymond Toy, 2015-02-04 19:00:39]

Thanks so much for reviewing this and your patience on this CL.

https://codereview.chromium.org/520433002/diff/280001/Source/modules/webaudio...
File Source/modules/webaudio/MediaElementAudioSourceNode.cpp (right):

https://codereview.chromium.org/520433002/diff/280001/Source/modules/webaudio...
Source/modules/webaudio/MediaElementAudioSourceNode.cpp:134: ||
context()->securityOrigin()->canRequest(mediaElement()->currentSrc()))) {
On 2015/02/04 01:28:19, Ken Russell wrote:
> The indentation looks confusing here. I'd indent the || a couple more spaces
to
> be on the right hand side of the inner open-paren.
> http://www.chromium.org/blink/coding-style#TOC-Line-breaking (not really
covered
> there)

Clang-format puts || under the "d" in mediaElement, which is nicer.

[Raymond Toy, 2015-02-04 19:05:05]

New patchsets have been uploaded after l-g-t-m from kbr@chromium.org

[Raymond Toy, 2015-02-04 19:06:33]

On 2015/02/04 19:00:39, Raymond Toy wrote:
> Thanks so much for reviewing this and your patience on this CL.
> 
>
https://codereview.chromium.org/520433002/diff/280001/Source/modules/webaudio...
> File Source/modules/webaudio/MediaElementAudioSourceNode.cpp (right):
> 
>
https://codereview.chromium.org/520433002/diff/280001/Source/modules/webaudio...
> Source/modules/webaudio/MediaElementAudioSourceNode.cpp:134: ||
> context()->securityOrigin()->canRequest(mediaElement()->currentSrc()))) {
> On 2015/02/04 01:28:19, Ken Russell wrote:
> > The indentation looks confusing here. I'd indent the || a couple more spaces
> to
> > be on the right hand side of the inner open-paren.
> > http://www.chromium.org/blink/coding-style#TOC-Line-breaking (not really
> covered
> > there)
> 
> Clang-format puts || under the "d" in mediaElement, which is nicer.

The presubmit check fails when I do this, and the examples from
http://www.chromium.org/blink/coding-style#TOC-Other-Punctuation shows 4-space
indent pattern. (I must have clang-format setup incorrectly.)

[Raymond Toy, 2015-02-04 19:06:55]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2015-02-04 19:08:00]

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

[commit-bot: I haz the power, 2015-02-04 20:29:13]

Committed patchset #16 (id:300001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189527