From 923797bac92541669bd90bf69515a8bdb7f2c98c Mon Sep 17 00:00:00 2001 From: Yoshisato Yanagisawa Date: Thu, 07 Nov 2024 10:14:59 +0000 Subject: [PATCH] [M131] Make GetCacheIdentifier() respect GetSkipServiceWorker(). Since the current GetCacheIdentifier() ignores GetSkipServiceWorker(), GetCacheIdentifier() returns ServiceWorkerId even if GetSkipServiceWorker() is true if the ServiceWorker has a fetch handler. To make the isolated world respected as an isolated world, the cache identifier should not be shared with a page under a ServiceWorker control. (cherry picked from commit 75f322ad1f64c0bc56fa77ab877b48d72cdb903c) Bug: 372512079, 373263969 Change-Id: Idd2d8900f2f720e0a4dc9837e2eb56474c60b587 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5961018 Reviewed-by: Justin Lulejian Reviewed-by: Kouhei Ueno Commit-Queue: Yoshisato Yanagisawa Cr-Original-Commit-Position: refs/heads/main@{#1376006} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6002191 Auto-Submit: Yoshisato Yanagisawa Commit-Queue: Kouhei Ueno Cr-Commit-Position: refs/branch-heads/6778@{#1849} Cr-Branched-From: b21671ca172dcfd1566d41a770b2808e7fa7cd88-refs/heads/main@{#1368529} --- 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 index d9e38ca..c21939c 100644 --- a/third_party/blink/renderer/core/html/parser/html_srcset_parser.cc +++ b/third_party/blink/renderer/core/html/parser/html_srcset_parser.cc @@ -418,7 +418,9 @@ KURL url = document->CompleteURL( StripLeadingAndTrailingHTMLSpaces(image_candidates[i]->Url())); auto* resource = MemoryCache::Get()->ResourceForURL( - url, document->Fetcher()->GetCacheIdentifier(url)); + url, + document->Fetcher()->GetCacheIdentifier(url, + /*skip_service_worker=*/false)); if ((resource && resource->IsLoaded()) || url.ProtocolIsData()) { return i; } diff --git a/third_party/blink/renderer/core/inspector/inspector_network_agent.cc b/third_party/blink/renderer/core/inspector/inspector_network_agent.cc index 4b6a8f1b..d414c72 100644 --- a/third_party/blink/renderer/core/inspector/inspector_network_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_network_agent.cc @@ -2440,7 +2440,8 @@ Resource* cached_resource = document->Fetcher()->CachedResource(url); if (!cached_resource) { cached_resource = MemoryCache::Get()->ResourceForURL( - url, document->Fetcher()->GetCacheIdentifier(url)); + url, document->Fetcher()->GetCacheIdentifier( + url, /*skip_service_worker=*/false)); } if (cached_resource && InspectorPageAgent::CachedResourceContent( cached_resource, content, base64_encoded)) { diff --git a/third_party/blink/renderer/core/inspector/inspector_page_agent.cc b/third_party/blink/renderer/core/inspector/inspector_page_agent.cc index 2547b68..59dfb35 100644 --- a/third_party/blink/renderer/core/inspector/inspector_page_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_page_agent.cc @@ -173,7 +173,8 @@ Resource* cached_resource = document->Fetcher()->CachedResource(url); if (!cached_resource) { cached_resource = MemoryCache::Get()->ResourceForURL( - url, document->Fetcher()->GetCacheIdentifier(url)); + url, document->Fetcher()->GetCacheIdentifier( + url, /*skip_service_worker=*/false)); } 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 index 5275097..7f8ba49 100644 --- a/third_party/blink/renderer/core/loader/image_loader.cc +++ b/third_party/blink/renderer/core/loader/image_loader.cc @@ -709,7 +709,8 @@ // content when style recalc is over and DOM mutation is allowed again. if (!url.IsNull()) { Resource* resource = MemoryCache::Get()->ResourceForURL( - url, element_->GetDocument().Fetcher()->GetCacheIdentifier(url)); + url, element_->GetDocument().Fetcher()->GetCacheIdentifier( + url, /*skip_service_worker=*/false)); if (resource && !resource->ErrorOccurred() && CanReuseFromListOfAvailableImages( diff --git a/third_party/blink/renderer/core/testing/internals.cc b/third_party/blink/renderer/core/testing/internals.cc index d3e6591..601714a 100644 --- a/third_party/blink/renderer/core/testing/internals.cc +++ b/third_party/blink/renderer/core/testing/internals.cc @@ -913,8 +913,8 @@ if (!document_) return false; const KURL full_url = document_->CompleteURL(url); - const String cache_identifier = - document_->Fetcher()->GetCacheIdentifier(full_url); + const String cache_identifier = document_->Fetcher()->GetCacheIdentifier( + full_url, /*skip_service_worker=*/false); Resource* resource = MemoryCache::Get()->ResourceForURL(full_url, cache_identifier); // We check loader() here instead of isLoading(), because a multipart @@ -926,8 +926,8 @@ if (!document_) return false; const KURL full_url = document_->CompleteURL(url); - const String cache_identifier = - document_->Fetcher()->GetCacheIdentifier(full_url); + const String cache_identifier = document_->Fetcher()->GetCacheIdentifier( + full_url, /*skip_service_worker=*/false); Resource* resource = MemoryCache::Get()->ResourceForURL(full_url, cache_identifier); return resource && resource->GetStatus() == ResourceStatus::kCached; 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 8396ba14..06868ae5 100644 --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc @@ -992,7 +992,8 @@ if (!archive_ && factory.GetType() == ResourceType::kRaw) return nullptr; - const String cache_identifier = GetCacheIdentifier(url); + const String cache_identifier = GetCacheIdentifier( + url, params.GetResourceRequest().GetSkipServiceWorker()); // Most off-main-thread resource fetches use Resource::kRaw and don't reach // this point, but off-main-thread module fetches might. if (IsMainThread()) { @@ -1408,7 +1409,10 @@ MakePreloadedResourceBlockOnloadIfNeeded(resource, params); } else if (IsMainThread()) { resource = MemoryCache::Get()->ResourceForURL( - params.Url(), GetCacheIdentifier(params.Url())); + params.Url(), + GetCacheIdentifier( + params.Url(), + params.GetResourceRequest().GetSkipServiceWorker())); if (resource) { policy = DetermineRevalidationPolicy(resource_type, params, *resource, is_static_data); @@ -1719,7 +1723,8 @@ const FetchParameters& params, const ResourceFactory& factory) { const String cache_identifier = - GetCacheIdentifier(params.GetResourceRequest().Url()); + GetCacheIdentifier(params.GetResourceRequest().Url(), + params.GetResourceRequest().GetSkipServiceWorker()); DCHECK(!IsMainThread() || params.IsStaleRevalidation() || !MemoryCache::Get()->ResourceForURL(params.GetResourceRequest().Url(), cache_identifier)); @@ -2788,9 +2793,11 @@ to_be_removed.clear(); } -String ResourceFetcher::GetCacheIdentifier(const KURL& url) const { - if (properties_->GetControllerServiceWorkerMode() != - mojom::ControllerServiceWorkerMode::kNoController) { +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()); } 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 809dd567..46ae3a7 100644 --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h @@ -278,7 +278,11 @@ uint32_t inflight_keepalive_bytes); blink::mojom::ControllerServiceWorkerMode IsControlledByServiceWorker() const; - String GetCacheIdentifier(const KURL& url) const; + // Returns a cache identifier for MemoryCache. + // `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; // If `url` exists as a resource in a subresource bundle in this frame, // returns its UnguessableToken; otherwise, returns std::nullopt.