CORS for sync redirects
Historically, synchronous fetches have had no associated ResourceClient.
This made no difference, because synchronous fetches blocked until the
fetch completed, so there would be no callbacks to listen to until the
function was ready to return anyway.
However, now we perform the synchronous fetch on a utility thread, and
https://chromium.googlesource.com/chromium/src.git/+/4bd024a4e885ced5d79f19c4e950c7ffb9ee29b0
allows us to unblock and re-block the requesting thread in order to
perform redirect checks. Before this CL, the redirect handling can only
process checks that don't require access to the ResourceClient (like CSP).
This CL allows DocumentThreadableLoader to register itself as a
ResourceClient on a sync fetch, so that it can perform CORS checks
on sync requests.
Because DocumentThreadableLoader now gets ResourceClient callbacks
for sync requests, DocumentThreadableLoader::LoadRequestSync is
substantially simpler.
Bug: 706331
Change-Id: I3031b8068b3501f7f8f7bf19e4ab46ad7b59e83a
Reviewed-on: https://chromium-review.googlesource.com/1067745
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562496}diff --git a/content/renderer/loader/sync_load_context.cc b/content/renderer/loader/sync_load_context.cc
index 17da79d..1357923d 100644
--- a/content/renderer/loader/sync_load_context.cc
+++ b/content/renderer/loader/sync_load_context.cc
@@ -113,8 +113,7 @@
signals_(std::make_unique<SignalHelper>(this,
redirect_or_response_event,
abort_event,
- timeout)),
- fetch_request_mode_(request->fetch_request_mode) {
+ timeout)) {
url_loader_factory_ =
network::SharedURLLoaderFactory::Create(std::move(url_loader_factory));
@@ -134,23 +133,6 @@
const net::RedirectInfo& redirect_info,
const network::ResourceResponseInfo& info) {
DCHECK(!Completed());
- // Synchronous loads in blink aren't associated with a ResourceClient, and
- // CORS checks are performed by ResourceClient subclasses, so there's
- // currently no way to perform CORS checks for redirects.
- // Err on the side of extreme caution and block any cross origin redirect
- // that might have CORS implications.
- if (fetch_request_mode_ != network::mojom::FetchRequestMode::kNoCORS &&
- redirect_info.new_url.GetOrigin() != response_->url.GetOrigin()) {
- LOG(ERROR) << "Cross origin redirect denied";
- response_->error_code = net::ERR_ABORTED;
-
- CompleteRequest(false /* remove_pending_request */);
-
- // Returning false here will cause the request to be cancelled and this
- // object deleted.
- return false;
- }
-
response_->url = redirect_info.new_url;
response_->info = info;
response_->redirect_info = redirect_info;
diff --git a/content/renderer/loader/sync_load_context.h b/content/renderer/loader/sync_load_context.h
index 23a7e7e..0d5cbde 100644
--- a/content/renderer/loader/sync_load_context.h
+++ b/content/renderer/loader/sync_load_context.h
@@ -113,8 +113,6 @@
class SignalHelper;
std::unique_ptr<SignalHelper> signals_;
- const network::mojom::FetchRequestMode fetch_request_mode_;
-
DISALLOW_COPY_AND_ASSIGN(SyncLoadContext);
};
diff --git a/third_party/WebKit/LayoutTests/external/wpt/xhr/access-control-and-redirects-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/xhr/access-control-and-redirects-expected.txt
deleted file mode 100644
index 8e655909..0000000
--- a/third_party/WebKit/LayoutTests/external/wpt/xhr/access-control-and-redirects-expected.txt
+++ /dev/null
@@ -1,9 +0,0 @@
-This is a testharness.js-based test.
-FAIL Local sync redirect to remote origin Failed to execute 'send' on 'XMLHttpRequest': Failed to load 'http://web-platform.test:8001/xhr/resources/redirect-cors.py?location=http://www1.web-platform.test:8001/xhr/resources/access-control-basic-allow.py'.
-PASS Local async redirect to remote origin
-FAIL Remote sync redirect to local origin Failed to execute 'send' on 'XMLHttpRequest': Failed to load 'http://www1.web-platform.test:8001/xhr/resources/redirect-cors.py?location=http://web-platform.test:8001/xhr/resources/access-control-basic-allow.py&allow_origin=true'.
-PASS Remote async redirect to local origin
-FAIL Remote sync redirect to same remote origin Failed to execute 'send' on 'XMLHttpRequest': Failed to load 'http://www1.web-platform.test:8001/xhr/resources/redirect-cors.py?location=http://www1.web-platform.test:8001/xhr/resources/access-control-basic-allow.py&allow_origin=true'.
-PASS Remote async redirect to same remote origin
-Harness: the test ran to completion.
-
diff --git a/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event-expected.txt b/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event-expected.txt
index 1736b068..4712755 100644
--- a/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event-expected.txt
+++ b/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event-expected.txt
@@ -1,6 +1,3 @@
-CONSOLE WARNING: line 27: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.
-CONSOLE ERROR: line 20: Uncaught RangeError: Maximum call stack size exceeded.
-CONSOLE ERROR: line 21: Uncaught RangeError: Maximum call stack size exceeded.
This tests that having infinite recursion in XMLHttpRequest event handler does not crash.
PASS
diff --git a/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html b/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
index ec88cf2..a4b917e 100644
--- a/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
+++ b/third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
@@ -11,6 +11,10 @@
{
if (window.testRunner) {
testRunner.dumpAsText();
+ // This test outputs a RangeError to console due to hitting the maximum
+ // call stack size. The exact error message and number of error messages
+ // depend on the testing environment, so suppress console messages.
+ testRunner.setDumpConsoleMessages(false);
}
var xhr = new XMLHttpRequest();
xhr. {
diff --git a/third_party/WebKit/LayoutTests/http/tests/security/xss-DENIED-xsl-external-entity-redirect-expected.txt b/third_party/WebKit/LayoutTests/http/tests/security/xss-DENIED-xsl-external-entity-redirect-expected.txt
index 8cd59a53..3831903f 100644
--- a/third_party/WebKit/LayoutTests/http/tests/security/xss-DENIED-xsl-external-entity-redirect-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/security/xss-DENIED-xsl-external-entity-redirect-expected.txt
@@ -1,2 +1,6 @@
+CONSOLE ERROR: Unsafe attempt to load URL http://localhost:8000/security/resources/target.xml from frame with URL http://127.0.0.1:8000/security/xss-DENIED-xsl-external-entity-redirect.xml. Domains, protocols and ports must match.
+
+CONSOLE ERROR: Unsafe attempt to load URL http://localhost:8000/security/resources/target.xml from frame with URL http://127.0.0.1:8000/security/xss-DENIED-xsl-external-entity-redirect.xml. Domains, protocols and ports must match.
+
This test includes a cross-origin external entity. It passes if the load fails and thus there is no text below this line.
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-post-sync-expected.txt b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-post-sync-expected.txt
index 4412c7cd..f380eaad 100644
--- a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-post-sync-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-post-sync-expected.txt
@@ -1,4 +1,5 @@
CONSOLE WARNING: line 30: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.
+CONSOLE ERROR: line 31: Failed to load http://localhost:8000/xmlhttprequest/resources/reply.xml: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
Test that a cross-origin redirect to a server that responds is indistinguishable from one that does not. Should say PASS:
PASS
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-double-expected.txt b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-double-expected.txt
index e2cde19..adb03c3 100644
--- a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-double-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-double-expected.txt
@@ -1,4 +1,5 @@
CONSOLE WARNING: line 25: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.
+CONSOLE ERROR: line 26: Failed to load http://localhost:8000/xmlhttprequest/resources/redirect.php?url=/xmlhttprequest/resources/reply.xml: Redirect from 'http://localhost:8000/xmlhttprequest/resources/redirect.php?url=/xmlhttprequest/resources/reply.xml' to 'http://localhost:8000/xmlhttprequest/resources/reply.xml' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
Test that a cross-origin chain of redirects to a server that responds is indistinguishable from one that does not. Should say PASS:
PASS
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-expected.txt b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-expected.txt
index 84783c5..28fa719 100644
--- a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-sync-expected.txt
@@ -1,4 +1,5 @@
CONSOLE WARNING: line 25: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.
+CONSOLE ERROR: line 26: Failed to load http://localhost:8000/xmlhttprequest/resources/reply.xml: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
Test that a cross-origin redirect to a server that responds is indistinguishable from one that does not. Should say PASS:
PASS
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-tripmine-expected.txt b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-tripmine-expected.txt
index 4cb39d41..ec2f7af 100644
--- a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-tripmine-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-tripmine-expected.txt
@@ -9,6 +9,16 @@
CONSOLE ERROR: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
CONSOLE ERROR: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
CONSOLE ERROR: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
+CONSOLE ERROR: line 1: Failed to load http://localhost:8000/resources/tripmine.php: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
Test that a cross-origin redirect does not result in a non-simple request being sent to the target.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect-expected.txt b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect-expected.txt
index d4e1fcbb..73ddf4b 100644
--- a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect-expected.txt
@@ -1,4 +1,5 @@
CONSOLE WARNING: line 52: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.
+CONSOLE ERROR: line 54: Failed to load http://localhost:8080/xmlhttprequest/resources/forbidden.txt: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
CONSOLE ERROR: Failed to load http://localhost:8080/xmlhttprequest/resources/forbidden.txt: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://127.0.0.1:8000' is therefore not allowed access.
This tests that unsafe redirects won't be allowed when making an XMLHttpRequest.
Sync XHR started.
diff --git a/third_party/blink/renderer/core/loader/document_threadable_loader.cc b/third_party/blink/renderer/core/loader/document_threadable_loader.cc
index 75969dd..511e86bc 100644
--- a/third_party/blink/renderer/core/loader/document_threadable_loader.cc
+++ b/third_party/blink/renderer/core/loader/document_threadable_loader.cc
@@ -599,7 +599,6 @@
const ResourceResponse& redirect_response) {
DCHECK(client_);
DCHECK_EQ(resource, GetResource());
- DCHECK(async_);
checker_.RedirectReceived();
@@ -781,7 +780,6 @@
DCHECK(client_);
DCHECK_EQ(resource, GetResource());
DCHECK(actual_request_.IsNull());
- DCHECK(async_);
checker_.DataDownloaded();
client_->DidDownloadData(data_length);
@@ -792,7 +790,6 @@
const ResourceTimingInfo& info) {
DCHECK(client_);
DCHECK_EQ(resource, GetResource());
- DCHECK(async_);
client_->DidReceiveResourceTiming(info);
}
@@ -802,7 +799,6 @@
scoped_refptr<BlobDataHandle> blob) {
DCHECK(client_);
DCHECK_EQ(resource, GetResource());
- DCHECK(async_);
checker_.DidDownloadToBlob();
client_->DidDownloadToBlob(std::move(blob));
@@ -813,7 +809,6 @@
const ResourceResponse& response,
std::unique_ptr<WebDataConsumerHandle> handle) {
DCHECK_EQ(resource, GetResource());
- DCHECK(async_);
checker_.ResponseReceived();
@@ -992,7 +987,6 @@
const char* data,
size_t data_length) {
DCHECK_EQ(resource, GetResource());
- DCHECK(async_);
checker_.DataReceived();
@@ -1020,11 +1014,16 @@
void DocumentThreadableLoader::NotifyFinished(Resource* resource) {
DCHECK(client_);
DCHECK_EQ(resource, GetResource());
- DCHECK(async_);
checker_.NotifyFinished(resource);
- if (resource->ErrorOccurred()) {
+ // Don't throw an exception for failed sync local file loads.
+ // TODO(japhet): This logic has been moved around but unchanged since 2007.
+ // Tested by fast/xmlhttprequest/xmlhttprequest-missing-file-exception.html
+ // Do we still need this?
+ bool is_sync_to_local_file = resource->Url().IsLocalFile() && !async_;
+
+ if (resource->ErrorOccurred() && !is_sync_to_local_file) {
DispatchDidFail(resource->GetResourceError());
} else {
HandleSuccessfulFinish(resource->Identifier());
@@ -1176,70 +1175,10 @@
fetch_params.MutableResourceRequest().SetTimeoutInterval(
options_.timeout_milliseconds / 1000.0);
}
- RawResource* resource = RawResource::FetchSynchronously(
- fetch_params, loading_context_->GetResourceFetcher());
- ResourceResponse response = resource->GetResponse();
- unsigned long identifier = resource->Identifier();
- ThreadableLoaderClient* client = client_;
- const KURL& request_url = request.Url();
- // No exception for file:/// resources, see <rdar://problem/4962298>. Also, if
- // we have an HTTP response, then it wasn't a network error in fact.
- if (resource->LoadFailedOrCanceled() && !request_url.IsLocalFile() &&
- response.HttpStatusCode() <= 0) {
- DispatchDidFail(resource->GetResourceError());
- return;
- }
-
- // FIXME: A synchronous request does not tell us whether a redirect happened
- // or not, so we guess by comparing the request and response URLs. This isn't
- // a perfect test though, since a server can serve a redirect to the same URL
- // that was requested. Also comparing the request and response URLs as strings
- // will fail if the requestURL still has its credentials.
- if (request_url != response.Url() &&
- !IsAllowedRedirect(request.GetFetchRequestMode(), response.Url())) {
- client_ = nullptr;
- client->DidFailRedirectCheck();
- return;
- }
-
- HandleResponse(identifier, request.GetFetchRequestMode(),
- request.GetFetchCredentialsMode(), response, nullptr);
-
- // HandleResponse() may detect an error. In such a case (check |m_client| as
- // it gets reset by clear() call), skip the rest.
- //
- // |this| is alive here since loadResourceSynchronously() keeps it alive until
- // the end of the function.
- if (!client_)
- return;
-
- if (scoped_refptr<const SharedBuffer> data = resource->ResourceBuffer()) {
- data->ForEachSegment([this](const char* segment, size_t segment_size,
- size_t segment_offset) -> bool {
- HandleReceivedData(segment, segment_size);
- // The client may cancel this loader in handleReceivedData().
- return client_;
- });
- }
-
- // The client may cancel this loader in handleReceivedData(). In such a case,
- // skip the rest.
- if (!client_)
- return;
-
- base::Optional<int64_t> downloaded_file_length =
- resource->DownloadedFileLength();
- if (downloaded_file_length) {
- client_->DidDownloadData(*downloaded_file_length);
- }
- if (request.DownloadToBlob()) {
- if (resource->DownloadedBlob())
- client_->DidDownloadData(resource->DownloadedBlob()->size());
- client_->DidDownloadToBlob(resource->DownloadedBlob());
- }
-
- HandleSuccessfulFinish(identifier);
+ checker_.WillAddClient();
+ RawResource::FetchSynchronously(fetch_params,
+ loading_context_->GetResourceFetcher(), this);
}
void DocumentThreadableLoader::LoadRequest(
diff --git a/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc b/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
index 309a75c..064c73f 100644
--- a/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
+++ b/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
@@ -41,10 +41,11 @@
namespace blink {
RawResource* RawResource::FetchSynchronously(FetchParameters& params,
- ResourceFetcher* fetcher) {
+ ResourceFetcher* fetcher,
+ RawResourceClient* client) {
params.MakeSynchronous();
return ToRawResource(fetcher->RequestResource(
- params, RawResourceFactory(Resource::kRaw), nullptr));
+ params, RawResourceFactory(Resource::kRaw), client));
}
RawResource* RawResource::FetchImport(FetchParameters& params,
diff --git a/third_party/blink/renderer/platform/loader/fetch/raw_resource.h b/third_party/blink/renderer/platform/loader/fetch/raw_resource.h
index 25777998..b2bc32b 100644
--- a/third_party/blink/renderer/platform/loader/fetch/raw_resource.h
+++ b/third_party/blink/renderer/platform/loader/fetch/raw_resource.h
@@ -43,7 +43,9 @@
class PLATFORM_EXPORT RawResource final : public Resource {
public:
- static RawResource* FetchSynchronously(FetchParameters&, ResourceFetcher*);
+ static RawResource* FetchSynchronously(FetchParameters&,
+ ResourceFetcher*,
+ RawResourceClient* = nullptr);
static RawResource* Fetch(FetchParameters&,
ResourceFetcher*,
RawResourceClient*);
diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
index 8bd77ea..ae7ae0a 100644
--- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
+++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
@@ -558,15 +558,25 @@
Resource* ResourceFetcher::ResourceForBlockedRequest(
const FetchParameters& params,
const ResourceFactory& factory,
- ResourceRequestBlockedReason blocked_reason) {
+ ResourceRequestBlockedReason blocked_reason,
+ ResourceClient* client) {
Resource* resource = factory.Create(
params.GetResourceRequest(), params.Options(), params.DecoderOptions());
+ // Sync requests need to be registered as a client before the fetch begins, so
+ // that they can process any redirects as they are received. Async requests,
+ // on the other hand, need to be registered after the fetch begins, because
+ // if the fetch fails during start, registering the client too early may lead
+ // to unexpected synchronous notifications.
+ if (client && params.Options().synchronous_policy == kRequestSynchronously)
+ client->SetResource(resource, Context().GetLoadingTaskRunner().get());
resource->SetStatus(ResourceStatus::kPending);
resource->NotifyStartLoad();
resource->SetSourceOrigin(GetSourceOrigin(params.Options()));
resource->FinishAsError(ResourceError::CancelledDueToAccessCheckError(
params.Url(), blocked_reason),
Context().GetLoadingTaskRunner().get());
+ if (client && params.Options().synchronous_policy == kRequestAsynchronously)
+ client->SetResource(resource, Context().GetLoadingTaskRunner().get());
return resource;
}
@@ -749,22 +759,6 @@
const ResourceFactory& factory,
ResourceClient* client,
const SubstituteData& substitute_data) {
- // Only async requests get ResourceClient callbacks, so sync requests
- // shouldn't provide a client.
- DCHECK(!client ||
- params.Options().synchronous_policy == kRequestAsynchronously);
- Resource* resource =
- RequestResourceInternal(params, factory, substitute_data);
- DCHECK(resource);
- if (client)
- client->SetResource(resource, Context().GetLoadingTaskRunner().get());
- return resource;
-}
-
-Resource* ResourceFetcher::RequestResourceInternal(
- FetchParameters& params,
- const ResourceFactory& factory,
- const SubstituteData& substitute_data) {
unsigned long identifier = CreateUniqueIdentifier();
ResourceRequest& resource_request = params.MutableResourceRequest();
network_instrumentation::ScopedResourceLoadTracker
@@ -789,8 +783,10 @@
base::Optional<ResourceRequestBlockedReason> blocked_reason =
PrepareRequest(params, factory, substitute_data, identifier);
- if (blocked_reason)
- return ResourceForBlockedRequest(params, factory, blocked_reason.value());
+ if (blocked_reason) {
+ return ResourceForBlockedRequest(params, factory, blocked_reason.value(),
+ client);
+ }
Resource::Type resource_type = factory.GetType();
@@ -815,8 +811,8 @@
// in the case of data URLs which might have resources such as fonts that
// need to be decoded only on demand. These data URLs are allowed to be
// processed using the normal ResourceFetcher machinery.
- return ResourceForBlockedRequest(params, factory,
- ResourceRequestBlockedReason::kOther);
+ return ResourceForBlockedRequest(
+ params, factory, ResourceRequestBlockedReason::kOther, client);
}
}
@@ -861,6 +857,14 @@
if (policy != kUse)
resource->SetIdentifier(identifier);
+ // Sync requests need to be registered as a client before the fetch begins, so
+ // that they can process any redirects as they are received. Async requests,
+ // on the other hand, need to be registered after the fetch begins, because
+ // if the fetch fails during start, registering the client too early may lead
+ // to unexpected synchronous notifications.
+ if (client && params.Options().synchronous_policy == kRequestSynchronously)
+ client->SetResource(resource, Context().GetLoadingTaskRunner().get());
+
// TODO(yoav): It is not clear why preloads are exempt from this check. Can we
// remove the exemption?
if (!params.IsSpeculativePreload() || policy != kUse) {
@@ -899,6 +903,9 @@
if (policy != kUse)
InsertAsPreloadIfNecessary(resource, params, resource_type);
+ if (client && params.Options().synchronous_policy == kRequestAsynchronously)
+ client->SetResource(resource, Context().GetLoadingTaskRunner().get());
+
return resource;
}
diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
index 872bf01..83148ec 100644
--- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
+++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
@@ -207,9 +207,6 @@
FetchParameters::SpeculativePreloadType::kNotSpeculative,
bool is_link_preload = false);
- Resource* RequestResourceInternal(FetchParameters&,
- const ResourceFactory&,
- const SubstituteData&);
base::Optional<ResourceRequestBlockedReason> PrepareRequest(
FetchParameters&,
const ResourceFactory&,
@@ -221,7 +218,8 @@
const SubstituteData&);
Resource* ResourceForBlockedRequest(const FetchParameters&,
const ResourceFactory&,
- ResourceRequestBlockedReason);
+ ResourceRequestBlockedReason,
+ ResourceClient*);
Resource* MatchPreload(const FetchParameters& params, Resource::Type);
void InsertAsPreloadIfNecessary(Resource*,
diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc b/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
index ce117eff..c608209 100644
--- a/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
+++ b/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
@@ -777,11 +777,9 @@
if (data_out.size()) {
data_out.ForEachSegment([this](const char* segment, size_t segment_size,
size_t segment_offset) {
- Context().DispatchDidReceiveData(resource_->Identifier(), segment,
- segment_size);
+ DidReceiveData(segment, segment_size);
return true;
});
- resource_->SetResourceBuffer(data_out);
}
if (downloaded_file_length) {
@@ -791,12 +789,9 @@
if (request.DownloadToBlob()) {
auto blob = downloaded_blob.GetBlobHandle();
if (blob) {
- Context().DispatchDidReceiveData(resource_->Identifier(), nullptr,
- blob->size());
- resource_->DidDownloadData(blob->size());
+ DidDownloadData(blob->size(), blob->size());
}
- Context().DispatchDidDownloadToBlob(resource_->Identifier(), blob.get());
- resource_->DidDownloadToBlob(blob);
+ FinishedCreatingBlob(blob);
}
DidFinishLoading(CurrentTimeTicks(), encoded_data_length, encoded_body_length,
decoded_body_length, false);