[renderblocking] Trigger font preload timer early from preload scanner

This patch fixes a first paint performance regression.

We allow font preloading to shortly block rendering, and unblocks when
a 50ms timer fires. Before crrev.com/c/3527422, the timer is started
when preload scanner finds a font resource to preload; After the patch,
the timer is started when a font preload link element is actually
inserted into the document.

When there are parser-blocking resources (e.g., scripts) before the
preload link, then there can be a significant delay between preload
scanner finds the resource and parser inserts the element into the
document, which means the timer is started much later than before, and
therefore, much later to fire, after crrev.com/c/3527422.

This patch fixes it by letting preload scanner start the timer when a
font resource preload is started. The fix has been verified by
https://pinpoint-dot-chromeperf.appspot.com/job/15de535c4a0000

Fixed: 1308083
Change-Id: I9ef664aecc3bd7c2da213a21008593ed4c5e0ef8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3543384
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#984119}

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

@@ -712,6 +712,10 @@ Resource* PreloadHelper::StartPreload(ResourceType type,
       break;
     case ResourceType::kFont:
       resource = FontResource::Fetch(params, resource_fetcher, nullptr);
+      if (document.GetRenderBlockingResourceManager()) {
+        document.GetRenderBlockingResourceManager()
+            ->EnsureStartFontPreloadTimer();
+      }
       break;
     case ResourceType::kAudio:
     case ResourceType::kVideo:

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

@@ -67,10 +67,8 @@ void RenderBlockingResourceManager::AddPendingPreload(
     return;
 
   pending_preloads_.insert(&link, type);
-  if (type == PreloadType::kShortBlockingFont) {
-    if (!font_preload_timer_.IsActive())
-      font_preload_timer_.StartOneShot(font_preload_timeout_, FROM_HERE);
-  }
+  if (type == PreloadType::kShortBlockingFont)
+    EnsureStartFontPreloadTimer();
 }
 
 void RenderBlockingResourceManager::AddImperativeFontLoading(
@@ -85,9 +83,7 @@ void RenderBlockingResourceManager::AddImperativeFontLoading(
       MakeGarbageCollected<ImperativeFontLoadFinishedCallback>(*document_);
   font_face->AddCallback(callback);
   ++imperative_font_loading_count_;
-
-  if (!font_preload_timer_.IsActive())
-    font_preload_timer_.StartOneShot(font_preload_timeout_, FROM_HERE);
+  EnsureStartFontPreloadTimer();
 }
 
 void RenderBlockingResourceManager::RemovePendingPreload(
@@ -107,6 +103,11 @@ void RenderBlockingResourceManager::RemoveImperativeFontLoading() {
   document_->RenderBlockingResourceUnblocked();
 }
 
+void RenderBlockingResourceManager::EnsureStartFontPreloadTimer() {
+  if (!font_preload_timer_.IsActive())
+    font_preload_timer_.StartOneShot(font_preload_timeout_, FROM_HERE);
+}
+
 void RenderBlockingResourceManager::FontPreloadingTimerFired(TimerBase*) {
   font_preload_timer_has_fired_ = true;
   VectorOf<const LinkLoaderClient> short_blocking_font_preloads;
@@ -133,6 +134,10 @@ void RenderBlockingResourceManager::DisableFontPreloadTimeoutForTest() {
     font_preload_timer_.Stop();
 }
 
+bool RenderBlockingResourceManager::FontPreloadTimerIsActiveForTest() const {
+  return font_preload_timer_.IsActive();
+}
+
 bool RenderBlockingResourceManager::AddPendingStylesheet(
     const Node& owner_node) {
   if (document_->body())

third_party/blink/renderer/core/loader/render_blocking_resource_manager.h

@@ -58,6 +58,7 @@ class CORE_EXPORT RenderBlockingResourceManager final
 
   void AddImperativeFontLoading(FontFace*);
   void RemoveImperativeFontLoading();
+  void EnsureStartFontPreloadTimer();
   void FontPreloadingTimerFired(TimerBase*);
 
   void Trace(Visitor* visitor) const;
@@ -68,6 +69,7 @@ class CORE_EXPORT RenderBlockingResourceManager final
   // Exposed to unit tests only.
   void SetFontPreloadTimeoutForTest(base::TimeDelta timeout);
   void DisableFontPreloadTimeoutForTest();
+  bool FontPreloadTimerIsActiveForTest() const;
 
   Member<Document> document_;
 

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

@@ -39,6 +39,9 @@ class RenderBlockingResourceManagerTest : public SimTest {
   void SetFontPreloadTimeout(base::TimeDelta timeout) {
     GetRenderBlockingResourceManager().SetFontPreloadTimeoutForTest(timeout);
   }
+  bool FontPreloadTimerIsActive() {
+    return GetRenderBlockingResourceManager().FontPreloadTimerIsActiveForTest();
+  }
 
   Element* GetTarget() { return GetDocument().getElementById("target"); }
 
@@ -590,4 +593,41 @@ TEST_F(RenderBlockingResourceManagerTest, ScriptInsertedBodyUnblocksRendering) {
   main_resource.Finish();
 }
 
+// https://crbug.com/1308083
+TEST_F(RenderBlockingResourceManagerTest, ParserBlockingScriptBeforeFont) {
+  SimRequest main_resource("https://example.com", "text/html");
+  SimSubresourceRequest font_resource("https://example.com/font.woff2",
+                                      "font/woff2");
+  SimSubresourceRequest script_resource("https://example.com/script.js",
+                                        "application/javascript");
+
+  LoadURL("https://example.com");
+
+  // Make sure timer doesn't fire in case the test runs slow.
+  SetFontPreloadTimeout(base::TimeDelta::Max());
+
+  main_resource.Complete(R"HTML(
+    <!doctype html>
+    <script src="script.js"></script>
+    <link rel="preload" as="font" type="font/woff2"
+          href="font.woff2" crossorigin>
+    <div>
+      Lorem ipsum
+    </div>
+  )HTML");
+
+  // Rendering is still blocked.
+  EXPECT_TRUE(Compositor().DeferMainFrameUpdate());
+
+  // Parser is blocked by the synchronous script, so <link> isn't inserted yet.
+  EXPECT_FALSE(GetDocument().QuerySelector("link"));
+
+  // Preload scanner should have started font preloading and also the timer.
+  // This should happen before the parser sets up the preload link element.
+  EXPECT_TRUE(FontPreloadTimerIsActive());
+
+  script_resource.Complete();
+  font_resource.Complete();
+}
+
 }  // namespace blink