Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(363)

Unified Diff: third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

Issue 2871233002: Separate preload matching from MemoryCache (Closed)
Patch Set: fix Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
index 4ca1989f0786653063dd4dd5958df3a8e9fb5196..4cf233dd5305d4f379a6202ed52fb59ddbff9cca 100644
--- a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
+++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
@@ -493,11 +493,26 @@ void ResourceFetcher::UpdateMemoryCacheStats(Resource* resource,
// dead if MemoryCache holds weak references to Resource). Currently we check
// references to Resource from ResourceClient and |m_preloads| only, because
// they are major sources of references.
- if (resource && !resource->IsAlive() && !preloads_.Contains(resource)) {
+ if (resource && !resource->IsAlive() && !ContainsAsPreload(resource)) {
DEFINE_RESOURCE_HISTOGRAM("Dead.");
}
}
+bool ResourceFetcher::ContainsAsPreload(Resource* resource) const {
+ auto it = preloads_.find(PreloadKey(resource->Url(), resource->GetType()));
+ return it != preloads_.end() && it->value == resource;
+}
+
+void ResourceFetcher::RemovePreload(Resource* resource) {
+ auto it = preloads_.find(PreloadKey(resource->Url(), resource->GetType()));
+ if (it == preloads_.end())
+ return;
+ if (it->value == resource) {
+ resource->DecreasePreloadCount();
+ preloads_.erase(it);
+ }
+}
+
ResourceFetcher::PrepareRequestResult ResourceFetcher::PrepareRequest(
FetchParameters& params,
const ResourceFactory& factory,
@@ -604,19 +619,30 @@ Resource* ResourceFetcher::RequestResource(
if (!resource && !is_data_url && archive_)
return nullptr;
}
- if (!resource && IsMainThread()) {
- resource =
- GetMemoryCache()->ResourceForURL(params.Url(), GetCacheIdentifier());
+ RevalidationPolicy policy = kLoad;
+ bool preload_found = false;
+ if (!resource) {
+ resource = MatchPreload(params, factory.GetType());
+ if (resource) {
+ preload_found = true;
+ policy = kUse;
+ // If |param| is for a blocking resource and a preloaded resource is
+ // found, we may need to make it block the onload event.
+ MakePreloadedResourceBlockOnloadIfNeeded(resource, params);
+ }
}
+ if (!preload_found) {
+ if (!resource && IsMainThread()) {
+ resource =
+ GetMemoryCache()->ResourceForURL(params.Url(), GetCacheIdentifier());
+ }
- // If we got a preloaded resource from the cache for a non-preload request,
- // we may need to make it block the onload event.
- MakePreloadedResourceBlockOnloadIfNeeded(resource, params);
-
- const RevalidationPolicy policy = DetermineRevalidationPolicy(
- factory.GetType(), params, resource, is_static_data);
- TRACE_EVENT_INSTANT1("blink", "ResourceFetcher::determineRevalidationPolicy",
- TRACE_EVENT_SCOPE_THREAD, "revalidationPolicy", policy);
+ policy = DetermineRevalidationPolicy(factory.GetType(), params, resource,
+ is_static_data);
+ TRACE_EVENT_INSTANT1(
+ "blink", "ResourceFetcher::determineRevalidationPolicy",
+ TRACE_EVENT_SCOPE_THREAD, "revalidationPolicy", policy);
+ }
UpdateMemoryCacheStats(resource, policy, params, factory, is_static_data);
@@ -669,11 +695,17 @@ Resource* ResourceFetcher::RequestResource(
// loading immediately. If revalidation policy was determined as |Revalidate|,
// the resource was already initialized for the revalidation here, but won't
// start loading.
- if (!ResourceNeedsLoad(resource, params, policy))
+ if (!ResourceNeedsLoad(resource, params, policy)) {
+ if (policy != kUse)
+ InsertAsPreloadIfNecessary(resource, params, factory.GetType());
return resource;
+ }
if (!StartLoad(resource))
return nullptr;
+
+ if (policy != kUse)
+ InsertAsPreloadIfNecessary(resource, params, factory.GetType());
scoped_resource_load_tracker.ResourceLoadContinuesBeyondScope();
DCHECK(!resource->ErrorOccurred() ||
@@ -828,61 +860,57 @@ void ResourceFetcher::RecordResourceTimingOnRedirect(
}
}
-ResourceFetcher::RevalidationPolicy
-ResourceFetcher::DetermineRevalidationPolicy(
- Resource::Type type,
- const FetchParameters& fetch_params,
- Resource* existing_resource,
- bool is_static_data) const {
- const ResourceRequest& request = fetch_params.GetResourceRequest();
-
- if (!existing_resource)
- return kLoad;
+Resource* ResourceFetcher::MatchPreload(const FetchParameters& params,
+ Resource::Type type) {
+ auto it = preloads_.find(PreloadKey(params.Url(), type));
+ if (it == preloads_.end())
+ return nullptr;
- // If the existing resource is loading and the associated fetcher is not equal
- // to |this|, we must not use the resource. Otherwise, CSP violation may
- // happen in redirect handling.
- if (existing_resource->Loader() &&
- existing_resource->Loader()->Fetcher() != this) {
- return kReload;
- }
+ Resource* resource = it->value;
- // Checks if the resource has an explicit policy about integrity metadata.
- //
- // This is necessary because ScriptResource and CSSStyleSheetResource objects
- // do not keep the raw data around after the source is accessed once, so if
- // the resource is accessed from the MemoryCache for a second time, there is
- // no way to redo an integrity check.
- //
- // Thus, Blink implements a scheme where it caches the integrity information
- // for those resources after the first time it is checked, and if there is
- // another request for that resource, with the same integrity metadata, Blink
- // skips the integrity calculation. However, if the integrity metadata is a
- // mismatch, the MemoryCache must be skipped here, and a new request for the
- // resource must be made to get the raw data. This is expected to be an
- // uncommon case, however, as it implies two same-origin requests to the same
- // resource, but with different integrity metadata.
RecordSriResourceIntegrityMismatchEvent(kCheckingForIntegrityMismatch);
- if (existing_resource->MustRefetchDueToIntegrityMetadata(fetch_params)) {
- RecordSriResourceIntegrityMismatchEvent(kRefetchDueToIntegrityMismatch);
- return kReload;
- }
+ if (resource->MustRefetchDueToIntegrityMetadata(params))
+ return nullptr;
- // If the same URL has been loaded as a different type, we need to reload.
- if (existing_resource->GetType() != type) {
- // FIXME: If existingResource is a Preload and the new type is LinkPrefetch
- // We really should discard the new prefetch since the preload has more
- // specific type information! crbug.com/379893
- // fast/dom/HTMLLinkElement/link-and-subresource-test hits this case.
- RESOURCE_LOADING_DVLOG(1) << "ResourceFetcher::determineRevalidationPolicy "
- "reloading due to type mismatch.";
- return kReload;
+ if (params.IsSpeculativePreload())
+ return resource;
+ if (params.IsLinkPreload()) {
+ resource->SetLinkPreload(true);
+ return resource;
}
- // We already have a preload going for this URL.
- if (fetch_params.IsSpeculativePreload() && existing_resource->IsPreloaded())
- return kUse;
+ if (!IsReusableAlsoForPreloading(params, resource, false))
+ return nullptr;
+
+ resource->DecreasePreloadCount();
+ preloads_.erase(it);
+ matched_preloads_.push_back(resource);
+ return resource;
+}
+void ResourceFetcher::InsertAsPreloadIfNecessary(Resource* resource,
+ const FetchParameters& params,
+ Resource::Type type) {
+ if (!params.IsSpeculativePreload() && !params.IsLinkPreload())
+ return;
+ // CSP layout tests verify that preloads are subject to access checks by
+ // seeing if they are in the `preload started` list. Therefore do not add
+ // them to the list if the load is immediately denied.
+ if (resource->GetResourceError().IsAccessCheck())
+ return;
+ PreloadKey key(params.Url(), type);
+ if (preloads_.find(key) == preloads_.end()) {
+ preloads_.insert(key, resource);
+ resource->IncreasePreloadCount();
+ if (preloaded_urls_for_test_)
+ preloaded_urls_for_test_->insert(resource->Url().GetString());
+ }
+}
+
+bool ResourceFetcher::IsReusableAlsoForPreloading(const FetchParameters& params,
+ Resource* existing_resource,
+ bool is_static_data) const {
+ const ResourceRequest& request = params.GetResourceRequest();
// Do not load from cache if images are not enabled. There are two general
// cases:
//
@@ -899,13 +927,13 @@ ResourceFetcher::DetermineRevalidationPolicy(
// TODO(japhet): Can we get rid of one of these settings?
if (existing_resource->IsImage() &&
!Context().AllowImage(images_enabled_, existing_resource->Url())) {
- return kReload;
+ return false;
}
// Never use cache entries for downloadToFile / useStreamOnResponse requests.
// The data will be delivered through other paths.
if (request.DownloadToFile() || request.UseStreamOnResponse())
- return kReload;
+ return false;
// Never reuse opaque responses from a service worker for requests that are
// not no-cors. https://crbug.com/625575
@@ -913,11 +941,11 @@ ResourceFetcher::DetermineRevalidationPolicy(
existing_resource->GetResponse().ServiceWorkerResponseType() ==
kWebServiceWorkerResponseTypeOpaque &&
request.GetFetchRequestMode() != WebURLRequest::kFetchRequestModeNoCORS) {
- return kReload;
+ return false;
}
- if (!is_static_data && !existing_resource->CanReuse(fetch_params))
- return kReload;
+ if (!is_static_data && !existing_resource->CanReuse(params))
+ return false;
// Certain requests (e.g., XHRs) might have manually set headers that require
// revalidation. In theory, this should be a Revalidate case. In practice, the
@@ -933,17 +961,74 @@ ResourceFetcher::DetermineRevalidationPolicy(
if (!is_static_data &&
(request.IsConditional() ||
existing_resource->GetResponse().HttpStatusCode() == 304)) {
- return kReload;
+ return false;
}
if (!is_static_data &&
- !fetch_params.Options().CanReuseRequest(existing_resource->Options())) {
+ !params.Options().CanReuseRequest(existing_resource->Options())) {
+ return false;
+ }
+
+ return true;
+}
+
+ResourceFetcher::RevalidationPolicy
+ResourceFetcher::DetermineRevalidationPolicy(
+ Resource::Type type,
+ const FetchParameters& fetch_params,
+ Resource* existing_resource,
+ bool is_static_data) const {
+ const ResourceRequest& request = fetch_params.GetResourceRequest();
+
+ if (!existing_resource)
+ return kLoad;
+
+ // If the existing resource is loading and the associated fetcher is not equal
+ // to |this|, we must not use the resource. Otherwise, CSP violation may
+ // happen in redirect handling.
+ if (existing_resource->Loader() &&
+ existing_resource->Loader()->Fetcher() != this) {
return kReload;
}
- // Always use preloads.
- if (existing_resource->IsPreloaded())
- return kUse;
+ // Checks if the resource has an explicit policy about integrity metadata.
+ //
+ // This is necessary because ScriptResource and CSSStyleSheetResource objects
+ // do not keep the raw data around after the source is accessed once, so if
+ // the resource is accessed from the MemoryCache for a second time, there is
+ // no way to redo an integrity check.
+ //
+ // Thus, Blink implements a scheme where it caches the integrity information
+ // for those resources after the first time it is checked, and if there is
+ // another request for that resource, with the same integrity metadata, Blink
+ // skips the integrity calculation. However, if the integrity metadata is a
+ // mismatch, the MemoryCache must be skipped here, and a new request for the
+ // resource must be made to get the raw data. This is expected to be an
+ // uncommon case, however, as it implies two same-origin requests to the same
+ // resource, but with different integrity metadata.
+ RecordSriResourceIntegrityMismatchEvent(kCheckingForIntegrityMismatch);
+ if (existing_resource->MustRefetchDueToIntegrityMetadata(fetch_params)) {
+ RecordSriResourceIntegrityMismatchEvent(kRefetchDueToIntegrityMismatch);
+ return kReload;
+ }
+
+ // If the same URL has been loaded as a different type, we need to reload.
+ if (existing_resource->GetType() != type) {
+ // FIXME: If existingResource is a Preload and the new type is LinkPrefetch
+ // We really should discard the new prefetch since the preload has more
+ // specific type information! crbug.com/379893
+ // fast/dom/HTMLLinkElement/link-and-subresource-test hits this case.
+ RESOURCE_LOADING_DVLOG(1) << "ResourceFetcher::determineRevalidationPolicy "
+ "reloading due to type mismatch.";
+ return kReload;
+ }
+
+ // If |existing_resource| is not reusable as a preloaded resource, it should
+ // not be reusable as a normal resource as well.
+ if (!IsReusableAlsoForPreloading(fetch_params, existing_resource,
+ is_static_data)) {
+ return kReload;
+ }
// If resource was populated from a SubstituteData load or data: url, use it.
if (is_static_data)
@@ -1104,24 +1189,15 @@ int ResourceFetcher::NonblockingRequestCount() const {
return non_blocking_loaders_.size();
}
-void ResourceFetcher::PreloadStarted(Resource* resource) {
- if (preloads_.Contains(resource))
- return;
- resource->IncreasePreloadCount();
-
- preloads_.insert(resource);
-
- if (preloaded_urls_for_test_)
- preloaded_urls_for_test_->insert(resource->Url().GetString());
-}
-
void ResourceFetcher::EnableIsPreloadedForTest() {
if (preloaded_urls_for_test_)
return;
preloaded_urls_for_test_ = WTF::WrapUnique(new HashSet<String>);
- for (const auto& resource : preloads_)
+ for (const auto& pair : preloads_) {
+ Resource* resource = pair.value;
preloaded_urls_for_test_->insert(resource->Url().GetString());
+ }
}
bool ResourceFetcher::IsPreloadedForTest(const KURL& url) const {
@@ -1132,18 +1208,24 @@ bool ResourceFetcher::IsPreloadedForTest(const KURL& url) const {
void ResourceFetcher::ClearPreloads(ClearPreloadsPolicy policy) {
LogPreloadStats(policy);
- for (const auto& resource : preloads_) {
+ Vector<PreloadKey> keys_to_be_removed;
+ for (const auto& pair : preloads_) {
+ Resource* resource = pair.value;
if (policy == kClearAllPreloads || !resource->IsLinkPreload()) {
resource->DecreasePreloadCount();
if (resource->GetPreloadResult() == Resource::kPreloadNotReferenced)
- GetMemoryCache()->Remove(resource.Get());
- preloads_.erase(resource);
+ GetMemoryCache()->Remove(resource);
+ keys_to_be_removed.push_back(pair.key);
}
}
+ preloads_.RemoveAll(keys_to_be_removed);
+
+ matched_preloads_.clear();
}
void ResourceFetcher::WarnUnusedPreloads() {
- for (const auto& resource : preloads_) {
+ for (const auto& pair : preloads_) {
+ Resource* resource = pair.value;
if (resource && resource->IsLinkPreload() &&
resource->GetPreloadResult() == Resource::kPreloadNotReferenced) {
Context().AddConsoleMessage(
@@ -1250,6 +1332,8 @@ void ResourceFetcher::HandleLoaderError(Resource* resource,
resource->GetResponse().EncodedDataLength(),
is_internal_request);
+ if (error.IsCancellation())
+ RemovePreload(resource);
resource->FinishAsError(error);
HandleLoadCompletion(resource);
@@ -1409,7 +1493,8 @@ void ResourceFetcher::LogPreloadStats(ClearPreloadsPolicy policy) {
unsigned import_misses = 0;
unsigned raws = 0;
unsigned raw_misses = 0;
- for (const auto& resource : preloads_) {
+ for (const auto& pair : preloads_) {
+ Resource* resource = pair.value;
// Do not double count link rel preloads. These do not get cleared if the
// ClearPreloadsPolicy is only clearing speculative markup preloads.
if (resource->IsLinkPreload() &&
@@ -1563,6 +1648,7 @@ DEFINE_TRACE(ResourceFetcher) {
visitor->Trace(non_blocking_loaders_);
visitor->Trace(document_resources_);
visitor->Trace(preloads_);
+ visitor->Trace(matched_preloads_);
visitor->Trace(resource_timing_info_map_);
}

Powered by Google App Engine
This is Rietveld 408576698