From fd014a49b60d6b46ce8353fcb129ed37dd5dfd45 Mon Sep 17 00:00:00 2001 From: Alex Turner Date: Fri, 1 May 2020 18:12:09 +0000 Subject: [PATCH] Reland "Set initiator to stylesheet's URL for resources loaded from CSS" This is a reland of 0d5d0000e126083d750404b8266faf1be6351c69 The original CL caused http/tests/devtools/network/network-initiator.js to become flaky. The root cause is that fetching example.ttf results in a 404 console message and so waiting for 2 console messages isn't sufficient. Instead of adding a complete font file to the resources, we only count "Done." console messages, which should prevent similar flakiness in the future. Original change's description: > Set initiator to stylesheet's URL for resources loaded from CSS > > Currently, resources loaded from CSS (e.g. fonts) do not have their > initiator set appropriately. It is often set to the script that caused > a style recalculation. Instead, we fix these cases to use the referrer, > if present, as the initiator. > > This does not fix the case where a stylesheet without a URL (e.g. inline > CSS) loads a resource. In these cases, the initiator falls back to the > document loading the resource. This also does not set the TextPosition > of the initiator as it is non-trivial to plumb that through Blink's CSS > logic. > > Bug: 918196 > Change-Id: I48120b7d91ebad439824e886b5b9d2b07bd480f2 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2163039 > Reviewed-by: Rune Lillesveen > Reviewed-by: Nate Chapin > Reviewed-by: Andrey Kosyakov > Commit-Queue: Alex Turner > Cr-Commit-Position: refs/heads/master@{#764056} Bug: 918196, 1076693 Change-Id: I101c9e38ec9cea2cf3e081f0b6a6478c23b42027 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2174346 Reviewed-by: Andrey Kosyakov Reviewed-by: Nate Chapin Reviewed-by: Rune Lillesveen Commit-Queue: Alex Turner Cr-Commit-Position: refs/heads/master@{#764710} --- .../core/css/css_font_face_src_value.cc | 1 + .../renderer/core/css/css_image_set_value.cc | 1 + .../renderer/core/css/css_image_value.cc | 1 + .../renderer/core/css/style_rule_import.cc | 2 + .../core/inspector/inspector_network_agent.cc | 37 ++++++++++++++----- .../modulescript/module_script_loader.cc | 4 +- .../loader/fetch/fetch_initiator_info.h | 9 ++++- .../devtools/network/har-content-expected.txt | 4 +- .../network/network-initiator-expected.txt | 13 +++++-- .../devtools/network/network-initiator.js | 17 +++++++-- .../devtools/network/resources/initiator.css | 4 ++ .../resources/network-initiator-frame.html | 1 + 12 files changed, 70 insertions(+), 24 deletions(-) diff --git a/third_party/blink/renderer/core/css/css_font_face_src_value.cc b/third_party/blink/renderer/core/css/css_font_face_src_value.cc index b09e467f67ffde..928d9462520a4d 100644 --- a/third_party/blink/renderer/core/css/css_font_face_src_value.cc +++ b/third_party/blink/renderer/core/css/css_font_face_src_value.cc @@ -92,6 +92,7 @@ FontResource& CSSFontFaceSrcValue::Fetch(ExecutionContext* context, resource_request.SetIsAdResource(); ResourceLoaderOptions options; options.initiator_info.name = fetch_initiator_type_names::kCSS; + options.initiator_info.referrer = referrer_.referrer; FetchParameters params(std::move(resource_request), options); if (base::FeatureList::IsEnabled( features::kWebFontsCacheAwareTimeoutAdaption)) { diff --git a/third_party/blink/renderer/core/css/css_image_set_value.cc b/third_party/blink/renderer/core/css/css_image_set_value.cc index ee836e45ad07ff..3cc48f2f6b989b 100644 --- a/third_party/blink/renderer/core/css/css_image_set_value.cc +++ b/third_party/blink/renderer/core/css/css_image_set_value.cc @@ -128,6 +128,7 @@ StyleImage* CSSImageSetValue::CacheImage( options.initiator_info.name = parser_mode_ == kUASheetMode ? fetch_initiator_type_names::kUacss : fetch_initiator_type_names::kCSS; + options.initiator_info.referrer = image.referrer.referrer; FetchParameters params(std::move(resource_request), options); if (cross_origin != kCrossOriginAttributeNotSet) { diff --git a/third_party/blink/renderer/core/css/css_image_value.cc b/third_party/blink/renderer/core/css/css_image_value.cc index 5a44f4073651f6..018ad1720cb682 100644 --- a/third_party/blink/renderer/core/css/css_image_value.cc +++ b/third_party/blink/renderer/core/css/css_image_value.cc @@ -72,6 +72,7 @@ StyleImage* CSSImageValue::CacheImage( options.initiator_info.name = initiator_name_.IsEmpty() ? fetch_initiator_type_names::kCSS : initiator_name_; + options.initiator_info.referrer = referrer_.referrer; FetchParameters params(std::move(resource_request), options); if (cross_origin != kCrossOriginAttributeNotSet) { diff --git a/third_party/blink/renderer/core/css/style_rule_import.cc b/third_party/blink/renderer/core/css/style_rule_import.cc index ae7c44280d7713..efa48fb27e5c28 100644 --- a/third_party/blink/renderer/core/css/style_rule_import.cc +++ b/third_party/blink/renderer/core/css/style_rule_import.cc @@ -150,6 +150,8 @@ void StyleRuleImport::RequestStyleSheet() { ResourceLoaderOptions options; options.initiator_info.name = fetch_initiator_type_names::kCSS; + options.initiator_info.referrer = + parent_style_sheet_->ParserContext()->GetReferrer().referrer; ResourceRequest resource_request(abs_url); if (parent_style_sheet_->ParserContext()->IsAdRelated()) resource_request.SetIsAdResource(); 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 93de4806c96985..fd21ad09a6e272 100644 --- a/third_party/blink/renderer/core/inspector/inspector_network_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_network_agent.cc @@ -1206,31 +1206,48 @@ InspectorNetworkAgent::BuildInitiatorObject( Document* document, const FetchInitiatorInfo& initiator_info, int max_async_depth) { - if (!initiator_info.imported_module_referrer.IsEmpty()) { + if (initiator_info.is_imported_module && !initiator_info.referrer.IsEmpty()) { std::unique_ptr initiator_object = protocol::Network::Initiator::create() .setType(protocol::Network::Initiator::TypeEnum::Script) .build(); - initiator_object->setUrl(initiator_info.imported_module_referrer); + initiator_object->setUrl(initiator_info.referrer); initiator_object->setLineNumber( initiator_info.position.line_.ZeroBasedInt()); return initiator_object; } - std::unique_ptr - current_stack_trace = - SourceLocation::Capture(document ? document->GetExecutionContext() - : nullptr) - ->BuildInspectorObject(max_async_depth); - if (current_stack_trace) { + bool was_requested_by_stylesheet = + initiator_info.name == fetch_initiator_type_names::kCSS || + initiator_info.name == fetch_initiator_type_names::kUacss; + if (was_requested_by_stylesheet && !initiator_info.referrer.IsEmpty()) { std::unique_ptr initiator_object = protocol::Network::Initiator::create() - .setType(protocol::Network::Initiator::TypeEnum::Script) + .setType(protocol::Network::Initiator::TypeEnum::Parser) .build(); - initiator_object->setStack(std::move(current_stack_trace)); + initiator_object->setUrl(initiator_info.referrer); return initiator_object; } + // We skip stack checking for stylesheet-initiated requests as it may + // represent the cause of a style recalculation rather than the actual + // resources themselves. See crbug.com/918196. + if (!was_requested_by_stylesheet) { + std::unique_ptr + current_stack_trace = + SourceLocation::Capture(document ? document->GetExecutionContext() + : nullptr) + ->BuildInspectorObject(max_async_depth); + if (current_stack_trace) { + std::unique_ptr initiator_object = + protocol::Network::Initiator::create() + .setType(protocol::Network::Initiator::TypeEnum::Script) + .build(); + initiator_object->setStack(std::move(current_stack_trace)); + return initiator_object; + } + } + while (document && !document->GetScriptableDocumentParser()) document = document->LocalOwner() ? document->LocalOwner()->ownerDocument() : nullptr; diff --git a/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc b/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc index 0296fc6c7866ae..60517e94b89dda 100644 --- a/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc +++ b/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc @@ -147,8 +147,8 @@ void ModuleScriptLoader::FetchInternal( options.reject_coep_unsafe_none = options_.GetRejectCoepUnsafeNone(); if (level == ModuleGraphLevel::kDependentModuleFetch) { - options.initiator_info.imported_module_referrer = - module_request.ReferrerString(); + options.initiator_info.is_imported_module = true; + options.initiator_info.referrer = module_request.ReferrerString(); options.initiator_info.position = module_request.GetReferrerPosition(); } diff --git a/third_party/blink/renderer/platform/loader/fetch/fetch_initiator_info.h b/third_party/blink/renderer/platform/loader/fetch/fetch_initiator_info.h index 4ee9d7ceeff790..9fdbb8a949b30c 100644 --- a/third_party/blink/renderer/platform/loader/fetch/fetch_initiator_info.h +++ b/third_party/blink/renderer/platform/loader/fetch/fetch_initiator_info.h @@ -38,12 +38,17 @@ struct FetchInitiatorInfo { FetchInitiatorInfo() : name(), position(TextPosition::BelowRangePosition()), - is_link_preload(false) {} + is_link_preload(false), + is_imported_module(false) {} AtomicString name; TextPosition position; bool is_link_preload; - String imported_module_referrer; + bool is_imported_module; + + // Should only be set when |is_imported_module| or when |name| is + // fetch_initiator_type_names::kCSS or fetch_initiator_type_names::kUaCSS. + String referrer; }; } // namespace blink diff --git a/third_party/blink/web_tests/http/tests/devtools/network/har-content-expected.txt b/third_party/blink/web_tests/http/tests/devtools/network/har-content-expected.txt index dcd43234d77663..90e7da5d64d964 100644 --- a/third_party/blink/web_tests/http/tests/devtools/network/har-content-expected.txt +++ b/third_party/blink/web_tests/http/tests/devtools/network/har-content-expected.txt @@ -3,8 +3,8 @@ Tests conversion of Inspector's resource representation into HAR format. initiator.css: compression: 0 mimeType: "text/css" - size: 192 - text: ".image-background {\n background-image: url(\"resource.php?type=image&random=1&size=200\");\n}\n\n.image-background-2 {\n background-image: url(\"resource.php?type=image&random=1&size=300\");\n}\n\n" + size: 262 + text: ".image-background {\n background-image: url(\"resource.php?type=image&random=1&size=200\");\n}\n\n.image-background-2 {\n background-image: url(\"resource.php?type=image&random=1&size=300\");\n}\n\n@font-face {\n font-family: Example;\n src: url(\"example.ttf\");\n}\n" binary.data: compression: 0 diff --git a/third_party/blink/web_tests/http/tests/devtools/network/network-initiator-expected.txt b/third_party/blink/web_tests/http/tests/devtools/network/network-initiator-expected.txt index afadbad200e50d..8ab9826d076b45 100644 --- a/third_party/blink/web_tests/http/tests/devtools/network/network-initiator-expected.txt +++ b/third_party/blink/web_tests/http/tests/devtools/network/network-initiator-expected.txt @@ -4,14 +4,19 @@ http://127.0.0.1:8000/devtools/network/resources/initiator.css: parser http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 2 http://127.0.0.1:8000/devtools/network/resources/resource.php?type=image&random=1&size=100: parser http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 4 +http://127.0.0.1:8000/devtools/network/resources/resource.php?type=image&random=1&size=200: parser + http://127.0.0.1:8000/devtools/network/resources/initiator.css undefined +size=300 NOT FOUND http://127.0.0.1:8000/devtools/network/resources/resource.php?type=image&random=1&size=400: script - loadData http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 12 + loadData http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 13 http://127.0.0.1:8000/devtools/network/resources/style.css: parser - http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 7 + http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 8 http://127.0.0.1:8000/devtools/network/resources/empty.html: parser - http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 16 + http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 17 http://127.0.0.1:8000/devtools/network/resources/module1.js: script - http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 18 + http://127.0.0.1:8000/devtools/network/resources/network-initiator-frame.html 19 http://127.0.0.1:8000/devtools/network/resources/module2.js: script http://127.0.0.1:8000/devtools/network/resources/module1.js 2 +http://127.0.0.1:8000/devtools/network/resources/example.ttf: parser + http://127.0.0.1:8000/devtools/network/resources/initiator.css undefined diff --git a/third_party/blink/web_tests/http/tests/devtools/network/network-initiator.js b/third_party/blink/web_tests/http/tests/devtools/network/network-initiator.js index 5c831307e13e2c..2a65baf7c7bbfe 100644 --- a/third_party/blink/web_tests/http/tests/devtools/network/network-initiator.js +++ b/third_party/blink/web_tests/http/tests/devtools/network/network-initiator.js @@ -24,10 +24,16 @@ TestRunner.evaluateInPage('loadData()'); } - var awaitedMessages = 2; - function step3() { - --awaitedMessages; - if (awaitedMessages > 0) + var expectedDoneMessages = 2; + async function step3() { + let messages = await ConsoleTestRunner.dumpConsoleMessagesIntoArray(); + let doneMessagesCount = 0; + for (let i = 0; i < messages.length; i++) { + if (messages[i].endsWith("Done.")) { + doneMessagesCount++; + } + } + if (doneMessagesCount < expectedDoneMessages) return; function dumpInitiator(url) { var matching_requests = NetworkTestRunner.findRequestsByURLPattern(new RegExp(url.replace('.', '\\.'))); @@ -54,11 +60,14 @@ dumpInitiator('initiator.css'); dumpInitiator('size=100'); + dumpInitiator('size=200'); + dumpInitiator('size=300'); dumpInitiator('size=400'); dumpInitiator('style.css'); dumpInitiator('empty.html'); dumpInitiator('module1.js'); dumpInitiator('module2.js'); + dumpInitiator('example.ttf'); TestRunner.completeTest(); } })(); diff --git a/third_party/blink/web_tests/http/tests/devtools/network/resources/initiator.css b/third_party/blink/web_tests/http/tests/devtools/network/resources/initiator.css index 5c5de7869f355d..4dc6e6c2288eef 100644 --- a/third_party/blink/web_tests/http/tests/devtools/network/resources/initiator.css +++ b/third_party/blink/web_tests/http/tests/devtools/network/resources/initiator.css @@ -6,3 +6,7 @@ background-image: url("resource.php?type=image&random=1&size=300"); } +@font-face { + font-family: Example; + src: url("example.ttf"); +} diff --git a/third_party/blink/web_tests/http/tests/devtools/network/resources/network-initiator-frame.html b/third_party/blink/web_tests/http/tests/devtools/network/resources/network-initiator-frame.html index 8dc0a162186506..1d824472747cfb 100644 --- a/third_party/blink/web_tests/http/tests/devtools/network/resources/network-initiator-frame.html +++ b/third_party/blink/web_tests/http/tests/devtools/network/resources/network-initiator-frame.html @@ -4,6 +4,7 @@
This div has background image set from CSS.
+
This div will have font set from CSS.
This div will have background image set from JavaScript.