Reland "Set initiator to stylesheet's URL for resources loaded from CSS"

This is a reland of 0d5d000 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 <futhark@chromium.org> > Reviewed-by: Nate Chapin <japhet@chromium.org> > Reviewed-by: Andrey Kosyakov <caseq@chromium.org> > Commit-Queue: Alex Turner <alexmt@chromium.org> > 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 <caseq@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Alex Turner <alexmt@chromium.org> Cr-Commit-Position: refs/heads/master@{#764710}
FairyWorld · May 1, 2020 · fd014a4 · fd014a4
1 parent 2ada03f
commit fd014a4
Show file tree

Hide file tree

Showing 12 changed files with 70 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<protocol::Network::Initiator> 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<v8_inspector::protocol::Runtime::API::StackTrace>
-      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<protocol::Network::Initiator> 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<v8_inspector::protocol::Runtime::API::StackTrace>
+        current_stack_trace =
+            SourceLocation::Capture(document ? document->GetExecutionContext()
+                                             : nullptr)
+                ->BuildInspectorObject(max_async_depth);
+    if (current_stack_trace) {
+      std::unique_ptr<protocol::Network::Initiator> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/..._party/blink/web_tests/http/tests/devtools/network/resources/network-initiator-frame.html b/..._party/blink/web_tests/http/tests/devtools/network/resources/network-initiator-frame.html
@@ -4,6 +4,7 @@
 <body>
 <img src="resource.php?type=image&random=1&size=100">
 <div class="image-background">This div has background image set from CSS.</div>
+<div style="font-family: Example">This div will have font set from CSS.</div>
 <div id="div-without-class">This div will have background image set from JavaScript.</div>
 <style>@import "style.css";</style>
 <script>