Commit 544a2f36 authored by Yutaka Hirano's avatar Yutaka Hirano Committed by Commit Bot

Print a warning message when a close-to-matching preload is found

It is often hard to point the exact cause why a preload is not used
for a request |r|. This CL adds a warning message when there is a
preloaded request for |r|'s URL and type, but somehow not used, with
the reason.

Bug: 878225
Change-Id: Ib4ed4655cd9a964c3d60058f8a330b1c098c93b6
Reviewed-on: https://chromium-review.googlesource.com/1193163
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarYoav Weiss <yoav@yoav.ws>
Cr-Commit-Position: refs/heads/master@{#587499}
parent 49d3354b
CONSOLE WARNING: line 15: A preload for 'http://127.0.0.1:8000/preload/warning/resources/hello.txt' is found, but is not used because the new request loads the content as a blob.
This is a testharness.js-based test.
PASS Preload match fails because the request wants to load the content as a blob (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="fetch" crossorigin="anonymous" href="resources/hello.txt"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
promise_test((t) => {
const xhr = new XMLHttpRequest;
xhr.open('GET', 'resources/hello.txt');
xhr.responseType = 'blob';
const promise = new Promise(resolve => {
xhr.onloadend = resolve;
});
xhr.send();
return promise;
}, 'Preload match fails because the request wants to load the content as a blob (see the console log).');
</script>
</body>
</html>
CONSOLE WARNING: A preload for 'http://127.0.0.1:8000/preload/warning/resources/script.js' is found, but is not used due to an integrity mismatch.
This is a testharness.js-based test.
PASS Preload match fails due to an integrity mismatch (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="script" href="resources/script.js"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script src="resources/script.js" integrity="sha384-qfsBakWU2bdKoSdjyv97lIxUQH9+clPjBB/8ExlmEdvB6R/XCard/w4M5y82DA8D"></script>
<script>
promise_test(() => {
return new Promise(resolve => {
window.addEventListener('load', resolve);
});
}, "Preload match fails due to an integrity mismatch (see the console log).");
</script>
</body>
</html>
CONSOLE WARNING: line 9: A preload for 'http://127.0.0.1:8000/preload/warning/resources/hello.txt' is found, but is not used because the keepalive flag is set.
This is a testharness.js-based test.
PASS Preload match fails because keepalive is set (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="fetch" crossorigin="anonymous" href="resources/hello.txt"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
promise_test(async (t) => {
await fetch('resources/hello.txt', {keepalive: true});
}, 'Preload match fails because keepalive is set (see the console log).');
</script>
</body>
</html>
CONSOLE WARNING: line 9: A preload for 'http://127.0.0.1:8000/preload/warning/resources/hello.txt' is found, but is not used because the request credentials mode does not match. Consider taking a look at crossorigin attribute.
This is a testharness.js-based test.
PASS Preload match fails because the request credentials mode does not match (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="fetch" crossorigin="anonymous" href="resources/hello.txt"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
promise_test(async (t) => {
await fetch('resources/hello.txt', {credentials: 'include'});
}, 'Preload match fails because the request credentials mode does not match (see the console log).');
</script>
</body>
</html>
CONSOLE WARNING: line 9: A preload for 'http://127.0.0.1:8000/preload/warning/resources/hello.txt' is found, but is not used because the request headers do not match.
This is a testharness.js-based test.
PASS Preload match fails because the request headers do not match (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="fetch" crossorigin="anonymous" href="resources/hello.txt"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
promise_test(async (t) => {
await fetch('resources/hello.txt', {headers: {'foo': 'bar'}});
}, 'Preload match fails because the request headers do not match (see the console log).');
</script>
</body>
</html>
CONSOLE WARNING: line 9: A preload for 'http://127.0.0.1:8000/preload/warning/resources/hello.txt' is found, but is not used because the request HTTP method does not match.
This is a testharness.js-based test.
PASS Preload match fails because the request method does not match (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="fetch" crossorigin="anonymous" href="resources/hello.txt"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
promise_test(async (t) => {
await fetch('resources/hello.txt', {method: 'foobar'});
}, 'Preload match fails because the request method does not match (see the console log).');
</script>
</body>
</html>
CONSOLE WARNING: line 9: A preload for 'http://127.0.0.1:8000/preload/warning/resources/hello.txt' is found, but is not used because the request mode does not match. Consider taking a look at crossorigin attribute.
This is a testharness.js-based test.
PASS Preload match fails because the request mode does not match (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="fetch" crossorigin="anonymous" href="resources/hello.txt"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
promise_test(async (t) => {
await fetch('resources/hello.txt', {mode: 'same-origin'});
}, 'Preload match fails because the request mode does not match (see the console log).');
</script>
</body>
</html>
CONSOLE WARNING: line 10: 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 WARNING: line 11: A preload for 'http://127.0.0.1:8000/preload/warning/resources/hello.txt' is found, but is not used because the new request is synchronous.
This is a testharness.js-based test.
PASS Preload match fails because the request is synchronous (see the console log).
Harness: the test ran to completion.
<!doctype html>
<html>
<link rel="preload" as="fetch" crossorigin="anonymous" href="resources/hello.txt"></link>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
test(() => {
const xhr = new XMLHttpRequest;
xhr.open('GET', 'resources/hello.txt', false);
xhr.send();
}, "Preload match fails because the request is synchronous (see the console log).");
</script>
</body>
</html>
......@@ -193,6 +193,12 @@ void BaseFetchContext::AddInfoConsoleMessage(const String& message,
ConvertLogSourceToMessageSource(source), kInfoMessageLevel, message));
}
void BaseFetchContext::AddWarningConsoleMessage(const String& message,
LogSource source) const {
AddConsoleMessage(ConsoleMessage::Create(
ConvertLogSourceToMessageSource(source), kWarningMessageLevel, message));
}
void BaseFetchContext::AddErrorConsoleMessage(const String& message,
LogSource source) const {
AddConsoleMessage(ConsoleMessage::Create(
......
......@@ -61,6 +61,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
CreateWebSocketHandshakeThrottle() = 0;
void AddInfoConsoleMessage(const String&, LogSource) const override;
void AddWarningConsoleMessage(const String&, LogSource) const override;
void AddErrorConsoleMessage(const String&, LogSource) const override;
bool IsAdResource(const KURL&,
Resource::Type,
......
......@@ -184,15 +184,16 @@ ImageResource* ImageResource::Fetch(FetchParameters& params,
return resource;
}
bool ImageResource::CanReuse(
Resource::MatchStatus ImageResource::CanReuse(
const FetchParameters& params,
scoped_refptr<const SecurityOrigin> new_source_origin) const {
// If the image is a placeholder, but this fetch doesn't allow a
// placeholder, then do not reuse this resource.
if (params.GetImageRequestOptimization() !=
FetchParameters::kAllowPlaceholder &&
placeholder_option_ != PlaceholderOption::kDoNotReloadPlaceholder)
return false;
placeholder_option_ != PlaceholderOption::kDoNotReloadPlaceholder) {
return MatchStatus::kImagePlaceholder;
}
return Resource::CanReuse(params, std::move(new_source_origin));
}
......
......@@ -79,7 +79,7 @@ class CORE_EXPORT ImageResource final
void AllClientsAndObserversRemoved() override;
bool CanReuse(
MatchStatus CanReuse(
const FetchParameters&,
scoped_refptr<const SecurityOrigin> new_source_origin) const override;
bool CanUseCacheValidator() const override;
......
......@@ -112,6 +112,8 @@ void FetchContext::AddResourceTiming(const ResourceTimingInfo&) {}
void FetchContext::AddInfoConsoleMessage(const String&, LogSource) const {}
void FetchContext::AddWarningConsoleMessage(const String&, LogSource) const {}
void FetchContext::AddErrorConsoleMessage(const String&, LogSource) const {}
void FetchContext::PopulateResourceRequest(
......
......@@ -214,6 +214,7 @@ class PLATFORM_EXPORT FetchContext
}
virtual void AddInfoConsoleMessage(const String&, LogSource) const;
virtual void AddWarningConsoleMessage(const String&, LogSource) const;
virtual void AddErrorConsoleMessage(const String&, LogSource) const;
virtual const SecurityOrigin* GetSecurityOrigin() const { return nullptr; }
......
......@@ -359,7 +359,7 @@ static bool ShouldIgnoreHeaderForCacheReuse(AtomicString header_name) {
return headers.Contains(header_name);
}
bool RawResource::CanReuse(
Resource::MatchStatus RawResource::CanReuse(
const FetchParameters& new_fetch_parameters,
scoped_refptr<const SecurityOrigin> new_source_origin) const {
const ResourceRequest& new_request =
......@@ -375,15 +375,17 @@ bool RawResource::CanReuse(
for (const auto& header : new_headers) {
AtomicString header_name = header.key;
if (!ShouldIgnoreHeaderForCacheReuse(header_name) &&
header.value != old_headers.Get(header_name))
return false;
header.value != old_headers.Get(header_name)) {
return MatchStatus::kRequestHeadersDoNotMatch;
}
}
for (const auto& header : old_headers) {
AtomicString header_name = header.key;
if (!ShouldIgnoreHeaderForCacheReuse(header_name) &&
header.value != new_headers.Get(header_name))
return false;
header.value != new_headers.Get(header_name)) {
return MatchStatus::kRequestHeadersDoNotMatch;
}
}
return Resource::CanReuse(new_fetch_parameters, std::move(new_source_origin));
......
......@@ -80,7 +80,7 @@ class PLATFORM_EXPORT RawResource final : public Resource {
}
// Resource implementation
bool CanReuse(
MatchStatus CanReuse(
const FetchParameters&,
scoped_refptr<const SecurityOrigin> new_source_origin) const override;
bool WillFollowRedirect(const ResourceRequest&,
......
......@@ -74,9 +74,9 @@ TEST_F(RawResourceTest, DontIgnoreAcceptForCacheReuse) {
ResourceRequest png_request;
png_request.SetHTTPAccept("image/png");
EXPECT_FALSE(
jpeg_resource->CanReuse(FetchParameters(png_request), source_origin));
EXPECT_NE(
jpeg_resource->CanReuse(FetchParameters(png_request), source_origin),
Resource::MatchStatus::kOk);
}
class DummyClient final : public GarbageCollectedFinalized<DummyClient>,
......
......@@ -764,7 +764,7 @@ void Resource::FinishPendingClients() {
DCHECK(clients_awaiting_callback_.IsEmpty() || scheduled);
}
bool Resource::CanReuse(
Resource::MatchStatus Resource::CanReuse(
const FetchParameters& params,
scoped_refptr<const SecurityOrigin> new_source_origin) const {
const ResourceRequest& new_request = params.GetResourceRequest();
......@@ -778,7 +778,7 @@ bool Resource::CanReuse(
GetResponse().GetType() == network::mojom::FetchResponseType::kOpaque &&
new_request.GetFetchRequestMode() !=
network::mojom::FetchRequestMode::kNoCORS) {
return false;
return MatchStatus::kUnknownFailure;
}
// If credentials were sent with the previous request and won't be with this
......@@ -788,8 +788,9 @@ bool Resource::CanReuse(
// "Access-Control-Allow-Origin: *" all the time, but some of the client's
// requests are made without CORS and some with.
if (GetResourceRequest().AllowStoredCredentials() !=
new_request.AllowStoredCredentials())
return false;
new_request.AllowStoredCredentials()) {
return MatchStatus::kRequestCredentialsModeDoesNotMatch;
}
// Certain requests (e.g., XHRs) might have manually set headers that require
// revalidation. In theory, this should be a Revalidate case. In practice, the
......@@ -802,8 +803,9 @@ bool Resource::CanReuse(
// status code, but for a manual revalidation the response code remains 304.
// In this case, the Resource likely has insufficient context to provide a
// useful cache hit or revalidation. See http://crbug.com/643659
if (new_request.IsConditional() || response_.HttpStatusCode() == 304)
return false;
if (new_request.IsConditional() || response_.HttpStatusCode() == 304) {
return MatchStatus::kUnknownFailure;
}
// Answers the question "can a separate request with different options be
// re-used" (e.g. preload request). The safe (but possibly slow) answer is
......@@ -828,38 +830,39 @@ bool Resource::CanReuse(
// (crbug.com/618967) and bypassing redirect restriction around revalidation
// (crbug.com/613971 for 2. and crbug.com/614989 for 3.).
if (new_options.synchronous_policy == kRequestSynchronously ||
options_.synchronous_policy == kRequestSynchronously)
return false;
if (resource_request_.GetKeepalive() || new_request.GetKeepalive()) {
return false;
options_.synchronous_policy == kRequestSynchronously) {
return MatchStatus::kSynchronousFlagDoesNotMatch;
}
if (resource_request_.GetKeepalive() || new_request.GetKeepalive())
return MatchStatus::kKeepaliveSet;
if (GetResourceRequest().HttpMethod() != new_request.HttpMethod())
return false;
return MatchStatus::kRequestMethodDoesNotMatch;
if (GetResourceRequest().HttpBody() != new_request.HttpBody())
return false;
return MatchStatus::kUnknownFailure;
DCHECK(source_origin_);
DCHECK(new_source_origin);
// Don't reuse an existing resource when the source origin is different.
if (!source_origin_->IsSameSchemeHostPort(new_source_origin.get()))
return false;
return MatchStatus::kUnknownFailure;
// securityOrigin has more complicated checks which callers are responsible
// for.
if (new_request.GetFetchCredentialsMode() !=
resource_request_.GetFetchCredentialsMode())
return false;
resource_request_.GetFetchCredentialsMode()) {
return MatchStatus::kRequestCredentialsModeDoesNotMatch;
}
const auto new_mode = new_request.GetFetchRequestMode();
const auto existing_mode = resource_request_.GetFetchRequestMode();
if (new_mode != existing_mode)
return false;
return MatchStatus::kRequestModeDoesNotMatch;
switch (new_mode) {
case network::mojom::FetchRequestMode::kNoCORS:
......@@ -878,7 +881,7 @@ bool Resource::CanReuse(
// new one is handled in ResourceLoader, reusing the existing one will
// lead to CORS violations.
if (!options_.cors_handling_by_resource_fetcher)
return false;
return MatchStatus::kUnknownFailure;
// Otherwise (i.e., if the existing one is handled in ResourceLoader
// and the new one is handled in ThreadableLoader), reusing
......@@ -887,7 +890,7 @@ bool Resource::CanReuse(
break;
}
return true;
return MatchStatus::kOk;
}
void Resource::Prune() {
......
......@@ -88,6 +88,53 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
WTF_MAKE_NONCOPYABLE(Resource);
public:
// An enum representing whether a resource match with another resource.
// There are three kinds of status.
// - kOk, which represents the success.
// - kUnknownFailure, which represents miscellaneous failures. This includes
// failures which cannot happen for preload matching (for example,
// a failure due to non-cacheable request method cannot be happen for
// preload matching).
// - other specific error status
enum class MatchStatus {
// Match succeeds.
kOk,
// Match fails because of an unknown reason.
kUnknownFailure,
// Subresource integrity value doesn't match.
kIntegrityMismatch,
// Match fails because the new request wants to load the content
// as a blob.
kBlobRequest,
// Match fails because loading image is disabled.
kImageLoadingDisabled,
// Match fails due to different synchronous flags.
kSynchronousFlagDoesNotMatch,
// Match fails due to different request modes.
kRequestModeDoesNotMatch,
// Match fails due to different request credentials modes.
kRequestCredentialsModeDoesNotMatch,
// Match fails because keepalive flag is set on either requests.
kKeepaliveSet,
// Match fails due to different request methods.
kRequestMethodDoesNotMatch,
// Match fails due to different request headers.
kRequestHeadersDoNotMatch,
// Match fails due to different image placeholder policies.
kImagePlaceholder,
};
// |Type| enum values are used in UMAs, so do not change the values of
// existing |Type|. When adding a new |Type|, append it at the end and update
// |kLastResourceType|.
......@@ -325,8 +372,9 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
response_.SetDecodedBodyLength(value);
}
virtual bool CanReuse(
const FetchParameters&,
// Returns |kOk| when |this| can be resused for the given arguments.
virtual MatchStatus CanReuse(
const FetchParameters& params,
scoped_refptr<const SecurityOrigin> new_source_origin) const;
// If cache-aware loading is activated, this callback is called when the first
......
......@@ -66,6 +66,7 @@
#include "third_party/blink/renderer/platform/weborigin/security_violation_reporting_policy.h"
#include "third_party/blink/renderer/platform/wtf/assertions.h"
#include "third_party/blink/renderer/platform/wtf/text/cstring.h"
#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
#include "third_party/blink/renderer/platform/wtf/wtf.h"
......@@ -1097,8 +1098,11 @@ Resource* ResourceFetcher::MatchPreload(const FetchParameters& params,
Resource* resource = it->value;
if (resource->MustRefetchDueToIntegrityMetadata(params))
if (resource->MustRefetchDueToIntegrityMetadata(params)) {
if (!params.IsSpeculativePreload() && !params.IsLinkPreload())
PrintPreloadWarning(resource, Resource::MatchStatus::kIntegrityMismatch);
return nullptr;
}
if (params.IsSpeculativePreload())
return resource;
......@@ -1108,20 +1112,87 @@ Resource* ResourceFetcher::MatchPreload(const FetchParameters& params,
}
const ResourceRequest& request = params.GetResourceRequest();
if (request.DownloadToBlob())
if (request.DownloadToBlob()) {
PrintPreloadWarning(resource, Resource::MatchStatus::kBlobRequest);
return nullptr;
}
if (IsImageResourceDisallowedToBeReused(*resource) ||
!resource->CanReuse(params, GetSourceOrigin(params.Options())))
if (IsImageResourceDisallowedToBeReused(*resource)) {
PrintPreloadWarning(resource, Resource::MatchStatus::kImageLoadingDisabled);
return nullptr;
}
if (!resource->MatchPreload(params, Context().GetLoadingTaskRunner().get()))
const Resource::MatchStatus match_status =
resource->CanReuse(params, GetSourceOrigin(params.Options()));
if (match_status != Resource::MatchStatus::kOk) {
PrintPreloadWarning(resource, match_status);
return nullptr;
}
if (!resource->MatchPreload(params, Context().GetLoadingTaskRunner().get())) {
PrintPreloadWarning(resource, Resource::MatchStatus::kUnknownFailure);
return nullptr;
}
preloads_.erase(it);
matched_preloads_.push_back(resource);
return resource;
}
void ResourceFetcher::PrintPreloadWarning(Resource* resource,
Resource::MatchStatus status) {
if (!resource->IsLinkPreload())
return;
StringBuilder builder;
builder.Append("A preload for '");
builder.Append(resource->Url());
builder.Append("' is found, but is not used ");
switch (status) {
case Resource::MatchStatus::kOk:
NOTREACHED();
break;
case Resource::MatchStatus::kUnknownFailure:
builder.Append("due to an unknown reason.");
break;
case Resource::MatchStatus::kIntegrityMismatch:
builder.Append("due to an integrity mismatch.");
break;
case Resource::MatchStatus::kBlobRequest:
builder.Append("because the new request loads the content as a blob.");
break;
case Resource::MatchStatus::kImageLoadingDisabled:
builder.Append("because image loading is disabled.");
break;
case Resource::MatchStatus::kSynchronousFlagDoesNotMatch:
builder.Append("because the new request is synchronous.");
break;
case Resource::MatchStatus::kRequestModeDoesNotMatch:
builder.Append("because the request mode does not match. ");
builder.Append("Consider taking a look at crossorigin attribute.");
break;
case Resource::MatchStatus::kRequestCredentialsModeDoesNotMatch:
builder.Append("because the request credentials mode does not match. ");
builder.Append("Consider taking a look at crossorigin attribute.");
break;
case Resource::MatchStatus::kKeepaliveSet:
builder.Append("because the keepalive flag is set.");
break;
case Resource::MatchStatus::kRequestMethodDoesNotMatch:
builder.Append("because the request HTTP method does not match.");
break;
case Resource::MatchStatus::kRequestHeadersDoNotMatch:
builder.Append("because the request headers do not match.");
break;
case Resource::MatchStatus::kImagePlaceholder:
builder.Append("due to different image placeholder policies.");
break;
}
Context().AddWarningConsoleMessage(builder.ToString(),
FetchContext::kOtherSource);
}
void ResourceFetcher::InsertAsPreloadIfNecessary(Resource* resource,
const FetchParameters& params,
Resource::Type type) {
......@@ -1245,8 +1316,9 @@ ResourceFetcher::DetermineRevalidationPolicyInternal(
if (is_static_data)
return kUse;
if (!existing_resource.CanReuse(fetch_params,
GetSourceOrigin(fetch_params.Options()))) {
if (existing_resource.CanReuse(fetch_params,
GetSourceOrigin(fetch_params.Options())) !=
Resource::MatchStatus::kOk) {
RESOURCE_LOADING_DVLOG(1) << "ResourceFetcher::DetermineRevalidationPolicy "
"reloading due to Resource::CanReuse() "
"returning false.";
......
......@@ -228,6 +228,7 @@ class PLATFORM_EXPORT ResourceFetcher
ResourceClient*);
Resource* MatchPreload(const FetchParameters& params, Resource::Type);
void PrintPreloadWarning(Resource*, Resource::MatchStatus);
void InsertAsPreloadIfNecessary(Resource*,
const FetchParameters& params,
Resource::Type);
......
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