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 | 57 ++++++++++++++++--- .../platform/loader/fetch/resource_fetcher.h | 7 ++- 8 files changed, 64 insertions(+), 23 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 @@ -426,7 +426,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 @@ -2653,7 +2653,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 @@ -179,7 +179,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 @@ -728,7 +728,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 @@ -212,7 +212,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 = @@ -228,13 +228,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(WTF::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 @@ -122,10 +122,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*); @@ -163,6 +160,8 @@ class PLATFORM_EXPORT MemoryCache final : public GarbageCollected, base::MemoryPressureListener::MemoryPressureLevel) override; 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 @@ -1023,7 +1023,8 @@ Resource* ResourceFetcher::CreateResourceForStaticData( } const String cache_identifier = GetCacheIdentifier( - url, params.GetResourceRequest().GetSkipServiceWorker()); + url, 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. if (IsMainThread()) { @@ -1460,7 +1461,8 @@ Resource* ResourceFetcher::RequestResource(FetchParameters& params, params.Url(), GetCacheIdentifier( params.Url(), - params.GetResourceRequest().GetSkipServiceWorker())); + params.GetResourceRequest().GetSkipServiceWorker(), + params.GetResourceRequest())); if (resource) { policy = DetermineRevalidationPolicy(resource_type, params, *resource, is_static_data); @@ -1785,7 +1787,8 @@ Resource* ResourceFetcher::CreateResourceForLoading( const ResourceFactory& factory) { const String cache_identifier = GetCacheIdentifier(params.GetResourceRequest().Url(), - params.GetResourceRequest().GetSkipServiceWorker()); + params.GetResourceRequest().GetSkipServiceWorker(), + params.GetResourceRequest()); DCHECK(!IsMainThread() || params.IsStaleRevalidation() || !MemoryCache::Get()->ResourceForURL(params.GetResourceRequest().Url(), cache_identifier)); @@ -2920,11 +2923,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 @@ -2939,7 +2978,7 @@ String ResourceFetcher::GetCacheIdentifier(const KURL& url, return bundle->GetCacheIdentifier(); } - return MemoryCache::DefaultCacheIdentifier(); + return origin_url; } 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 @@ -282,7 +282,12 @@ 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; // If `url` exists as a resource in a subresource bundle in this frame, // returns its UnguessableToken; otherwise, returns std::nullopt. --