diff options
author | davidben@chromium.org <davidben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-14 20:06:34 +0000 |
---|---|---|
committer | davidben@chromium.org <davidben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-14 20:06:34 +0000 |
commit | b97fc381121ddc9996f9239d57fa4b24dda5893e (patch) | |
tree | 6854be4662b43f5a8677ab7b45f311eb9a363486 /content/browser/loader | |
parent | 95e6ff492da4de37500d411babfeea2479686ae3 (diff) | |
download | chromium_src-b97fc381121ddc9996f9239d57fa4b24dda5893e.zip chromium_src-b97fc381121ddc9996f9239d57fa4b24dda5893e.tar.gz chromium_src-b97fc381121ddc9996f9239d57fa4b24dda5893e.tar.bz2 |
Move first-party cookie URL logic to browser process.
Rather than have the renderer decide the value and inform the browser, the
browser decides and informs the render.
This removes one piece of navigation request redirect policy that needs to come
from the renderer. Redundant logic in Blink's DocumentLoader::willSendRequest to
be removed in a follow-up.
BUG=368813
Review URL: https://codereview.chromium.org/264613006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270463 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content/browser/loader')
4 files changed, 19 insertions, 17 deletions
diff --git a/content/browser/loader/async_resource_handler.cc b/content/browser/loader/async_resource_handler.cc index 7e3bd2e..886c49a 100644 --- a/content/browser/loader/async_resource_handler.cc +++ b/content/browser/loader/async_resource_handler.cc @@ -108,18 +108,12 @@ bool AsyncResourceHandler::OnMessageReceived(const IPC::Message& message, return handled; } -void AsyncResourceHandler::OnFollowRedirect( - int request_id, - bool has_new_first_party_for_cookies, - const GURL& new_first_party_for_cookies) { +void AsyncResourceHandler::OnFollowRedirect(int request_id) { if (!request()->status().is_success()) { DVLOG(1) << "OnFollowRedirect for invalid request"; return; } - if (has_new_first_party_for_cookies) - request()->set_first_party_for_cookies(new_first_party_for_cookies); - ResumeIfDeferred(); } @@ -164,8 +158,13 @@ bool AsyncResourceHandler::OnRequestRedirected(int request_id, reported_transfer_size_ = 0; response->head.request_start = request()->creation_time(); response->head.response_start = TimeTicks::Now(); + // TODO(davidben): Is it necessary to pass the new first party URL for + // cookies? The only case where it can change is top-level navigation requests + // and hopefully those will eventually all be owned by the browser. It's + // possible this is still needed while renderer-owned ones exist. return info->filter()->Send(new ResourceMsg_ReceivedRedirect( - request_id, new_url, response->head)); + request_id, new_url, request()->first_party_for_cookies(), + response->head)); } bool AsyncResourceHandler::OnResponseStarted(int request_id, diff --git a/content/browser/loader/async_resource_handler.h b/content/browser/loader/async_resource_handler.h index 7a51b77..3c0fce7 100644 --- a/content/browser/loader/async_resource_handler.h +++ b/content/browser/loader/async_resource_handler.h @@ -68,9 +68,7 @@ class AsyncResourceHandler : public ResourceHandler, private: // IPC message handlers: - void OnFollowRedirect(int request_id, - bool has_new_first_party_for_cookies, - const GURL& new_first_party_for_cookies); + void OnFollowRedirect(int request_id); void OnDataReceivedACK(int request_id); bool EnsureResourceBufferIsInitialized(); diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc index a7e8f12..3e88d33 100644 --- a/content/browser/loader/cross_site_resource_handler.cc +++ b/content/browser/loader/cross_site_resource_handler.cc @@ -121,6 +121,11 @@ bool CrossSiteResourceHandler::OnRequestRedirected( const GURL& new_url, ResourceResponse* response, bool* defer) { + // Top-level requests change their cookie first-party URL on redirects, while + // subframes retain the parent's value. + if (GetRequestInfo()->GetResourceType() == ResourceType::MAIN_FRAME) + request()->set_first_party_for_cookies(new_url); + // We should not have started the transition before being redirected. DCHECK(!in_cross_site_transition_); return next_handler_->OnRequestRedirected( diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc index 08e5f4d..815dda8 100644 --- a/content/browser/loader/resource_dispatcher_host_unittest.cc +++ b/content/browser/loader/resource_dispatcher_host_unittest.cc @@ -1067,7 +1067,7 @@ TEST_F(ResourceDispatcherHostTest, TestMany) { MakeTestRequest(0, 5, net::URLRequestTestJob::test_url_redirect_to_url_2()); // Finish the redirection - ResourceHostMsg_FollowRedirect redirect_msg(5, false, GURL()); + ResourceHostMsg_FollowRedirect redirect_msg(5); bool msg_was_ok; host_.OnMessageReceived(redirect_msg, filter_.get(), &msg_was_ok); base::MessageLoop::current()->RunUntilIdle(); @@ -2278,7 +2278,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationHtml) { SetResponse("HTTP/1.1 200 OK\n" "Content-Type: text/html\n\n", kResponseBody); - ResourceHostMsg_FollowRedirect redirect_msg(request_id, false, GURL()); + ResourceHostMsg_FollowRedirect redirect_msg(request_id); bool msg_was_ok; host_.OnMessageReceived(redirect_msg, filter_.get(), &msg_was_ok); base::MessageLoop::current()->RunUntilIdle(); @@ -2353,7 +2353,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationText) { SetResponse("HTTP/1.1 200 OK\n" "Content-Type: text/plain\n\n", kResponseBody); - ResourceHostMsg_FollowRedirect redirect_msg(request_id, false, GURL()); + ResourceHostMsg_FollowRedirect redirect_msg(request_id); bool msg_was_ok; host_.OnMessageReceived(redirect_msg, filter_.get(), &msg_was_ok); base::MessageLoop::current()->RunUntilIdle(); @@ -2436,7 +2436,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithProcessCrash) { SetResponse("HTTP/1.1 200 OK\n" "Content-Type: text/html\n\n", kResponseBody); - ResourceHostMsg_FollowRedirect redirect_msg(request_id, false, GURL()); + ResourceHostMsg_FollowRedirect redirect_msg(request_id); host_.OnMessageReceived(redirect_msg, first_filter.get(), &msg_was_ok); base::MessageLoop::current()->RunUntilIdle(); @@ -2510,7 +2510,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithTwoRedirects) { // Now that we're blocked on the redirect, simulate hitting another redirect. SetResponse("HTTP/1.1 302 Found\n" "Location: http://other.com/blerg\n\n"); - ResourceHostMsg_FollowRedirect redirect_msg(request_id, false, GURL()); + ResourceHostMsg_FollowRedirect redirect_msg(request_id); bool msg_was_ok; host_.OnMessageReceived(redirect_msg, filter_.get(), &msg_was_ok); base::MessageLoop::current()->RunUntilIdle(); @@ -2523,7 +2523,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithTwoRedirects) { SetResponse("HTTP/1.1 200 OK\n" "Content-Type: text/plain\n\n", kResponseBody); - ResourceHostMsg_FollowRedirect redirect_msg2(request_id, false, GURL()); + ResourceHostMsg_FollowRedirect redirect_msg2(request_id); host_.OnMessageReceived(redirect_msg2, filter_.get(), &msg_was_ok); base::MessageLoop::current()->RunUntilIdle(); |