Enforce referrer policies for workers

Enforce referrer policies for workers

When requesting a resource on behalf of a worker, use the worker's
referrer policy instead of the default.

This CL does three things:
1. Move the referrer policy from Document to ExecutionContext, so that
other contexts (i.e. WorkerGlobalScope) get referrer policies too.
2. When binding a CSP to an ExecutionContext, set the referrer policy
for all types of contexts, not just Document.
3. When setting up a MainThreadBridge to load a resource from a worker,
use the worker's referrer policy to generate the referrer for the
request, instead of always using the default referrer policy.

Added layout tests to check that workers can have a referrer policy
different from the document's.

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

[estark, 2015-05-01 14:16:44]

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

[estark, 2015-05-01 14:17:47]

Mike, Jochen: please take a look. Thanks!

[Mike West, 2015-05-01 14:34:29]

This is looking great, thanks for taking the time to work this out.

LGTM % the `setReferrerPolicy` override, which seems unnecessary.

https://codereview.chromium.org/1117203002/diff/1/LayoutTests/http/tests/secu...
File LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker.php
(right):

https://codereview.chromium.org/1117203002/diff/1/LayoutTests/http/tests/secu...
LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker.php:87:
var xhr = new XMLHttpRequest;
Tiny tiny nit: It would be nice for our tests to start using `fetch` rather than
`XMLHttpRequest`. You don't need to change this CL at all, but think about it
going forward.

https://codereview.chromium.org/1117203002/diff/1/Source/core/dom/Document.cpp
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/1117203002/diff/1/Source/core/dom/Document.cp...
Source/core/dom/Document.cpp:3074: ExecutionContext::setReferrerPolicy(policy);
Isn't `Document` an `ExecutionContext`? Can't we just move this whole
implementation over there? I'd prefer to stick the UseCounter in one place that
catches both Document and Worker-based policy decisions.

[estark, 2015-05-01 18:19:55]

https://codereview.chromium.org/1117203002/diff/1/LayoutTests/http/tests/secu...
File LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker.php
(right):

https://codereview.chromium.org/1117203002/diff/1/LayoutTests/http/tests/secu...
LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker.php:87:
var xhr = new XMLHttpRequest;
On 2015/05/01 14:34:29, Mike West (traveling. slow.) wrote:
> Tiny tiny nit: It would be nice for our tests to start using `fetch` rather
than
> `XMLHttpRequest`. You don't need to change this CL at all, but think about it
> going forward.

Acknowledged.

https://codereview.chromium.org/1117203002/diff/1/Source/core/dom/Document.cpp
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/1117203002/diff/1/Source/core/dom/Document.cp...
Source/core/dom/Document.cpp:3074: ExecutionContext::setReferrerPolicy(policy);
On 2015/05/01 14:34:29, Mike West (traveling. slow.) wrote:
> Isn't `Document` an `ExecutionContext`? Can't we just move this whole
> implementation over there? I'd prefer to stick the UseCounter in one place
that
> catches both Document and Worker-based policy decisions.

Oh ok, yeah, that works. I wasn't sure if we wanted to keep the UseCounter
counting exactly what it's counting now (document policies only) or want to
count both documents and workers. Done.

[Mike West, 2015-05-01 21:45:17]

Still LGTM. Thanks!

[jochen (gone - plz use gerrit), 2015-05-04 07:20:31]

overall lgtm

thanks for doing this

https://codereview.chromium.org/1117203002/diff/20001/LayoutTests/http/tests/...
File LayoutTests/http/tests/security/referrer-policy-worker-has-referrer.html
(right):

https://codereview.chromium.org/1117203002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/referrer-policy-worker-has-referrer.html:4:
<script src="/resources/testharness.js"></script>
nit 4sp indent

https://codereview.chromium.org/1117203002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/referrer-policy-worker-has-referrer.html:12: var
worker = new Worker(workerUrl);
have you tried creating a shared worker? I'd expect that with your patch this
should work as well.

https://codereview.chromium.org/1117203002/diff/20001/Source/core/dom/Executi...
File Source/core/dom/ExecutionContext.cpp (right):

https://codereview.chromium.org/1117203002/diff/20001/Source/core/dom/Executi...
Source/core/dom/ExecutionContext.cpp:257: if (m_referrerPolicy !=
ReferrerPolicyDefault)
i notice this is just copy/paste, but shouldn't this be == ?

[Mike West, 2015-05-04 14:17:21]

https://codereview.chromium.org/1117203002/diff/20001/Source/core/dom/Executi...
File Source/core/dom/ExecutionContext.cpp (right):

https://codereview.chromium.org/1117203002/diff/20001/Source/core/dom/Executi...
Source/core/dom/ExecutionContext.cpp:257: if (m_referrerPolicy !=
ReferrerPolicyDefault)
On 2015/05/04 07:20:31, jochen wrote:
> i notice this is just copy/paste, but shouldn't this be == ?

I don't think so. If the referrer policy is the default policy (as opposed to
`ReferrerPolicyNoReferrerWhenDowngrade`), then no policy has been set. If a
policy has been set, it will be something other than "default", so we log a
reset.

Also, I think we have enough data here showing that resets are common to simply
throw away the merge algorithm in the spec and define one that matches the web.

[jochen (gone - plz use gerrit), 2015-05-04 14:36:12]

https://codereview.chromium.org/1117203002/diff/20001/Source/core/dom/Executi...
File Source/core/dom/ExecutionContext.cpp (right):

https://codereview.chromium.org/1117203002/diff/20001/Source/core/dom/Executi...
Source/core/dom/ExecutionContext.cpp:257: if (m_referrerPolicy !=
ReferrerPolicyDefault)
On 2015/05/04 14:17:20, Mike West (traveling. slow.) wrote:
> On 2015/05/04 07:20:31, jochen wrote:
> > i notice this is just copy/paste, but shouldn't this be == ?
> 
> I don't think so. If the referrer policy is the default policy (as opposed to
> `ReferrerPolicyNoReferrerWhenDowngrade`), then no policy has been set. If a
> policy has been set, it will be something other than "default", so we log a
> reset.
> 
> Also, I think we have enough data here showing that resets are common to
simply
> throw away the merge algorithm in the spec and define one that matches the
web.

ah, right

[estark, 2015-05-04 18:09:22]

https://codereview.chromium.org/1117203002/diff/20001/LayoutTests/http/tests/...
File LayoutTests/http/tests/security/referrer-policy-worker-has-referrer.html
(right):

https://codereview.chromium.org/1117203002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/referrer-policy-worker-has-referrer.html:4:
<script src="/resources/testharness.js"></script>
On 2015/05/04 07:20:31, jochen wrote:
> nit 4sp indent

Done.

https://codereview.chromium.org/1117203002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/referrer-policy-worker-has-referrer.html:12: var
worker = new Worker(workerUrl);
On 2015/05/04 07:20:31, jochen wrote:
> have you tried creating a shared worker? I'd expect that with your patch this
> should work as well.

Unfortunately it doesn't quite work yet, because we're still using the
document's CSP when creating shared workers:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

However, once I fix that, then it should work and I can come back and make this
test create a shared worker too.

[estark, 2015-05-04 18:09:31]

The CQ bit was checked by estark@chromium.org

[estark, 2015-05-04 18:09:32]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1117203002/#ps40001
(title: "fix layout test indentation")

[commit-bot: I haz the power, 2015-05-04 18:09:57]

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

[commit-bot: I haz the power, 2015-05-04 19:49:06]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194885