From a6417cc2657b11a122620d0d32da3c84b1faa24e Mon Sep 17 00:00:00 2001 From: Keeley Hammond Date: Tue, 12 Dec 2023 01:13:31 -0800 Subject: [PATCH] build: remove font flooding in devtools patch (#40746) --- patches/chromium/.patches | 1 - .../fix_font_flooding_in_dev_tools.patch | 279 ------------------ 2 files changed, 280 deletions(-) delete mode 100644 patches/chromium/fix_font_flooding_in_dev_tools.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index ac0e9c6e6ce3..dd4a02f9da2d 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -127,5 +127,4 @@ fix_activate_background_material_on_windows.patch feat_allow_passing_of_objecttemplate_to_objecttemplatebuilder.patch chore_remove_check_is_test_on_script_injection_tracker.patch fix_restore_original_resize_performance_on_macos.patch -fix_font_flooding_in_dev_tools.patch feat_allow_code_cache_in_custom_schemes.patch diff --git a/patches/chromium/fix_font_flooding_in_dev_tools.patch b/patches/chromium/fix_font_flooding_in_dev_tools.patch deleted file mode 100644 index db7130d7344c..000000000000 --- a/patches/chromium/fix_font_flooding_in_dev_tools.patch +++ /dev/null @@ -1,279 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Ryan Manuel -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 a31f741d472830ec17291e3fad935902e042bca0..0f710af9690e54e2509a9a3abff020322047dbe0 100644 ---- a/AUTHORS -+++ b/AUTHORS -@@ -1151,6 +1151,7 @@ Rulong Chen - Russell Davis - Ryan Ackley - Ryan Gonzalez -+Ryan Manuel - Ryan Norton - Ryan Sleevi - Ryan Yoakum -diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc -index 619ccba17514b22ab9def18715047f2cefcde7a2..e0ac0cd89fc5c39cbb001b68c99392d80aff3c51 100644 ---- a/third_party/blink/common/features.cc -+++ b/third_party/blink/common/features.cc -@@ -2010,6 +2010,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 ad716edf68ec5beb50cf833af28bb1b69099af78..76917f9b8ecc9c015477f576894ceae134e8b9b8 100644 ---- a/third_party/blink/public/common/features.h -+++ b/third_party/blink/public/common/features.h -@@ -1348,6 +1348,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> - 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> 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 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(); -+ -+ // 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(); -+ -+ // 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