-
Notifications
You must be signed in to change notification settings - Fork 15.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: cherry pick 9009d76968b1ec2ed825bc95e47d086ceea07520 from chromi…
…um (#40688) * chore: cherry pick 9009d76968b1ec2ed825bc95e47d086ceea07520 from chromium Co-authored-by: Ryan Manuel <[email protected]> * update patch message Co-authored-by: Ryan Manuel <[email protected]> * chore: update patches --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Ryan Manuel <[email protected]> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
- Loading branch information
3 people
authored
Dec 4, 2023
1 parent
e809537
commit 34d115a
Showing
2 changed files
with
280 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,279 @@ | ||
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
| From: Ryan Manuel <[email protected]> | ||
| Date: Fri, 1 Dec 2023 16:40:02 -0600 | ||
| Subject: fix font flooding in dev tools | ||
|
|
||
| Added in this CL: https://chromium-review.googlesource.com/c/chromium/src/+/5033885 | ||
|
|
||
| This patch resolves an issue that has been fixed in chromium involving font requests being sent multiple times to DevTools for a single request. | ||
|
|
||
| This patch can be removed when chromium reaches version 121.0.6157.0. | ||
|
|
||
| diff --git a/AUTHORS b/AUTHORS | ||
| index dec500d09880d1bbe22af60b1a00d1632b24dbca..bc153bc670f2938bf270e37b24c6243f1aac2493 100644 | ||
| --- a/AUTHORS | ||
| +++ b/AUTHORS | ||
| @@ -1144,6 +1144,7 @@ Rulong Chen <[email protected]> | ||
| Russell Davis <[email protected]> | ||
| Ryan Ackley <[email protected]> | ||
| Ryan Gonzalez <[email protected]> | ||
| +Ryan Manuel <[email protected]> | ||
| Ryan Norton <[email protected]> | ||
| Ryan Sleevi <[email protected]> | ||
| Ryan Yoakum <[email protected]> | ||
| diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc | ||
| index 8345548b0701dbe284b608819a63cef6b8940994..671a04eede5699d7ac28b37ecd681a2dcaa9fad2 100644 | ||
| --- a/third_party/blink/common/features.cc | ||
| +++ b/third_party/blink/common/features.cc | ||
| @@ -1961,6 +1961,14 @@ BASE_FEATURE(kUACHOverrideBlank, | ||
| "UACHOverrideBlank", | ||
| base::FEATURE_DISABLED_BY_DEFAULT); | ||
|
|
||
| +// If enabled, the body of `EmulateLoadStartedForInspector` is executed only | ||
| +// once per Resource per ResourceFetcher, and thus duplicated network load | ||
| +// entries in DevTools caused by `EmulateLoadStartedForInspector` are removed. | ||
| +// https://crbug.com/1502591 | ||
| +BASE_FEATURE(kEmulateLoadStartedForInspectorOncePerResource, | ||
| + "kEmulateLoadStartedForInspectorOncePerResource", | ||
| + base::FEATURE_ENABLED_BY_DEFAULT); | ||
| + | ||
| BASE_FEATURE(kUseBlinkSchedulerTaskRunnerWithCustomDeleter, | ||
| "UseBlinkSchedulerTaskRunnerWithCustomDeleter", | ||
| base::FEATURE_ENABLED_BY_DEFAULT); | ||
| diff --git a/third_party/blink/public/common/features.h b/third_party/blink/public/common/features.h | ||
| index e611ceb2afaf89bca3f3822fd3816a733e23cc43..67d083a37064ad57d727ffaeaa9f415f4bc0a5e3 100644 | ||
| --- a/third_party/blink/public/common/features.h | ||
| +++ b/third_party/blink/public/common/features.h | ||
| @@ -1292,6 +1292,9 @@ BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kTimedHTMLParserBudget); | ||
|
|
||
| BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kUACHOverrideBlank); | ||
|
|
||
| +BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE( | ||
| + kEmulateLoadStartedForInspectorOncePerResource); | ||
| + | ||
| // Kill switch for using a custom task runner in the blink scheduler that makes | ||
| // DeleteSoon/ReleaseSoon less prone to memory leaks. | ||
| BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE( | ||
| 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 3d889e448fe706e69eac40e61035616b2be3f74c..d8d5b0e0379c4fc65debc4e25766997e5502191e 100644 | ||
| --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc | ||
| +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc | ||
| @@ -681,6 +681,19 @@ Resource* ResourceFetcher::CachedResource(const KURL& resource_url) const { | ||
| return it->value.Get(); | ||
| } | ||
|
|
||
| +bool ResourceFetcher::ResourceHasBeenEmulatedLoadStartedForInspector( | ||
| + const KURL& resource_url) const { | ||
| + if (resource_url.IsEmpty()) { | ||
| + return false; | ||
| + } | ||
| + KURL url = MemoryCache::RemoveFragmentIdentifierIfNeeded(resource_url); | ||
| + const auto it = emulated_load_started_for_inspector_resources_map_.find(url); | ||
| + if (it == emulated_load_started_for_inspector_resources_map_.end()) { | ||
| + return false; | ||
| + } | ||
| + return true; | ||
| +} | ||
| + | ||
| const HeapHashSet<Member<Resource>> | ||
| ResourceFetcher::MoveResourceStrongReferences() { | ||
| document_resource_strong_refs_total_size_ = 0; | ||
| @@ -2463,11 +2476,25 @@ void ResourceFetcher::EmulateLoadStartedForInspector( | ||
| if (CachedResource(url)) { | ||
| return; | ||
| } | ||
| + | ||
| + if (ResourceHasBeenEmulatedLoadStartedForInspector(url)) { | ||
| + return; | ||
| + } | ||
| + | ||
| if (resource->ErrorOccurred()) { | ||
| // We should ideally replay the error steps, but we cannot. | ||
| return; | ||
| } | ||
|
|
||
| + if (base::FeatureList::IsEnabled( | ||
| + features::kEmulateLoadStartedForInspectorOncePerResource)) { | ||
| + // Update the emulated load started for inspector resources map with the | ||
| + // resource so that future emulations of the same resource won't happen. | ||
| + String resource_url = MemoryCache::RemoveFragmentIdentifierIfNeeded(url); | ||
| + emulated_load_started_for_inspector_resources_map_.Set(resource_url, | ||
| + resource); | ||
| + } | ||
| + | ||
| ResourceRequest resource_request(url); | ||
| resource_request.SetRequestContext(request_context); | ||
| resource_request.SetRequestDestination(request_destination); | ||
| @@ -2767,6 +2794,7 @@ void ResourceFetcher::Trace(Visitor* visitor) const { | ||
| visitor->Trace(loaders_); | ||
| visitor->Trace(non_blocking_loaders_); | ||
| visitor->Trace(cached_resources_map_); | ||
| + visitor->Trace(emulated_load_started_for_inspector_resources_map_); | ||
| visitor->Trace(image_resources_); | ||
| visitor->Trace(not_loaded_image_resources_); | ||
| visitor->Trace(preloads_); | ||
| 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 b0775b4d44bf800159d0c8ef07a2258df8674b94..af96732a045c35ab443b492bd5b34868eb1ba73e 100644 | ||
| --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h | ||
| +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h | ||
| @@ -178,6 +178,7 @@ class PLATFORM_EXPORT ResourceFetcher | ||
| CodeCacheHost* GetCodeCacheHost(); | ||
|
|
||
| Resource* CachedResource(const KURL&) const; | ||
| + bool ResourceHasBeenEmulatedLoadStartedForInspector(const KURL&) const; | ||
|
|
||
| // Registers an callback to be called with the resource priority of the fetch | ||
| // made to the specified URL. When `new_load_only` is set to false, | ||
| @@ -564,6 +565,15 @@ class PLATFORM_EXPORT ResourceFetcher | ||
| // Weak reference to all the fetched resources. | ||
| DocumentResourceMap cached_resources_map_; | ||
|
|
||
| + // When a resource is in the global memory cache but not in the | ||
| + // cached_resources_map_ and it is referenced (e.g. when the StyleEngine | ||
| + // processes a @font-face rule), the resource will be emulated via | ||
| + // `EmulateLoadStartedForInspector` so that it shows up in DevTools. | ||
| + // In order to ensure that this only occurs once per resource, we keep | ||
| + // a weak reference to all emulated resources and only emulate resources | ||
| + // that have not been previously emulated. | ||
| + DocumentResourceMap emulated_load_started_for_inspector_resources_map_; | ||
| + | ||
| // document_resource_strong_refs_ keeps strong references for fonts, images, | ||
| // scripts and stylesheets within their freshness lifetime. | ||
| HeapHashSet<Member<Resource>> document_resource_strong_refs_; | ||
| diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher_test.cc b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher_test.cc | ||
| index d1947b5070cd537b29fdf4cb1a9e6780e803111f..6f8ef7af61abd84519543ee7628fb1e88ab9b5de 100644 | ||
| --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher_test.cc | ||
| +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher_test.cc | ||
| @@ -161,6 +161,8 @@ class ResourceFetcherTest : public testing::Test { | ||
| return request_; | ||
| } | ||
|
|
||
| + void ClearLastRequest() { request_ = absl::nullopt; } | ||
| + | ||
| private: | ||
| absl::optional<PartialResourceRequest> request_; | ||
| }; | ||
| @@ -1627,4 +1629,123 @@ TEST_F(ResourceFetcherTest, StrongReferenceThreshold) { | ||
| ASSERT_FALSE(perform_fetch.Run(KURL("http://127.0.0.1:8000/baz.png"))); | ||
| } | ||
|
|
||
| +TEST_F(ResourceFetcherTest, | ||
| + EmulateLoadStartedForInspectorOncePerResourceDisabled) { | ||
| + base::test::ScopedFeatureList scoped_feature_list; | ||
| + scoped_feature_list.InitAndDisableFeature( | ||
| + features::kEmulateLoadStartedForInspectorOncePerResource); | ||
| + auto* observer = MakeGarbageCollected<TestResourceLoadObserver>(); | ||
| + | ||
| + // Set up the initial fetcher and mark the resource as cached. | ||
| + auto* fetcher = CreateFetcher(); | ||
| + KURL url("http://127.0.0.1:8000/foo.woff2"); | ||
| + RegisterMockedURLLoad(url); | ||
| + FetchParameters fetch_params = | ||
| + FetchParameters::CreateForTest(ResourceRequest(url)); | ||
| + Resource* resource = MockResource::Fetch(fetch_params, fetcher, nullptr); | ||
| + resource->SetStatus(ResourceStatus::kCached); | ||
| + | ||
| + ASSERT_NE(fetcher->CachedResource(url), nullptr); | ||
| + ASSERT_FALSE(fetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + | ||
| + // Set up the second fetcher. | ||
| + auto* otherContextFetcher = CreateFetcher(); | ||
| + otherContextFetcher->SetResourceLoadObserver(observer); | ||
| + | ||
| + // Ensure that the url is initially not marked as cached or | ||
| + // emulated and the observer's last request is empty. | ||
| + ASSERT_EQ(otherContextFetcher->CachedResource(url), nullptr); | ||
| + ASSERT_FALSE( | ||
| + otherContextFetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + ASSERT_EQ(observer->GetLastRequest(), absl::nullopt); | ||
| + | ||
| + otherContextFetcher->EmulateLoadStartedForInspector( | ||
| + resource, url, mojom::blink::RequestContextType::FONT, | ||
| + network::mojom::RequestDestination::kFont, | ||
| + fetch_initiator_type_names::kCSS); | ||
| + | ||
| + // After the first emulation, ensure that the url is not cached, | ||
| + // is not marked as emulated and the observer's last | ||
| + // request is not empty with the feature disabled. | ||
| + ASSERT_EQ(otherContextFetcher->CachedResource(url), nullptr); | ||
| + ASSERT_FALSE( | ||
| + otherContextFetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + ASSERT_NE(observer->GetLastRequest(), absl::nullopt); | ||
| + | ||
| + // Clear out the last request to start fresh | ||
| + observer->ClearLastRequest(); | ||
| + | ||
| + otherContextFetcher->EmulateLoadStartedForInspector( | ||
| + resource, url, mojom::blink::RequestContextType::FONT, | ||
| + network::mojom::RequestDestination::kFont, | ||
| + fetch_initiator_type_names::kCSS); | ||
| + | ||
| + // After the second emulation, ensure that the url is not cached, | ||
| + // the resource is not marked as emulated, and the observer's last | ||
| + // request is not empty with the feature disabled. This means that | ||
| + // the observer was notified with this emulation. | ||
| + ASSERT_EQ(otherContextFetcher->CachedResource(url), nullptr); | ||
| + ASSERT_FALSE( | ||
| + otherContextFetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + ASSERT_NE(observer->GetLastRequest(), absl::nullopt); | ||
| +} | ||
| + | ||
| +TEST_F(ResourceFetcherTest, | ||
| + EmulateLoadStartedForInspectorOncePerResourceEnabled) { | ||
| + auto* observer = MakeGarbageCollected<TestResourceLoadObserver>(); | ||
| + | ||
| + // Set up the initial fetcher and mark the resource as cached. | ||
| + auto* fetcher = CreateFetcher(); | ||
| + KURL url("http://127.0.0.1:8000/foo.woff2"); | ||
| + RegisterMockedURLLoad(url); | ||
| + FetchParameters fetch_params = | ||
| + FetchParameters::CreateForTest(ResourceRequest(url)); | ||
| + Resource* resource = MockResource::Fetch(fetch_params, fetcher, nullptr); | ||
| + resource->SetStatus(ResourceStatus::kCached); | ||
| + | ||
| + ASSERT_NE(fetcher->CachedResource(url), nullptr); | ||
| + ASSERT_FALSE(fetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + | ||
| + // Set up the second fetcher. | ||
| + auto* otherContextFetcher = CreateFetcher(); | ||
| + otherContextFetcher->SetResourceLoadObserver(observer); | ||
| + | ||
| + // Ensure that the url is initially not cached, not marked as emulated, | ||
| + // and the observer's last request is empty. | ||
| + ASSERT_EQ(otherContextFetcher->CachedResource(url), nullptr); | ||
| + ASSERT_FALSE( | ||
| + otherContextFetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + ASSERT_EQ(observer->GetLastRequest(), absl::nullopt); | ||
| + | ||
| + otherContextFetcher->EmulateLoadStartedForInspector( | ||
| + resource, url, mojom::blink::RequestContextType::FONT, | ||
| + network::mojom::RequestDestination::kFont, | ||
| + fetch_initiator_type_names::kCSS); | ||
| + | ||
| + // After the first emulation, ensure that the url is not cached, | ||
| + // marked as emulated, and the observer's last request is not empty with | ||
| + // the feature enabled. | ||
| + ASSERT_EQ(otherContextFetcher->CachedResource(url), nullptr); | ||
| + ASSERT_TRUE( | ||
| + otherContextFetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + ASSERT_NE(observer->GetLastRequest(), absl::nullopt); | ||
| + | ||
| + // Clear out the last request to start fresh | ||
| + observer->ClearLastRequest(); | ||
| + | ||
| + otherContextFetcher->EmulateLoadStartedForInspector( | ||
| + resource, url, mojom::blink::RequestContextType::FONT, | ||
| + network::mojom::RequestDestination::kFont, | ||
| + fetch_initiator_type_names::kCSS); | ||
| + | ||
| + // After the first emulation, ensure that the url is not cached, | ||
| + // marked as emulated, and the observer's last request is empty with | ||
| + // the feature enabled. This means that the observer was not | ||
| + // notified with this emulation. | ||
| + ASSERT_EQ(otherContextFetcher->CachedResource(url), nullptr); | ||
| + ASSERT_TRUE( | ||
| + otherContextFetcher->ResourceHasBeenEmulatedLoadStartedForInspector(url)); | ||
| + ASSERT_EQ(observer->GetLastRequest(), absl::nullopt); | ||
| +} | ||
| + | ||
| } // namespace blink |