Commit 7a7c46db authored by Nate Chapin's avatar Nate Chapin Committed by Commit Bot

Merge ThreadableLoaderClient's redirect callbacks

Two behavior changes:
* Previously, DidReceiveRedirectTo() was called for almost all
  redirects (barring a few early-exits), but WillFollowRedirect()
  was only called for same-origin redirects. Now the single callback,
  WillFollowRedirect(), is called whenever DidReceiveRedirectTo()
  used to be called.
* Previously, due to a bug in
  https://chromium-review.googlesource.com/c/chromium/src/+/1147890,
  the return value of WillFollowRedirect() was ignored when
  out-of-blink CORS was enabled. That return value is now honored.

Change-Id: I9c1ebb30b23aea77114711e0ee1b5dd0409c7166
Reviewed-on: https://chromium-review.googlesource.com/1150695
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579253}
parent 3860ee78
...@@ -520,7 +520,7 @@ TEST_F(WebAssociatedURLLoaderTest, ...@@ -520,7 +520,7 @@ TEST_F(WebAssociatedURLLoaderTest,
ServeRequests(); ServeRequests();
// We should get a notification about access control check failure. // We should get a notification about access control check failure.
EXPECT_FALSE(will_follow_redirect_); EXPECT_TRUE(will_follow_redirect_);
EXPECT_FALSE(did_receive_response_); EXPECT_FALSE(did_receive_response_);
EXPECT_FALSE(did_receive_data_); EXPECT_FALSE(did_receive_data_);
EXPECT_TRUE(did_fail_); EXPECT_TRUE(did_fail_);
...@@ -568,8 +568,7 @@ TEST_F(WebAssociatedURLLoaderTest, ...@@ -568,8 +568,7 @@ TEST_F(WebAssociatedURLLoaderTest,
EXPECT_TRUE(expected_loader_); EXPECT_TRUE(expected_loader_);
expected_loader_->LoadAsynchronously(request, this); expected_loader_->LoadAsynchronously(request, this);
ServeRequests(); ServeRequests();
// We should not receive a notification for the redirect. EXPECT_TRUE(will_follow_redirect_);
EXPECT_FALSE(will_follow_redirect_);
EXPECT_TRUE(did_receive_response_); EXPECT_TRUE(did_receive_response_);
EXPECT_TRUE(did_receive_data_); EXPECT_TRUE(did_receive_data_);
EXPECT_TRUE(did_finish_loading_); EXPECT_TRUE(did_finish_loading_);
......
...@@ -179,7 +179,7 @@ class FetchManager::Loader final ...@@ -179,7 +179,7 @@ class FetchManager::Loader final
~Loader() override; ~Loader() override;
virtual void Trace(blink::Visitor*); virtual void Trace(blink::Visitor*);
void DidReceiveRedirectTo(const KURL&) override; bool WillFollowRedirect(const KURL&, const ResourceResponse&) override;
void DidReceiveResponse(unsigned long, void DidReceiveResponse(unsigned long,
const ResourceResponse&, const ResourceResponse&,
std::unique_ptr<WebDataConsumerHandle>) override; std::unique_ptr<WebDataConsumerHandle>) override;
...@@ -376,8 +376,10 @@ void FetchManager::Loader::Trace(blink::Visitor* visitor) { ...@@ -376,8 +376,10 @@ void FetchManager::Loader::Trace(blink::Visitor* visitor) {
visitor->Trace(execution_context_); visitor->Trace(execution_context_);
} }
void FetchManager::Loader::DidReceiveRedirectTo(const KURL& url) { bool FetchManager::Loader::WillFollowRedirect(const KURL& url,
const ResourceResponse&) {
url_list_.push_back(url); url_list_.push_back(url);
return true;
} }
void FetchManager::Loader::DidReceiveResponse( void FetchManager::Loader::DidReceiveResponse(
......
...@@ -671,16 +671,10 @@ bool ThreadableLoader::RedirectReceived( ...@@ -671,16 +671,10 @@ bool ThreadableLoader::RedirectReceived(
return false; return false;
} }
if (out_of_blink_cors_) {
client_->DidReceiveRedirectTo(new_url);
client_->WillFollowRedirect(new_url, redirect_response);
return true;
}
// Allow same origin requests to continue after allowing clients to audit // Allow same origin requests to continue after allowing clients to audit
// the redirect. // the redirect.
if (IsAllowedRedirect(new_request.GetFetchRequestMode(), new_url)) { if (out_of_blink_cors_ ||
client_->DidReceiveRedirectTo(new_url); IsAllowedRedirect(new_request.GetFetchRequestMode(), new_url)) {
return client_->WillFollowRedirect(new_url, redirect_response); return client_->WillFollowRedirect(new_url, redirect_response);
} }
...@@ -721,7 +715,7 @@ bool ThreadableLoader::RedirectReceived( ...@@ -721,7 +715,7 @@ bool ThreadableLoader::RedirectReceived(
} }
} }
client_->DidReceiveRedirectTo(new_url); client_->WillFollowRedirect(new_url, redirect_response);
// FIXME: consider combining this with CORS redirect handling performed by // FIXME: consider combining this with CORS redirect handling performed by
// CrossOriginAccessControl::handleRedirect(). // CrossOriginAccessControl::handleRedirect().
......
...@@ -50,8 +50,6 @@ class CORE_EXPORT ThreadableLoaderClient { ...@@ -50,8 +50,6 @@ class CORE_EXPORT ThreadableLoaderClient {
public: public:
virtual void DidSendData(unsigned long long /*bytesSent*/, virtual void DidSendData(unsigned long long /*bytesSent*/,
unsigned long long /*totalBytesToBeSent*/) {} unsigned long long /*totalBytesToBeSent*/) {}
// TODO(japhet?): Merge these redirect callbacks.
virtual void DidReceiveRedirectTo(const KURL&) {}
virtual bool WillFollowRedirect(const KURL& new_url, virtual bool WillFollowRedirect(const KURL& new_url,
const ResourceResponse&) { const ResourceResponse&) {
return true; return true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment