Send net's ReferrerPolicy back to Blink while following redirects

Send net's ReferrerPolicy back to Blink while following redirects

//net might modify a request's referrer policy while following server
redirects that contain a Referrer-Policy header. Blink learns about the
redirects via the ResourceMsg_ReceivedRedirect IPC, and in some cases
(notably, CORS-enabled redirects), Blink chooses to tell //net not to
follow the redirect but instead kicks off a new request to follow the
redirect. (See DocumentThreadableLoader::redirectReceived().) In these
cases, Blink needs to know the referrer policy that //net assigned to
the request, based on the Referrer-Policy header that was received, so
that, for example, Blink can apply that referrer policy while following
a CORS-enabled redirect itself.

This CL changes WebURLLoaderImpl::PopulateURLRequestForRedirect() to
translate the request's net::URLRequest::ReferrerPolicy into a
WebReferrerPolicy that gets sent to Blink.

So that this translation can always be accurate, this CL also stops
using NEVER_CLEAR_REFERRER for multiple referrer policies ('origin',
'unsafe-url', and 'no-referrer') and instead always uses the
net::URLRequest::ReferrerPolicy corresponding to the actual referrer
policy for the request. That is: ORIGIN and NO_REFERRER are no longer
used only internally by URLRequest for processing Referrer-Policy
headers, but are now values that callers should use to convey a referrer
policy of 'origin' or 'no-referrer', so that the
net::URLRequest::ReferrerPolicy of a request can always be relied upon
to translate back to a WebReferrerPolicy.

BUG=628457
Committed: https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b
Cr-Commit-Position: refs/heads/master@{#424203}

[estark, 2016-10-05 23:24:55]

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

[commit-bot: I haz the power, 2016-10-05 23:25:37]

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

[estark, 2016-10-05 23:31:06]

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

[commit-bot: I haz the power, 2016-10-05 23:32:28]

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, 2016-10-06 02:02:29]

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

[commit-bot: I haz the power, 2016-10-06 02:02:30]

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

[estark, 2016-10-06 02:15:09]

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

[estark, 2016-10-06 02:15:09]

mmenke, jochen, PTAL?

[jochen (gone - plz use gerrit), 2016-10-06 12:20:59]

lgtm

[mmenke, 2016-10-06 15:22:54]

On 2016/10/06 12:20:59, jochen wrote:
> lgtm

LGTM.  Do we want to pass it back to Blink in the ServiceWorker case?  I think
this would need more work if we wanted to do it there.

i.e., renderer makes a request, SW intercepts, makes a request, it's redirected
to another URL, with another policy, that new redirect policy might make it to
SW, but the original renderer never sees the redirect, let alone the new policy.

[estark, 2016-10-06 17:45:45]

On 2016/10/06 15:22:54, mmenke wrote:
> On 2016/10/06 12:20:59, jochen wrote:
> > lgtm
> 
> LGTM.  Do we want to pass it back to Blink in the ServiceWorker case?  I think
> this would need more work if we wanted to do it there.
> 
> i.e., renderer makes a request, SW intercepts, makes a request, it's
redirected
> to another URL, with another policy, that new redirect policy might make it to
> SW, but the original renderer never sees the redirect, let alone the new
policy.

When I last thought about this, I had convinced myself that we don't need to do
anything special for ServiceWorkers, because redirects skip service workers: see
step 4.2 of https://fetch.spec.whatwg.org/#concept-http-fetch.

That might be an oversimplification though; I feel like I might not be
understanding the scenario you have in mind? IIUC you're talking about a
scenario in which the redirect is followed in the net stack; when that's the
case, I don't think we care about sending the updated referrer policy to Blink.

[mmenke, 2016-10-10 14:52:42]

On 2016/10/06 17:45:45, estark wrote:
> On 2016/10/06 15:22:54, mmenke wrote:
> > On 2016/10/06 12:20:59, jochen wrote:
> > > lgtm
> > 
> > LGTM.  Do we want to pass it back to Blink in the ServiceWorker case?  I
think
> > this would need more work if we wanted to do it there.
> > 
> > i.e., renderer makes a request, SW intercepts, makes a request, it's
> redirected
> > to another URL, with another policy, that new redirect policy might make it
to
> > SW, but the original renderer never sees the redirect, let alone the new
> policy.
> 
> When I last thought about this, I had convinced myself that we don't need to
do
> anything special for ServiceWorkers, because redirects skip service workers:
see
> step 4.2 of https://fetch.spec.whatwg.org/#concept-http-fetch.
> 
> That might be an oversimplification though; I feel like I might not be
> understanding the scenario you have in mind? IIUC you're talking about a
> scenario in which the redirect is followed in the net stack; when that's the
> case, I don't think we care about sending the updated referrer policy to
Blink.

Are you waiting for me to respond to this?  You're right that I'm thinking about
the case where the network stack followed redirects for a request issued from
the SW.

[estark, 2016-10-10 17:38:25]

On 2016/10/10 14:52:42, mmenke wrote:
> On 2016/10/06 17:45:45, estark wrote:
> > On 2016/10/06 15:22:54, mmenke wrote:
> > > On 2016/10/06 12:20:59, jochen wrote:
> > > > lgtm
> > > 
> > > LGTM.  Do we want to pass it back to Blink in the ServiceWorker case?  I
> think
> > > this would need more work if we wanted to do it there.
> > > 
> > > i.e., renderer makes a request, SW intercepts, makes a request, it's
> > redirected
> > > to another URL, with another policy, that new redirect policy might make
it
> to
> > > SW, but the original renderer never sees the redirect, let alone the new
> > policy.
> > 
> > When I last thought about this, I had convinced myself that we don't need to
> do
> > anything special for ServiceWorkers, because redirects skip service workers:
> see
> > step 4.2 of https://fetch.spec.whatwg.org/#concept-http-fetch.
> > 
> > That might be an oversimplification though; I feel like I might not be
> > understanding the scenario you have in mind? IIUC you're talking about a
> > scenario in which the redirect is followed in the net stack; when that's the
> > case, I don't think we care about sending the updated referrer policy to
> Blink.
> 
> Are you waiting for me to respond to this?  You're right that I'm thinking
about
> the case where the network stack followed redirects for a request issued from
> the SW.

Oh ok, cool, thanks.

[estark, 2016-10-10 17:38:30]

The CQ bit was checked by estark@chromium.org

[commit-bot: I haz the power, 2016-10-10 17:38:47]

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, 2016-10-10 19:05:02]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-10 19:08:50]

Description was changed from

==========
Send net's ReferrerPolicy back to Blink while following redirects

//net might modify a request's referrer policy while following server
redirects that contain a Referrer-Policy header. Blink learns about the
redirects via the ResourceMsg_ReceivedRedirect IPC, and in some cases
(notably, CORS-enabled redirects), Blink chooses to tell //net not to
follow the redirect but instead kicks off a new request to follow the
redirect. (See DocumentThreadableLoader::redirectReceived().) In these
cases, Blink needs to know the referrer policy that //net assigned to
the request, based on the Referrer-Policy header that was received, so
that, for example, Blink can apply that referrer policy while following
a CORS-enabled redirect itself.

This CL changes WebURLLoaderImpl::PopulateURLRequestForRedirect() to
translate the request's net::URLRequest::ReferrerPolicy into a
WebReferrerPolicy that gets sent to Blink.

So that this translation can always be accurate, this CL also stops
using NEVER_CLEAR_REFERRER for multiple referrer policies ('origin',
'unsafe-url', and 'no-referrer') and instead always uses the
net::URLRequest::ReferrerPolicy corresponding to the actual referrer
policy for the request. That is: ORIGIN and NO_REFERRER are no longer
used only internally by URLRequest for processing Referrer-Policy
headers, but are now values that callers should use to convey a referrer
policy of 'origin' or 'no-referrer', so that the
net::URLRequest::ReferrerPolicy of a request can always be relied upon
to translate back to a WebReferrerPolicy.

BUG=628457
==========

to

==========
Send net's ReferrerPolicy back to Blink while following redirects

//net might modify a request's referrer policy while following server
redirects that contain a Referrer-Policy header. Blink learns about the
redirects via the ResourceMsg_ReceivedRedirect IPC, and in some cases
(notably, CORS-enabled redirects), Blink chooses to tell //net not to
follow the redirect but instead kicks off a new request to follow the
redirect. (See DocumentThreadableLoader::redirectReceived().) In these
cases, Blink needs to know the referrer policy that //net assigned to
the request, based on the Referrer-Policy header that was received, so
that, for example, Blink can apply that referrer policy while following
a CORS-enabled redirect itself.

This CL changes WebURLLoaderImpl::PopulateURLRequestForRedirect() to
translate the request's net::URLRequest::ReferrerPolicy into a
WebReferrerPolicy that gets sent to Blink.

So that this translation can always be accurate, this CL also stops
using NEVER_CLEAR_REFERRER for multiple referrer policies ('origin',
'unsafe-url', and 'no-referrer') and instead always uses the
net::URLRequest::ReferrerPolicy corresponding to the actual referrer
policy for the request. That is: ORIGIN and NO_REFERRER are no longer
used only internally by URLRequest for processing Referrer-Policy
headers, but are now values that callers should use to convey a referrer
policy of 'origin' or 'no-referrer', so that the
net::URLRequest::ReferrerPolicy of a request can always be relied upon
to translate back to a WebReferrerPolicy.

BUG=628457

Committed: https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b
Cr-Commit-Position: refs/heads/master@{#424203}
==========

[commit-bot: I haz the power, 2016-10-10 19:08:51]

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