WebURLLoaderMock should set a request context for redirects.

WebURLLoaderMock should set a request context for redirects.

BUG=417841
Committed: https://crrev.com/11f1567dce41d3f15481fa2df0f6a334455e7adb
Cr-Commit-Position: refs/heads/master@{#297397}

[Mike West, 2014-09-29 07:51:35]

mkwst@chromium.org changed reviewers:
+ phajdan@google.com

[Mike West, 2014-09-29 07:51:48]

mkwst@chromium.org changed reviewers:
+ phajdan.jr@chromium.org
- phajdan@google.com

[Mike West, 2014-09-29 07:52:22]

Would you mind taking a look at this 1-line change? It fixes the test failures
that are blocking https://codereview.chromium.org/608733002/. :)

Thanks!

-mike

[Paweł Hajdan Jr., 2014-09-29 08:49:18]

I'm not familiar with exact code in question.

Could you add a reviewer which has more knowledge of the specific code, and then
I can happily use my OWNER stamp?

Even though it looks like a simple change, a reviewer with more specific
knowledge of the code might have some further suggestions.

[Mike West, 2014-09-29 09:00:46]

On 2014/09/29 08:49:18, Paweł Hajdan Jr. wrote:
> Even though it looks like a simple change, a reviewer with more specific
> knowledge of the code might have some further suggestions.

This file hasn't been touched since June, and hasn't been modified in
non-trivial ways since before Sep 2013, when it was renamed. Moreover, I
introduced the WebURLRequest::RequestContext concept, and I think I'm the only
one actively adding patches that use it.

Perhaps I could explain the background of the change in a way which would make
you more likely to just stamp it? :)

There's an ASSERT (which I added) on the Blink side of the Fetch implementation
which ensures that "raw" requests have a request context. Since we're now using
RequestContext for security checks, this seems like a reasonable thing to want
to verify.

That ASSERT is currently broken by this code, but it's hidden by the fact that
we force all redirect requests to the "internal" context. That blanket policy is
bad, as it breaks mixed content checking, hence
https://codereview.chromium.org/608733002/, which exposes this error.

This mock creates a new WebURLRequest object in order to pretend to execute a
redirect. Since that new object doesn't have a RequestContext assigned, it
triggers the ASSERT. This patch explicitly labels that new WebURLRequest as an
"internal" context in order to satisfy the ASSERT's conditions.

This is a safe thing to do, as a) it only effects tests, b) after
https://codereview.chromium.org/608733002/ we will have we have mixed content
checking test coverage in Blink which ensures that redirects do the right thing.

[Paweł Hajdan Jr., 2014-09-30 10:02:57]

LGTM

[Mike West, 2014-09-30 10:11:48]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-09-30 10:12:17]

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

[commit-bot: I haz the power, 2014-09-30 10:16:52]

Committed patchset #1 (id:1) as cde2a37a70324119240bbae15a80e1f70a7cb45a

[commit-bot: I haz the power, 2014-09-30 10:17:22]

Patchset 1 (id:??) landed as
https://crrev.com/11f1567dce41d3f15481fa2df0f6a334455e7adb
Cr-Commit-Position: refs/heads/master@{#297397}