From: uazo Date: Wed, 13 Jul 2022 14:51:09 +0000 Subject: Partition Blink memory cache Blink's in-memory cache is not partitioned (see also: http://crbug.com/1127971) This patch partitions it by the top-level site. This mitigation is effective in case the rendering process is re-used, because on such case the cache would be re-used as well and transfer information between different contexts. See also: * https://github.com/bromite/bromite/pull/2173 Original License: GPL-2.0-or-later - https://spdx.org/licenses/GPL-2.0-or-later.html License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html --- .../core/html/parser/html_srcset_parser.cc | 3 +- .../core/inspector/inspector_network_agent.cc | 2 +- .../core/inspector/inspector_page_agent.cc | 2 +- .../renderer/core/loader/image_loader.cc | 3 +- .../platform/loader/fetch/memory_cache.cc | 8 +-- .../platform/loader/fetch/memory_cache.h | 5 +- .../platform/loader/fetch/resource_fetcher.cc | 62 +++++++++++++++---- .../platform/loader/fetch/resource_fetcher.h | 10 ++- .../platform/runtime_enabled_features.json5 | 3 +- 9 files changed, 70 insertions(+), 28 deletions(-) diff --git a/third_party/blink/renderer/core/html/parser/html_srcset_parser.cc b/third_party/blink/renderer/core/html/parser/html_srcset_parser.cc --- a/third_party/blink/renderer/core/html/parser/html_srcset_parser.cc +++ b/third_party/blink/renderer/core/html/parser/html_srcset_parser.cc @@ -425,7 +425,8 @@ static unsigned AvoidDownloadIfHigherDensityResourceIsInCache( auto* resource = MemoryCache::Get()->ResourceForURL( url, document->Fetcher()->GetCacheIdentifier(url, - /*skip_service_worker=*/false)); + /*skip_service_worker=*/false, + document->TopFrameOrigin())); if (resource && resource->IsLoaded()) { UseCounter::Count(document, WebFeature::kSrcSetUsedHigherDensityImageFromCache); diff --git a/third_party/blink/renderer/core/inspector/inspector_network_agent.cc b/third_party/blink/renderer/core/inspector/inspector_network_agent.cc --- a/third_party/blink/renderer/core/inspector/inspector_network_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_network_agent.cc @@ -2806,7 +2806,7 @@ bool InspectorNetworkAgent::FetchResourceContent(Document* document, if (!cached_resource) { cached_resource = MemoryCache::Get()->ResourceForURL( url, document->Fetcher()->GetCacheIdentifier( - url, /*skip_service_worker=*/false)); + url, /*skip_service_worker=*/false, document->TopFrameOrigin())); } if (cached_resource && InspectorPageAgent::CachedResourceContent(cached_resource, content, diff --git a/third_party/blink/renderer/core/inspector/inspector_page_agent.cc b/third_party/blink/renderer/core/inspector/inspector_page_agent.cc --- a/third_party/blink/renderer/core/inspector/inspector_page_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_page_agent.cc @@ -182,7 +182,7 @@ Resource* CachedResource(LocalFrame* frame, if (!cached_resource) { cached_resource = MemoryCache::Get()->ResourceForURL( url, document->Fetcher()->GetCacheIdentifier( - url, /*skip_service_worker=*/false)); + url, /*skip_service_worker=*/false, document->TopFrameOrigin())); } if (!cached_resource) { cached_resource = loader->ResourceForURL(url); diff --git a/third_party/blink/renderer/core/loader/image_loader.cc b/third_party/blink/renderer/core/loader/image_loader.cc --- a/third_party/blink/renderer/core/loader/image_loader.cc +++ b/third_party/blink/renderer/core/loader/image_loader.cc @@ -749,7 +749,8 @@ bool ImageLoader::ShouldLoadImmediately(const KURL& url) const { if (!url.IsNull()) { Resource* resource = MemoryCache::Get()->ResourceForURL( url, element_->GetDocument().Fetcher()->GetCacheIdentifier( - url, /*skip_service_worker=*/false)); + url, /*skip_service_worker=*/false, + element_->GetDocument().TopFrameOrigin())); if (resource && !resource->ErrorOccurred() && CanReuseFromListOfAvailableImages( diff --git a/third_party/blink/renderer/platform/loader/fetch/memory_cache.cc b/third_party/blink/renderer/platform/loader/fetch/memory_cache.cc --- a/third_party/blink/renderer/platform/loader/fetch/memory_cache.cc +++ b/third_party/blink/renderer/platform/loader/fetch/memory_cache.cc @@ -354,7 +354,7 @@ void MemoryCache::RemoveInternal(ResourceMap* resource_map, } bool MemoryCache::Contains(const Resource* resource) const { - if (!resource || resource->Url().IsEmpty()) + if (!resource || resource->Url().IsEmpty() || resource->CacheIdentifier().empty()) return false; const auto resource_maps_it = @@ -370,13 +370,9 @@ bool MemoryCache::Contains(const Resource* resource) const { return resource == resources_it->value->GetResource(); } -Resource* MemoryCache::ResourceForURLForTesting( - const KURL& resource_url) const { - return ResourceForURL(resource_url, DefaultCacheIdentifier()); -} - Resource* MemoryCache::ResourceForURL(const KURL& resource_url, const String& cache_identifier) const { + if (cache_identifier.empty()) return nullptr; DCHECK(IsMainThread()); if (!resource_url.IsValid() || resource_url.IsNull()) return nullptr; diff --git a/third_party/blink/renderer/platform/loader/fetch/memory_cache.h b/third_party/blink/renderer/platform/loader/fetch/memory_cache.h --- a/third_party/blink/renderer/platform/loader/fetch/memory_cache.h +++ b/third_party/blink/renderer/platform/loader/fetch/memory_cache.h @@ -131,10 +131,7 @@ class PLATFORM_EXPORT MemoryCache final : public GarbageCollected, // Do not use this method outside test purposes. // A resourfe URL is not enough to do a correct MemoryCache lookup, and // relying on the method would likely yield wrong results. - Resource* ResourceForURLForTesting(const KURL&) const; - Resource* ResourceForURL(const KURL&, const String& cache_identifier) const; - HeapVector> ResourcesForURL(const KURL&) const; void Add(Resource*); void Remove(Resource*); @@ -178,6 +175,8 @@ class PLATFORM_EXPORT MemoryCache final : public GarbageCollected, bool HasStrongReferenceForTesting(Resource*) const; private: + HeapVector> ResourcesForURL(const KURL&) const; + // A URL-based map of all resources that are in the cache (including the // freshest version of objects that are currently being referenced by a Web // page). removeFragmentIdentifierIfNeeded() should be called for the url diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc @@ -1016,7 +1016,8 @@ Resource* ResourceFetcher::CreateResourceForStaticData( const String cache_identifier = GetCacheIdentifier(factory.GetType(), url, - params.GetResourceRequest().GetSkipServiceWorker()); + params.GetResourceRequest().GetSkipServiceWorker(), + params.GetResourceRequest()); // Most off-main-thread resource fetches use Resource::kRaw and don't reach // this point, but off-main-thread module fetches might. @@ -1477,7 +1478,8 @@ Resource* ResourceFetcher::RequestResource(FetchParameters& params, } else if (IsMainThread()) { const String cache_identifier = GetCacheIdentifier( resource_type, params.GetResourceRequest().Url(), - params.GetResourceRequest().GetSkipServiceWorker()); + params.GetResourceRequest().GetSkipServiceWorker(), + params.GetResourceRequest()); resource = MemoryCache::Get()->ResourceForURL(params.Url(), cache_identifier); @@ -1787,7 +1789,8 @@ Resource* ResourceFetcher::CreateResourceForLoading( const ResourceFactory& factory) { const String cache_identifier = GetCacheIdentifier(factory.GetType(), params.GetResourceRequest().Url(), - params.GetResourceRequest().GetSkipServiceWorker()); + params.GetResourceRequest().GetSkipServiceWorker(), + params.GetResourceRequest()); DCHECK(!IsMainThread() || params.IsStaleRevalidation() || !MemoryCache::Get()->ResourceForURL(params.GetResourceRequest().Url(), cache_identifier)); @@ -2936,11 +2939,47 @@ void ResourceFetcher::UpdateImagePrioritiesAndSpeculativeDecodes() { } String ResourceFetcher::GetCacheIdentifier(const KURL& url, - bool skip_service_worker) const { - if (!skip_service_worker && - properties_->GetControllerServiceWorkerMode() != - mojom::ControllerServiceWorkerMode::kNoController) { - return String::Number(properties_->ServiceWorkerId()); + bool skip_service_worker, + const ResourceRequest& resource_request) const { + if (const scoped_refptr top_origin = + resource_request.TopFrameOrigin()) { + String origin_url = top_origin ? top_origin->ToRawString() : ""; + String cache_identifier = ResourceFetcher::GetCacheIdentifier( + url, skip_service_worker, origin_url); + // LOG(INFO) << "---t (" << cache_identifier << ") " + // << url.GetString() << "='" << origin_url << "'"; + return cache_identifier; + } + // service workers cannot use the memory cache + // } else if (resource_request.GetRequestContext() == + // mojom::blink::RequestContextType::SERVICE_WORKER) { + // const scoped_refptr requestor_origin = + // resource_request.RequestorOrigin(); + // String origin_url = requestor_origin + // ? requestor_origin->ToRawString() + // : ""; //context_.Url()->ToRawString(); + // String cache_identifier = ResourceFetcher::GetCacheIdentifier( + // url, skip_service_worker, origin_url); + // LOG(INFO) << "---o (" << cache_identifier << ") " + // << url.GetString() << "='" << origin_url << "'"; + // return cache_identifier; + // } + return MemoryCache::DefaultCacheIdentifier(); +} + +String ResourceFetcher::GetCacheIdentifier(const KURL& url, + bool skip_service_worker, + scoped_refptr origin) const { + String origin_url = origin ? origin->ToRawString() : ""; + return ResourceFetcher::GetCacheIdentifier(url, skip_service_worker, origin_url); +} + +String ResourceFetcher::GetCacheIdentifier(const KURL& url, + bool skip_service_worker, + const String origin_url) const { + if (!skip_service_worker && properties_->GetControllerServiceWorkerMode() != + mojom::ControllerServiceWorkerMode::kNoController) { + return origin_url + " " + String::Number(properties_->ServiceWorkerId()); } // Requests that can be satisfied via `archive_` (i.e. MHTML) or @@ -2955,12 +2994,13 @@ String ResourceFetcher::GetCacheIdentifier(const KURL& url, return bundle->GetCacheIdentifier(); } - return MemoryCache::DefaultCacheIdentifier(); + return origin_url; } String ResourceFetcher::GetCacheIdentifier(ResourceType type, const KURL& url, - bool skip_service_worker) const { + bool skip_service_worker, + const ResourceRequest& resource_request) const { // For SVG resource documents, use the SVG-specific cache identifier when the // feature is enabled and a cache identifier is available from the fetch // context. @@ -2973,7 +3013,7 @@ String ResourceFetcher::GetCacheIdentifier(ResourceType type, } // Fallback to the standard cache identifier logic. - return GetCacheIdentifier(url, skip_service_worker); + return GetCacheIdentifier(url, skip_service_worker, resource_request); } std::optional diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h @@ -284,10 +284,16 @@ class PLATFORM_EXPORT ResourceFetcher // `url` is used for finding a matching WebBundle. // If `skip_service_worker` is true, the identifier won't be a ServiceWorker's // identifier to keep the cache separated. - String GetCacheIdentifier(const KURL& url, bool skip_service_worker) const; + String GetCacheIdentifier(const KURL& url, bool skip_service_worker, + const ResourceRequest& resource_request) const; + String GetCacheIdentifier(const KURL& url, bool skip_service_worker, + scoped_refptr origin) const; + String GetCacheIdentifier(const KURL& url, bool skip_service_worker, + const String origin_url) const; String GetCacheIdentifier(ResourceType type, const KURL& url, - bool skip_service_worker) const; + bool skip_service_worker, + const ResourceRequest& resource_request) const; // If `url` exists as a resource in a subresource bundle in this frame, // returns its UnguessableToken; otherwise, returns std::nullopt. diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5 --- a/third_party/blink/renderer/platform/runtime_enabled_features.json5 +++ b/third_party/blink/renderer/platform/runtime_enabled_features.json5 @@ -5594,8 +5594,7 @@ status: "stable", }, { - name: "SvgPartitionSVGDocumentResourcesInMemoryCache", - status: "stable", + status: "experimental", name: "SvgPartitionSVGDocumentResourcesInMemoryCache", }, { name: "SvgPathLengthCssProperty", --