Add basic Referrer-Policy support for Documents

Add basic Referrer-Policy support for Documents

This CL adds (runtime-enabled) support for the Referrer-Policy header,
currently only supported for Documents.

Intent to Implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Umj9iVRJM70

BUG=619228
Committed: https://crrev.com/b89eeea12cc26b0164a89398b6c3e519b3d9fa4f
Cr-Commit-Position: refs/heads/master@{#401709}

[estark, 2016-06-17 23:39:46]

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

[estark, 2016-06-17 23:39:46]

jochen, PTAL?

cc mkwst because why not

[jochen (gone - plz use gerrit), 2016-06-21 07:37:26]

lgtm

[Mike West, 2016-06-21 07:40:02]

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

[Mike West, 2016-06-21 07:40:03]

LGTM % one organizational question.

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/referrer-policy-header-then-csp.php
(right):

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/referrer-policy-header-then-csp.php:6:
<meta http-equiv="Content-Security-Policy" content="script-src 'unsafe-inline'
*; child-src *; referrer no-referrer">
Did you add metrics for this already? It would be lovely to remove it. :/

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:521: ReferrerPolicy
referrerPolicy = parseReferrerPolicyHeader(referrerPolicyHeader);
Could you call `Document::processReferrerPolicy(referrerPolicyHeader)` instead,
and either inline the new parsing code there (or in
`SecurityPolicy::referrerPolicyFromString(...)`, I suppose?), making the header
behave exactly the same way as `<meta>`? This would have the impact of allowing
comma-separated tokens in the `<meta>` content, but I think I'd prefer that to
having two separate code paths.

[estark, 2016-06-21 17:41:33]

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/referrer-policy-header-then-csp.php
(right):

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/referrer-policy-header-then-csp.php:6:
<meta http-equiv="Content-Security-Policy" content="script-src 'unsafe-inline'
*; child-src *; referrer no-referrer">
On 2016/06/21 07:40:03, Mike West wrote:
> Did you add metrics for this already? It would be lovely to remove it. :/

No, but I promise I will soon! I filed a bug for it (crbug.com/621219)

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:521: ReferrerPolicy
referrerPolicy = parseReferrerPolicyHeader(referrerPolicyHeader);
On 2016/06/21 07:40:03, Mike West wrote:
> Could you call `Document::processReferrerPolicy(referrerPolicyHeader)`
instead,
> and either inline the new parsing code there (or in
> `SecurityPolicy::referrerPolicyFromString(...)`, I suppose?), making the
header
> behave exactly the same way as `<meta>`? This would have the impact of
allowing
> comma-separated tokens in the `<meta>` content, but I think I'd prefer that to
> having two separate code paths.

Hmm. If I put the new parsing code in Document::processReferrerPolicy(), then I
can't easily reuse it for processing headers delivered on Worker scripts. If I
put it in SecurityPolicy::referrerPolicyFromString(), then that means
comma-separated values will also be allowed for the 'referrerpolicy' attribute,
which seems weird to me.

I could move Document::processReferrerPolicy() to be a method on
ExecutionContext instead of Document and do the new parsing there, so that I can
call it for both Documents and Workers. WDYT of that plan?

[Mike West, 2016-06-22 18:57:04]

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2075053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:521: ReferrerPolicy
referrerPolicy = parseReferrerPolicyHeader(referrerPolicyHeader);
On 2016/06/21 at 17:41:33, estark wrote:
> On 2016/06/21 07:40:03, Mike West wrote:
> > Could you call `Document::processReferrerPolicy(referrerPolicyHeader)`
instead,
> > and either inline the new parsing code there (or in
> > `SecurityPolicy::referrerPolicyFromString(...)`, I suppose?), making the
header
> > behave exactly the same way as `<meta>`? This would have the impact of
allowing
> > comma-separated tokens in the `<meta>` content, but I think I'd prefer that
to
> > having two separate code paths.
> 
> Hmm. If I put the new parsing code in Document::processReferrerPolicy(), then
I can't easily reuse it for processing headers delivered on Worker scripts. If I
put it in SecurityPolicy::referrerPolicyFromString(), then that means
comma-separated values will also be allowed for the 'referrerpolicy' attribute,
which seems weird to me.
> 
> I could move Document::processReferrerPolicy() to be a method on
ExecutionContext instead of Document and do the new parsing there, so that I can
call it for both Documents and Workers. WDYT of that plan?

I trust your judgement. I think it's probably reasonable to just accept `a, b,
c` in `<meta>` and send them through the same processing internally, but if
you'd prefer not to do that, more power to you.

LGTM either way.

[estark, 2016-06-23 03:09:20]

Alrighty, I moved the new parsing logic into
ExecutionContext::parseAndSetReferrerPolicy(), replacing
Document::processReferrerPolicy(). This'll let me re-use it for workers, and
also make meta tags accept comma-separated values, without affecting how
'referrerpolicy' attributes on anchors, etc. behave.

Won't CQ it for a few hours until I look at it again with a fresh brain, so feel
free to re-review in the meantime if you feel so inclined.

Thanks!

[estark, 2016-06-23 03:09:20]

Alrighty, I moved the new parsing logic into
ExecutionContext::parseAndSetReferrerPolicy(), replacing
Document::processReferrerPolicy(). This'll let me re-use it for workers, and
also make meta tags accept comma-separated values, without affecting how
'referrerpolicy' attributes on anchors, etc. behave.

Won't CQ it for a few hours until I look at it again with a fresh brain, so feel
free to re-review in the meantime if you feel so inclined.

Thanks!

[estark, 2016-06-23 17:31:52]

The CQ bit was checked by estark@chromium.org

[estark, 2016-06-23 17:31:52]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2075053003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-23 17:32:04]

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

[commit-bot: I haz the power, 2016-06-23 18:59:24]

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

[commit-bot: I haz the power, 2016-06-23 18:59:25]

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-06-23 19:05:57]

The CQ bit was checked by estark@chromium.org

[estark, 2016-06-23 19:05:57]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2075053003/#ps100001
(title: "update test expectation for tweaked console message")

[commit-bot: I haz the power, 2016-06-23 19:06:27]

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

[commit-bot: I haz the power, 2016-06-23 20:43:51]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-23 20:45:30]

Description was changed from

==========
Add basic Referrer-Policy support for Documents

This CL adds (runtime-enabled) support for the Referrer-Policy header,
currently only supported for Documents.

Intent to Implement:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Umj9iVRJM70

BUG=619228
==========

to

==========
Add basic Referrer-Policy support for Documents

This CL adds (runtime-enabled) support for the Referrer-Policy header,
currently only supported for Documents.

Intent to Implement:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Umj9iVRJM70

BUG=619228

Committed: https://crrev.com/b89eeea12cc26b0164a89398b6c3e519b3d9fa4f
Cr-Commit-Position: refs/heads/master@{#401709}
==========

[commit-bot: I haz the power, 2016-06-23 20:45:31]

Patchset 6 (id:??) landed as
https://crrev.com/b89eeea12cc26b0164a89398b6c3e519b3d9fa4f
Cr-Commit-Position: refs/heads/master@{#401709}