Use Referrer-Policy headers for CSS stylesheets

Use Referrer-Policy headers for CSS stylesheets

Link to intent to ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/YkXg6ZkW2Bs/6aDLAs0qBwAJ

BUG=676645
R=estark@chromium.org

Review-Url: https://codereview.chromium.org/2780533002
Cr-Commit-Position: refs/heads/master@{#460344}
Committed: https://chromium.googlesource.com/chromium/src/+/57c4423dceffc997459ae34640abdfc1f067cc2e

[jochen (gone - plz use gerrit), 2017-03-27 15:22:23]

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

[jochen (gone - plz use gerrit), 2017-03-27 15:23:01]

still need to fix the test failure for inline stylesheets, and add a test for
XSL documents, but the overall structure is probably going to look like this

[commit-bot: I haz the power, 2017-03-27 15:23:10]

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 16:16:42]

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

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

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jochen (gone - plz use gerrit), 2017-03-28 10:44:49]

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

[commit-bot: I haz the power, 2017-03-28 10:44:59]

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-28 11:05:29]

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

[commit-bot: I haz the power, 2017-03-28 11:05:30]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[estark, 2017-03-28 11:26:23]

(haven't looked at the tests yet)

https://codereview.chromium.org/2780533002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp (right):

https://codereview.chromium.org/2780533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp:306:
cssPreloadScanner.scan(chunk, SegmentedString(), preloads,
I don't know this code very well, so I might be misunderstanding, but it looks
like these preload requests wouldn't get the Referrer-Policy header applied?

[jochen (gone - plz use gerrit), 2017-03-28 13:08:34]

https://codereview.chromium.org/2780533002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp (right):

https://codereview.chromium.org/2780533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp:306:
cssPreloadScanner.scan(chunk, SegmentedString(), preloads,
On 2017/03/28 at 11:26:23, estark wrote:
> I don't know this code very well, so I might be misunderstanding, but it looks
like these preload requests wouldn't get the Referrer-Policy header applied?

Excellent catch!

I don't know that code either, so I wrote a test for it

[jochen (gone - plz use gerrit), 2017-03-28 13:10:23]

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

[commit-bot: I haz the power, 2017-03-28 13:10:41]

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

[jochen (gone - plz use gerrit), 2017-03-28 13:15:27]

Description was changed from

==========
Use Referrer-Policy headers for CSS stylesheets

BUG=676645
R=estark@chromium.org
==========

to

==========
Use Referrer-Policy headers for CSS stylesheets

Link to intent to ship:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/YkXg6ZkW2Bs/6a...

BUG=676645
R=estark@chromium.org
==========

[commit-bot: I haz the power, 2017-03-28 15:41:38]

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

[commit-bot: I haz the power, 2017-03-28 15:41:39]

Dry run: This issue passed the CQ dry run.

[estark, 2017-03-28 18:17:21]

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/referrer-policy/css-integration/external-import-stylesheet.html
(right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/referrer-policy/css-integration/external-import-stylesheet.html:22:
var crossOriginUrlPrefix = location.protocol + "//www1." + location.hostname +
":" + location.port;
nit: naming convention differs here from line 20
(I'm not sure which is correct)

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/referrer-policy/css-integration/external-import-stylesheet.html:25:
var cssReferrer = urlPrefix +
"/referrer-policy/generic/subresource/stylesheet.py?id=" + id;
I've been staring at this test and can't figure out how it works. :) Why is
|cssReferrer| different than |cssUrl|? (no "&import-rule" on the former) I'd
expect them to be the same (that is, I'd expect the referrer to be equal to the
stylesheet's location). And why does the XHR to |imgUrl| use the stylesheet's
referrer policy? I'd expect the image when requested from the stylesheet to use
the stylesheet's policy, but the XHR to |imgUrl| to use the document's policy.

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/referrer-policy/css-integration/external-stylesheet.html
(right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/referrer-policy/css-integration/external-stylesheet.html:31:
queryXhr.bind(this, imgUrl,
Same here, I'm confused why this XHR uses the stylesheet's policy.

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp (right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp:316:
cssPreloadScanner.setReferrerPolicy(referrerPolicy);
Hmmm... I think this will end up using the document's URL as the referrer URL:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/pars...

Maybe the preload scanner needs a setReferrerURL() in addition to
setReferrerPolicy()?

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp
(right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp:46: bool
m_storeReferrer;
unused

[jochen (gone - plz use gerrit), 2017-03-28 18:49:37]

yeah, the wpt tests are pretty magic. The stylesheet references an image that
stores the requests headers server side.

When I later (after a timeout of 2s, yay, there's no way to figure out when
resources referenced from css are loaded...) request the image again via XHR
(adding ?report-header), it replies back with a json of the headers that were
used to request the image in the first place via CSS...

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp (right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp:316:
cssPreloadScanner.setReferrerPolicy(referrerPolicy);
On 2017/03/28 at 18:17:21, estark wrote:
> Hmmm... I think this will end up using the document's URL as the referrer URL:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/pars...
> 
> Maybe the preload scanner needs a setReferrerURL() in addition to
setReferrerPolicy()?

I changed that code (see two files down)

[estark, 2017-03-28 18:59:54]

Ahh, thanks for the wpt explanation. Tricky! lgtm % previous nits

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp (right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp:316:
cssPreloadScanner.setReferrerPolicy(referrerPolicy);
On 2017/03/28 18:49:37, jochen wrote:
> On 2017/03/28 at 18:17:21, estark wrote:
> > Hmmm... I think this will end up using the document's URL as the referrer
URL:
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/pars...
> > 
> > Maybe the preload scanner needs a setReferrerURL() in addition to
> setReferrerPolicy()?
> 
> I changed that code (see two files down)

Doh, sorry!

[jochen (gone - plz use gerrit), 2017-03-29 08:13:11]

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/referrer-policy/css-integration/external-import-stylesheet.html
(right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/referrer-policy/css-integration/external-import-stylesheet.html:22:
var crossOriginUrlPrefix = location.protocol + "//www1." + location.hostname +
":" + location.port;
On 2017/03/28 at 18:17:20, estark wrote:
> nit: naming convention differs here from line 20
> (I'm not sure which is correct)

I opted for this_style

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp
(right):

https://codereview.chromium.org/2780533002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp:46: bool
m_storeReferrer;
On 2017/03/28 at 18:17:21, estark wrote:
> unused

deleted

[jochen (gone - plz use gerrit), 2017-03-29 08:24:00]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2017-03-29 08:24:01]

also added a README.md to the css-integration directory to document the general
structure of the tests.

[jochen (gone - plz use gerrit), 2017-03-29 08:24:01]

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

[commit-bot: I haz the power, 2017-03-29 08:24:11]

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 10:46:39]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490775840897540,
"parent_rev": "88d07c763d35621da2c62b9f18822109bbd7a261", "commit_rev":
"57c4423dceffc997459ae34640abdfc1f067cc2e"}

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

Description was changed from

==========
Use Referrer-Policy headers for CSS stylesheets

Link to intent to ship:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/YkXg6ZkW2Bs/6a...

BUG=676645
R=estark@chromium.org
==========

to

==========
Use Referrer-Policy headers for CSS stylesheets

Link to intent to ship:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/YkXg6ZkW2Bs/6a...

BUG=676645
R=estark@chromium.org

Review-Url: https://codereview.chromium.org/2780533002
Cr-Commit-Position: refs/heads/master@{#460344}
Committed:
https://chromium.googlesource.com/chromium/src/+/57c4423dceffc997459ae34640ab...
==========

[commit-bot: I haz the power, 2017-03-29 10:47:31]

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