Fixed and un-skipped csp/font-src wpt tests

Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.
Moved fonts.css into content-security-policy/support

BUG=694525
Review-Url: https://codereview.chromium.org/2777823003
Cr-Commit-Position: refs/heads/master@{#460333}
Committed: https://chromium.googlesource.com/chromium/src/+/d392fbe7235460ea17422fa98af5084698867afe

[andypaicu, 2017-03-27 09:28:07]

Description was changed from

==========
Fixed and un-skipped csp/font-src wpt tests

BUG=694525
==========

to

==========
Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.

BUG=694525
==========

[andypaicu, 2017-03-27 09:28:07]

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

[andypaicu, 2017-03-27 09:28:57]

Description was changed from

==========
Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.

BUG=694525
==========

to

==========
Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.
Moved fonts.css into content-security-policy/support

BUG=694525
==========

[andypaicu, 2017-03-27 09:29:36]

font-src tests

[andypaicu, 2017-03-27 10:48:00]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 10:48:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2017-03-27 11:47:03]

LGTM % one test.

https://codereview.chromium.org/2777823003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.html
(right):

https://codereview.chromium.org/2777823003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.html:18:
link.onload = t.step_func_done();
`onload` + `onload` = this test is a noop.

[commit-bot: I haz the power, 2017-03-27 12:01:09]

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

[commit-bot: I haz the power, 2017-03-27 12:01:10]

Dry run: This issue passed the CQ dry run.

[andypaicu, 2017-03-27 13:43:57]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 13:44:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[andypaicu, 2017-03-27 13:46:17]

Argh, I've copy pasted that test incorrectly on both the negative tests
Turns out the tests were not build correctly because the stylesheet loads even
if the fonts fail
to. So I've modified all the tests to attempt to load the font directly and
added another one where the font is loaded in the stylesheet.

[commit-bot: I haz the power, 2017-03-27 13:46:59]

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

[commit-bot: I haz the power, 2017-03-27 13:47:00]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[andypaicu, 2017-03-27 13:47:02]

The CQ bit was unchecked by andypaicu@chromium.org

[andypaicu, 2017-03-27 13:57:14]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 13:57:23]

Dry run: 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, 2017-03-27 14:58:08]

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

[commit-bot: I haz the power, 2017-03-27 14:58:09]

Dry run: This issue passed the CQ dry run.

[Mike West, 2017-03-28 10:18:37]

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

[Mike West, 2017-03-28 10:18:38]

Still LGTM.

CCing yoav@ for the preload question, but you don't need to wait for that to
land this. If I'm not misunderstanding things, it seems like a bug in our
preload implementation, and not in CSP.

https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html
(right):

https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html:17:
link.onload = t.unreached_func("Should not have loaded the font.");
Shouldn't this fail regardless, as fonts require CORS? I'm a bit surprised that
it works without a `crossorigin` attribute.

We can land it like this to record current behavior, but it's surprising
behavior.

https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-self-allowed.html
(right):

https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-self-allowed.html:18:
link.onerror = t.unreached_func("Should have loaded the font.");
Can you add something like:

```
document.addEventListener("securitypolicyviolation", t.unreached_func("Loading
allowed fonts should not trigger a violation."));
```

to this and the other allowed tests?

[Yoav Weiss, 2017-03-28 10:32:08]

https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html
(right):

https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html:17:
link.onload = t.unreached_func("Should not have loaded the font.");
On 2017/03/28 10:18:38, Mike West wrote:
> Shouldn't this fail regardless, as fonts require CORS? I'm a bit surprised
that
> it works without a `crossorigin` attribute.
> 
> We can land it like this to record current behavior, but it's surprising
> behavior.

AFAICT, without CSP the font will be loaded here, even without a crossorigin
attribute. It just won't be properly reused without one (meaning that styles
that will try to use that font will cause a double download).

[Mike West, 2017-03-28 14:37:00]

On 2017/03/28 at 10:32:08, yoav wrote:
>
https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html
(right):
> 
>
https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html:17:
link.onload = t.unreached_func("Should not have loaded the font.");
> On 2017/03/28 10:18:38, Mike West wrote:
> > Shouldn't this fail regardless, as fonts require CORS? I'm a bit surprised
that
> > it works without a `crossorigin` attribute.
> > 
> > We can land it like this to record current behavior, but it's surprising
> > behavior.
> 
> AFAICT, without CSP the font will be loaded here, even without a crossorigin
attribute. It just won't be properly reused without one (meaning that styles
that will try to use that font will cause a double download).

That seems strange. :/

[Yoav Weiss, 2017-03-28 15:58:43]

On 2017/03/28 14:37:00, Mike West wrote:
> On 2017/03/28 at 10:32:08, yoav wrote:
> >
>
https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
> > File
>
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html
> (right):
> > 
> >
>
https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
> >
>
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html:17:
> link.onload = t.unreached_func("Should not have loaded the font.");
> > On 2017/03/28 10:18:38, Mike West wrote:
> > > Shouldn't this fail regardless, as fonts require CORS? I'm a bit surprised
> that
> > > it works without a `crossorigin` attribute.
> > > 
> > > We can land it like this to record current behavior, but it's surprising
> > > behavior.
> > 
> > AFAICT, without CSP the font will be loaded here, even without a crossorigin
> attribute. It just won't be properly reused without one (meaning that styles
> that will try to use that font will cause a double download).
> 
> That seems strange. :/

We discussed at some point making the "font" destination set crossorigin by
default, but people claimed that it would be too magical...
Unfortunately, the opposite of "too magical" is "web developers have to know the
right crossorigin settings for all resource types in order to avoid double
downloads"

[Yoav Weiss, 2017-03-28 15:59:02]

Description was changed from

==========
Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.
Moved fonts.css into content-security-policy/support

BUG=694525
==========

to

==========
Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.
Moved fonts.css into content-security-policy/support

BUG=694525
==========

[andypaicu, 2017-03-29 07:54:17]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 07:54:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2017-03-29 08:36:36]

On 2017/03/28 at 15:58:43, yoav wrote:
> On 2017/03/28 14:37:00, Mike West wrote:
> > On 2017/03/28 at 10:32:08, yoav wrote:
> > >
> >
https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
> > > File
> >
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html
> > (right):
> > > 
> > >
> >
https://codereview.chromium.org/2777823003/diff/40001/third_party/WebKit/Layo...
> > >
> >
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/font-src/font-none-blocked.sub.html:17:
> > link.onload = t.unreached_func("Should not have loaded the font.");
> > > On 2017/03/28 10:18:38, Mike West wrote:
> > > > Shouldn't this fail regardless, as fonts require CORS? I'm a bit
surprised
> > that
> > > > it works without a `crossorigin` attribute.
> > > > 
> > > > We can land it like this to record current behavior, but it's surprising
> > > > behavior.
> > > 
> > > AFAICT, without CSP the font will be loaded here, even without a
crossorigin
> > attribute. It just won't be properly reused without one (meaning that styles
> > that will try to use that font will cause a double download).
> > 
> > That seems strange. :/
> 
> We discussed at some point making the "font" destination set crossorigin by
default, but people claimed that it would be too magical...
> Unfortunately, the opposite of "too magical" is "web developers have to know
the right crossorigin settings for all resource types in order to avoid double
downloads"

I see. That still seems strange, but is certainly unrelated to this CL. So, land
it! :)

[commit-bot: I haz the power, 2017-03-29 09:14:10]

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

[commit-bot: I haz the power, 2017-03-29 09:14:11]

Dry run: This issue passed the CQ dry run.

[andypaicu, 2017-03-29 09:29:31]

The CQ bit was checked by andypaicu@chromium.org

[andypaicu, 2017-03-29 09:29:31]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2777823003/#ps60001
(title: "CR changes")

[commit-bot: I haz the power, 2017-03-29 09:29:48]

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, 2017-03-29 09:35:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490779771360590,
"parent_rev": "95bac4bef674761b73170615dc960444b535695b", "commit_rev":
"d392fbe7235460ea17422fa98af5084698867afe"}

[commit-bot: I haz the power, 2017-03-29 09:36:22]

Description was changed from

==========
Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.
Moved fonts.css into content-security-policy/support

BUG=694525
==========

to

==========
Fixed and un-skipped csp/font-src wpt tests

Modified existing tests to use testharness instead.
Added more tests.
Moved fonts.css into content-security-policy/support

BUG=694525

Review-Url: https://codereview.chromium.org/2777823003
Cr-Commit-Position: refs/heads/master@{#460333}
Committed:
https://chromium.googlesource.com/chromium/src/+/d392fbe7235460ea17422fa98af5...
==========

[commit-bot: I haz the power, 2017-03-29 09:36:22]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/d392fbe7235460ea17422fa98af5...