Perform CORS access checks on Beacon redirects.

Perform CORS access checks on Beacon redirects.

Beacon "fetches" follow CORS, which means that a CORS check must be
performed on the redirect response.

R=mkwst
BUG=468527
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192294

[sof, 2015-03-19 16:28:56]

sigbjornf@opera.com changed reviewers:
+ japhet@chromium.org, mkwst@chromium.org

[sof, 2015-03-19 16:28:56]

please take a look.

[Mike West, 2015-03-20 16:51:27]

LGTM % nits.

https://codereview.chromium.org/1016373002/diff/1/LayoutTests/http/tests/navi...
File LayoutTests/http/tests/navigation/beacon-cross-origin-redirect.html
(right):

https://codereview.chromium.org/1016373002/diff/1/LayoutTests/http/tests/navi...
LayoutTests/http/tests/navigation/beacon-cross-origin-redirect.html:16:
shouldBeTrue('navigator.sendBeacon("http://127.0.0.1:8080/navigation/resources/redirection-response.php?status=302&target=non-existent.php",
"CrossOrigin");');
Would you mind adding a mixed content test, since we apparently don't have one?
You could just copy/paste this test with a `.https.html` suffix and expect false
here.

https://codereview.chromium.org/1016373002/diff/1/Source/core/loader/BeaconLo...
File Source/core/loader/BeaconLoader.cpp (right):

https://codereview.chromium.org/1016373002/diff/1/Source/core/loader/BeaconLo...
Source/core/loader/BeaconLoader.cpp:91: if
(MixedContentChecker::shouldBlockFetch(frame, request, request.url()))
Why do the check here? Won't it get picked up by FrameFetchContext::canRequest?

[sof, 2015-03-20 20:33:16]

https://codereview.chromium.org/1016373002/diff/1/LayoutTests/http/tests/navi...
File LayoutTests/http/tests/navigation/beacon-cross-origin-redirect.html
(right):

https://codereview.chromium.org/1016373002/diff/1/LayoutTests/http/tests/navi...
LayoutTests/http/tests/navigation/beacon-cross-origin-redirect.html:16:
shouldBeTrue('navigator.sendBeacon("http://127.0.0.1:8080/navigation/resources/redirection-response.php?status=302&target=non-existent.php",
"CrossOrigin");');
On 2015/03/20 16:51:27, Mike West wrote:
> Would you mind adding a mixed content test, since we apparently don't have
one?
> You could just copy/paste this test with a `.https.html` suffix and expect
false
> here.

Good idea, added.

https://codereview.chromium.org/1016373002/diff/1/Source/core/loader/BeaconLo...
File Source/core/loader/BeaconLoader.cpp (right):

https://codereview.chromium.org/1016373002/diff/1/Source/core/loader/BeaconLo...
Source/core/loader/BeaconLoader.cpp:91: if
(MixedContentChecker::shouldBlockFetch(frame, request, request.url()))
On 2015/03/20 16:51:27, Mike West wrote:
> Why do the check here? Won't it get picked up by
FrameFetchContext::canRequest?

This builds on just PingLoader, which is just a thinly veneered WebURLLoader. So
none of ResourceFetcher/Loader will be involved here.

The mixed content check mirrors the one in PingLoader::start(); I can't see a
tidy way of sharing that check.

[sof, 2015-03-20 21:50:05]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-03-20 21:50:05]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1016373002/#ps20001
(title: "rebased + add mixed content test")

[commit-bot: I haz the power, 2015-03-20 21:50:27]

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

[commit-bot: I haz the power, 2015-03-20 23:04:36]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192294