[renderblocking] Register finish callbacks for link header preloads

This patch creates a non-null PendingLinkPreload object for each link
header preload, and make it owned by the Document object. This has two
effects:

1. Now the preloaded resource has an observer owned by the Document,
   so the preload won't be cancelled if we add and remove a client
   before the preload finishes. This fixes crbug.com/1311370.

2. Now render-blocking link headers can also use PendingLinkPreload
   to block and unblock rendering, sharing the same logic with link
   elements.

There is also change to WPT header-inserted-preload-link.tentative.html.
Since there's no element for a link header, the test case cannot listen
to the 'load' event of any EventTarget. Therefore, this patch also
modifies the test code to allow waiting for the PerformanceEntry for
the link header's url.

Bug: 1271296
Fixed: 1311370
Change-Id: Ic64d26341516a510079c01dbff0b0196c7ec9f24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3551557
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988173}

third_party/blink/renderer/core/dom/document.cc

@@ -259,6 +259,7 @@
 #include "third_party/blink/renderer/core/loader/idleness_detector.h"
 #include "third_party/blink/renderer/core/loader/interactive_detector.h"
 #include "third_party/blink/renderer/core/loader/no_state_prefetch_client.h"
+#include "third_party/blink/renderer/core/loader/pending_link_preload.h"
 #include "third_party/blink/renderer/core/loader/progress_tracker.h"
 #include "third_party/blink/renderer/core/mathml/mathml_element.h"
 #include "third_party/blink/renderer/core/mathml/mathml_row_element.h"
@@ -8022,6 +8023,7 @@ void Document::Trace(Visitor* visitor) const {
   visitor->Trace(unassociated_listed_elements_);
   visitor->Trace(intrinsic_size_observer_);
   visitor->Trace(anchor_element_interaction_tracker_);
+  visitor->Trace(pending_link_header_preloads_);
   Supplementable<Document>::Trace(visitor);
   TreeScope::Trace(visitor);
   ContainerNode::Trace(visitor);
@@ -8351,6 +8353,15 @@ void Document::UnblockLoadEventAfterLayoutTreeUpdate() {
   }
 }
 
+void Document::AddPendingLinkHeaderPreload(const PendingLinkPreload& preload) {
+  pending_link_header_preloads_.insert(&preload);
+}
+
+void Document::RemovePendingLinkHeaderPreloadIfNeeded(
+    const PendingLinkPreload& preload) {
+  pending_link_header_preloads_.erase(&preload);
+}
+
 void Document::WriteIntoTrace(perfetto::TracedValue ctx) const {
   perfetto::TracedDictionary dict = std::move(ctx).WriteDictionary();
   dict.Add("url", Url());

third_party/blink/renderer/core/dom/document.h

@@ -189,6 +189,7 @@ class NodeIterator;
 class NthIndexCache;
 class Page;
 class PendingAnimations;
+class PendingLinkPreload;
 class ProcessingInstruction;
 class PropertyRegistry;
 class QualifiedName;
@@ -1837,6 +1838,11 @@ class CORE_EXPORT Document : public ContainerNode,
   // https://github.com/flackr/reduce-motion/blob/main/explainer.md
   bool ShouldForceReduceMotion() const;
 
+  void AddPendingLinkHeaderPreload(const PendingLinkPreload&);
+
+  // Has no effect if the preload is not initiated by link header.
+  void RemovePendingLinkHeaderPreloadIfNeeded(const PendingLinkPreload&);
+
   void WriteIntoTrace(perfetto::TracedValue ctx) const;
 
  protected:
@@ -2468,6 +2474,10 @@ class CORE_EXPORT Document : public ContainerNode,
   // Whether any resource loads that block printing are happening.
   bool loading_for_print_ = false;
 
+  // Document owns pending preloads, prefetches and modulepreloads initiated by
+  // link header so that they won't be incidentally GC-ed and cancelled.
+  HeapHashSet<Member<const PendingLinkPreload>> pending_link_header_preloads_;
+
   // If you want to add new data members to blink::Document, please reconsider
   // if the members really should be in blink::Document.  document.h is a very
   // popular header, and the size of document.h affects build time

third_party/blink/renderer/core/dom/document_test.cc

@@ -1763,4 +1763,39 @@ TEST_F(DocumentTest, DocumentDefiningElementWithMultipleBodies) {
   EXPECT_FALSE(body2->GetLayoutBox()->GetScrollableArea());
 }
 
+// https://crbug.com/1311370
+TEST_F(DocumentSimTest, HeaderPreloadRemoveReaddClient) {
+  SimRequest::Params main_params;
+  main_params.response_http_headers = {
+      {"Link", "<https://example.com/sheet.css>;rel=preload;as=style;"}};
+
+  SimRequest main_resource("https://example.com", "text/html", main_params);
+  SimSubresourceRequest css_resource("https://example.com/sheet.css",
+                                     "text/css");
+
+  LoadURL("https://example.com");
+  main_resource.Write(R"HTML(
+    <!doctype html>
+    <link rel="stylesheet" href="sheet.css">
+  )HTML");
+
+  // Remove and garbage-collect the pending stylesheet link element, which will
+  // remove it from the list of ResourceClients of the Resource being preloaded.
+  GetDocument().QuerySelector("link")->remove();
+  ThreadState::Current()->CollectAllGarbageForTesting();
+
+  // Removing the ResourceClient should not affect the preloading.
+  css_resource.Complete(".target { width: 100px; }");
+
+  // After the preload finishes, when a new ResourceClient is added, it should
+  // be able to use the Resource immediately.
+  main_resource.Complete(R"HTML(
+    <link rel="stylesheet" href="sheet.css">
+    <div class="target"></div>
+  )HTML");
+
+  Element* target = GetDocument().QuerySelector(".target");
+  EXPECT_EQ(100, target->OffsetWidth());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/loader/link_load_parameters.cc

@@ -54,6 +54,7 @@ LinkLoadParameters::LinkLoadParameters(const LinkHeader& header,
       referrer_policy(network::mojom::ReferrerPolicy::kDefault),
       href(KURL(base_url, header.Url())),
       image_srcset(header.ImageSrcset()),
-      image_sizes(header.ImageSizes()) {}
+      image_sizes(header.ImageSizes()),
+      blocking(header.Blocking()) {}
 
 }  // namespace blink

third_party/blink/renderer/core/loader/pending_link_preload.cc

@@ -69,13 +69,15 @@ void PendingLinkPreload::NotifyModuleLoadFinished(ModuleScript* module) {
   UnblockRendering();
   if (loader_)
     loader_->NotifyModuleLoadFinished(module);
+  document_->RemovePendingLinkHeaderPreloadIfNeeded(*this);
 }
 
 void PendingLinkPreload::NotifyFinished() {
   UnblockRendering();
   DCHECK(finish_observer_);
   if (loader_)
     loader_->NotifyFinished(finish_observer_->GetResource());
+  document_->RemovePendingLinkHeaderPreloadIfNeeded(*this);
 }
 
 void PendingLinkPreload::UnblockRendering() {
@@ -95,6 +97,7 @@ void PendingLinkPreload::Dispose() {
   if (finish_observer_)
     finish_observer_->Dispose();
   finish_observer_ = nullptr;
+  document_->RemovePendingLinkHeaderPreloadIfNeeded(*this);
 }
 
 Resource* PendingLinkPreload::GetResourceForTesting() {

third_party/blink/renderer/core/loader/preload_helper.cc

@@ -714,14 +714,16 @@ void PreloadHelper::LoadLinksFromHeader(
     }
     if (can_load_resources != kDoNotLoadResources) {
       DCHECK(document);
-      // TODO(crbug.com/1271296): Pass a PendingLinkPreload to enable
-      // render-blocking on link headers.
+      PendingLinkPreload* pending_preload =
+          MakeGarbageCollected<PendingLinkPreload>(*document,
+                                                   nullptr /* LinkLoader */);
+      document->AddPendingLinkHeaderPreload(*pending_preload);
       PreloadIfNeeded(params, *document, base_url, kLinkCalledFromHeader,
                       viewport_description, kNotParserInserted,
-                      nullptr /* PendingLinkPreload */);
-      PrefetchIfNeeded(params, *document, nullptr /* PendingLinkPreload */);
+                      pending_preload);
+      PrefetchIfNeeded(params, *document, pending_preload);
       ModulePreloadIfNeeded(params, *document, viewport_description,
-                            nullptr /* PendingLinkPreload */);
+                            pending_preload);
     }
     if (params.rel.IsServiceWorker()) {
       UseCounter::Count(document, WebFeature::kLinkHeaderServiceWorker);

third_party/blink/renderer/platform/loader/link_header.cc

@@ -7,6 +7,7 @@
 #include "base/strings/string_util.h"
 #include "components/link_header_util/link_header_util.h"
 #include "third_party/blink/public/common/web_package/signed_exchange_consts.h"
+#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/wtf/text/parsing_utilities.h"
 
 namespace blink {
@@ -56,6 +57,12 @@ static LinkHeader::LinkParameterName ParameterNameFromString(
     return LinkHeader::kLinkParameterVariants;
   if (base::EqualsCaseInsensitiveASCII(name, kSignedExchangeVariantKeyHeader))
     return LinkHeader::kLinkParameterVariantKey;
+
+  if (RuntimeEnabledFeatures::BlockingAttributeEnabled() &&
+      base::EqualsCaseInsensitiveASCII(name, "blocking")) {
+    return LinkHeader::kLinkParameterBlocking;
+  }
+
   return LinkHeader::kLinkParameterUnknown;
 }
 
@@ -86,6 +93,8 @@ void LinkHeader::SetValue(LinkParameterName name, const String& value) {
     variants_ = value;
   else if (name == kLinkParameterVariantKey)
     variant_key_ = value;
+  else if (name == kLinkParameterBlocking)
+    blocking_ = value;
 }
 
 template <typename Iterator>

third_party/blink/renderer/platform/loader/link_header.h

@@ -29,6 +29,7 @@ class LinkHeader {
   const String& HeaderIntegrity() const { return header_integrity_; }
   const String& Variants() const { return variants_; }
   const String& VariantKey() const { return variant_key_; }
+  const String& Blocking() const { return blocking_; }
   const absl::optional<String>& Anchor() const { return anchor_; }
   bool Valid() const { return is_valid_; }
   bool IsViewportDependent() const {
@@ -55,6 +56,7 @@ class LinkHeader {
     kLinkParameterHeaderIntegrity,
     kLinkParameterVariants,
     kLinkParameterVariantKey,
+    kLinkParameterBlocking,
   };
 
  private:
@@ -77,6 +79,7 @@ class LinkHeader {
   String header_integrity_;
   String variants_;
   String variant_key_;
+  String blocking_;
   absl::optional<String> anchor_;
   bool is_valid_;
 };

third_party/blink/web_tests/TestExpectations

@@ -7537,9 +7537,6 @@ crbug.com/1197296 [ Mac10.12 ] virtual/unified-autoplay/external/wpt/feature-pol
 crbug.com/1038139 [ Linux ] virtual/gpu-rasterization/images/2-comp.html [ Failure Pass ]
 crbug.com/1038139 [ Mac ] virtual/gpu-rasterization/images/2-comp.html [ Failure Pass ]
 
-# Need implementation of blocking=render on link headers
-crbug.com/626703 virtual/no-forced-frame-updates/external/wpt/html/dom/render-blocking/header-inserted-preload-link.tentative.html [ Timeout ]
-
 crbug.com/1309664 virtual/no-forced-frame-updates/external/wpt/html/dom/render-blocking/render-blocked-apis-by-preload-link.tentative.html [ Failure ]
 
 # Sheriff 2022-02-07

third_party/blink/web_tests/external/wpt/html/dom/render-blocking/header-inserted-preload-link.tentative.html

@@ -1,7 +1,8 @@
 <!doctype html>
-<title>Parser-inserted preload links with "blocking=render" are render-blocking</title>
+<title>Header-inserted preload links with "blocking=render" are render-blocking</title>
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
+<script src="/preload/resources/preload_helper.js"></script>
 <script src="support/test-render-blocking.js"></script>
 
 <style>
@@ -14,7 +15,7 @@
 
 <script>
 test_render_blocking(
-    null,
+    '/fonts/Ahem.ttf?pipe=trickle(d1)',
     () => {
       const target = document.getElementById('target');
       debugger

third_party/blink/web_tests/external/wpt/html/dom/render-blocking/support/test-render-blocking.js

@@ -1,12 +1,27 @@
+// Observes the `load` event of an EventTarget, or the finishing of a resource
+// given its url. Requires `/preload/resources/preload_helper.js` for the latter
+// usage.
 class LoadObserver {
-  constructor(object) {
+  constructor(target) {
     this.finishTime = null;
     this.load = new Promise((resolve, reject) => {
-      object.onload = ev => {
-        this.finishTime = ev.timeStamp;
-        resolve(ev);
-      };
-      object.onerror = reject;
+      if (target instanceof EventTarget) {
+        target.onload = ev => {
+          this.finishTime = ev.timeStamp;
+          resolve(ev);
+        };
+        target.onerror = reject;
+      } else if (typeof target === 'string') {
+        const observer = new PerformanceObserver(() => {
+          if (numberOfResourceTimingEntries(target)) {
+            this.finishTime = performance.now();
+            resolve();
+          }
+        });
+        observer.observe({type: 'resource', buffered: true});
+      } else {
+        reject('Unsupported target for LoadObserver');
+      }
     });
   }
 
@@ -57,16 +72,17 @@ function createAnimationTarget() {
 // are reported by different threads.
 const epsilon = 50;
 
-function test_render_blocking(optional_element, finalTest, finalTestTitle) {
+function test_render_blocking(optionalElementOrUrl, finalTest, finalTestTitle) {
   // Ideally, we should observe the 'load' event on the specific render-blocking
   // elements. However, this is not possible for script-blocking stylesheets, so
   // we have to observe the 'load' event on 'window' instead.
-  if (!(optional_element instanceof HTMLElement)) {
+  if (!(optionalElementOrUrl instanceof HTMLElement) &&
+      typeof optionalElementOrUrl !== 'string') {
     finalTestTitle = finalTest;
-    finalTest = optional_element;
-    optional_element = undefined;
+    finalTest = optionalElementOrUrl;
+    optionalElementOrUrl = undefined;
   }
-  const loadObserver = new LoadObserver(optional_element || window);
+  const loadObserver = new LoadObserver(optionalElementOrUrl || window);
 
   promise_test(async test => {
     assert_implements(window.PerformancePaintTiming);