Use RequestContext to apply CSP in FrameFetchContext

Use RequestContext to apply CSP in FrameFetchContext

Instead of a big switch statement on Resource::Type in
FrameFetchContext::canRequest(), use
ContentSecurityPolicy::allowRequest(), which decides which directives to
apply based on the RequestContext.

A nice-ish side effect of this is that DocumentThreadableLoader no
longer needs to know about (much less be responsible for enforcing)
CSP. Previously, DocumentThreadableLoader needed to enforce CSP because
Resource::Type doesn't cover things like EventSource, so
FrameFetchContext::canRequest() just let them pass through as raw
resources. Now that FrameFetchContext::canRequest() is based on
RequestContext, and RequestContext covers things like EventSource, the
proper CSP directive can be applied, instead of DocumentThreadableLoader
needing to do special CSP checks.

I say "nice-ish" instead of "nice" because kicking CSP checks out of
DocumentThreadableLoader introduces a slight complication: EventSource
and friends want to know about redirects that are blocked, so that they
can product an error for Javascript to consume. (We have layout tests
that test for this error.) So I introduced a new set of hooks for
RawResourceClients to get notified when ResourceFetcher declines to
follow a redirect. (The previous flow for EventSource and friends was
that ResourceFetcher would agree to follow the redirect, but
DocumentThreadableLoader would block it and notify the
RawResourceClient. Now that ResourceFetcher is deciding based on
RequestContexts, ResourceFetcher will want to block the redirect.)

BUG=474412
Committed: https://crrev.com/077fcfa04ed8d166cdd51bfd6f9ac36a2ccb8b5d
Cr-Commit-Position: refs/heads/master@{#385669}

[estark, 2016-04-05 23:09:01]

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

[estark, 2016-04-05 23:09:01]

mkwst, PTAL?

[Mike West, 2016-04-06 07:26:38]

This is great! LGTM.

https://codereview.chromium.org/1866433002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/RawResource.h (right):

https://codereview.chromium.org/1866433002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/RawResource.h:115: virtual void
redirectReceivedAndNotFollowed() {}
Tiny tiny nit: This sounds clunky, but I don't have a better suggestion.
`redirectBlocked`? Idunno.

[estark, 2016-04-06 17:52:57]

Thanks!

https://codereview.chromium.org/1866433002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/RawResource.h (right):

https://codereview.chromium.org/1866433002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/RawResource.h:115: virtual void
redirectReceivedAndNotFollowed() {}
On 2016/04/06 07:26:37, Mike West wrote:
> Tiny tiny nit: This sounds clunky, but I don't have a better suggestion.
> `redirectBlocked`? Idunno.

Sure, redirectBlocked works for me. Done.

[estark, 2016-04-06 17:53:04]

The CQ bit was checked by estark@chromium.org

[estark, 2016-04-06 17:53:05]

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/1866433002/#ps20001
(title: "rename redirectReceivedAndNotFollowed() to redirectBlocked()")

[commit-bot: I haz the power, 2016-04-06 17:53:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1866433002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1866433002/20001

[commit-bot: I haz the power, 2016-04-06 20:06:00]

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

[commit-bot: I haz the power, 2016-04-06 20:06:01]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[estark, 2016-04-06 20:08:27]

The CQ bit was checked by estark@chromium.org

[commit-bot: I haz the power, 2016-04-06 20:08:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1866433002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1866433002/20001

[commit-bot: I haz the power, 2016-04-07 00:05:20]

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

[commit-bot: I haz the power, 2016-04-07 00:05:21]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[estark, 2016-04-07 04:32:44]

The CQ bit was checked by estark@chromium.org

[commit-bot: I haz the power, 2016-04-07 04:32:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1866433002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1866433002/20001

[commit-bot: I haz the power, 2016-04-07 05:14:24]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-07 05:15:52]

Description was changed from

==========
Use RequestContext to apply CSP in FrameFetchContext

Instead of a big switch statement on Resource::Type in
FrameFetchContext::canRequest(), use
ContentSecurityPolicy::allowRequest(), which decides which directives to
apply based on the RequestContext.

A nice-ish side effect of this is that DocumentThreadableLoader no
longer needs to know about (much less be responsible for enforcing)
CSP. Previously, DocumentThreadableLoader needed to enforce CSP because
Resource::Type doesn't cover things like EventSource, so
FrameFetchContext::canRequest() just let them pass through as raw
resources. Now that FrameFetchContext::canRequest() is based on
RequestContext, and RequestContext covers things like EventSource, the
proper CSP directive can be applied, instead of DocumentThreadableLoader
needing to do special CSP checks.

I say "nice-ish" instead of "nice" because kicking CSP checks out of
DocumentThreadableLoader introduces a slight complication: EventSource
and friends want to know about redirects that are blocked, so that they
can product an error for Javascript to consume. (We have layout tests
that test for this error.) So I introduced a new set of hooks for
RawResourceClients to get notified when ResourceFetcher declines to
follow a redirect. (The previous flow for EventSource and friends was
that ResourceFetcher would agree to follow the redirect, but
DocumentThreadableLoader would block it and notify the
RawResourceClient. Now that ResourceFetcher is deciding based on
RequestContexts, ResourceFetcher will want to block the redirect.)

BUG=474412
==========

to

==========
Use RequestContext to apply CSP in FrameFetchContext

Instead of a big switch statement on Resource::Type in
FrameFetchContext::canRequest(), use
ContentSecurityPolicy::allowRequest(), which decides which directives to
apply based on the RequestContext.

A nice-ish side effect of this is that DocumentThreadableLoader no
longer needs to know about (much less be responsible for enforcing)
CSP. Previously, DocumentThreadableLoader needed to enforce CSP because
Resource::Type doesn't cover things like EventSource, so
FrameFetchContext::canRequest() just let them pass through as raw
resources. Now that FrameFetchContext::canRequest() is based on
RequestContext, and RequestContext covers things like EventSource, the
proper CSP directive can be applied, instead of DocumentThreadableLoader
needing to do special CSP checks.

I say "nice-ish" instead of "nice" because kicking CSP checks out of
DocumentThreadableLoader introduces a slight complication: EventSource
and friends want to know about redirects that are blocked, so that they
can product an error for Javascript to consume. (We have layout tests
that test for this error.) So I introduced a new set of hooks for
RawResourceClients to get notified when ResourceFetcher declines to
follow a redirect. (The previous flow for EventSource and friends was
that ResourceFetcher would agree to follow the redirect, but
DocumentThreadableLoader would block it and notify the
RawResourceClient. Now that ResourceFetcher is deciding based on
RequestContexts, ResourceFetcher will want to block the redirect.)

BUG=474412

Committed: https://crrev.com/077fcfa04ed8d166cdd51bfd6f9ac36a2ccb8b5d
Cr-Commit-Position: refs/heads/master@{#385669}
==========

[commit-bot: I haz the power, 2016-04-07 05:15:53]

Patchset 2 (id:??) landed as
https://crrev.com/077fcfa04ed8d166cdd51bfd6f9ac36a2ccb8b5d
Cr-Commit-Position: refs/heads/master@{#385669}