Tweak 'font-display:auto' loading timeline to improve LCP and CLS

This patch adds a feature that, after 2000ms since navigation start,
all pending 'font-display: auto' web fonts will immediately enter the
failure display period. This prevents web fonts to become a source of
bad LCP or CLS.

Note: The value "2000ms" might need to be adjusted later for the best
results. See https://crbug.com/1065508 for more details.

Bug: 1065508
Change-Id: Ic04de331221057b019d37f421ed61050929caa0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2133113
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757992}

third_party/blink/common/features.cc

@@ -499,5 +499,17 @@ const base::Feature kAppCache{"AppCache", base::FEATURE_ENABLED_BY_DEFAULT};
 // Enables the AV1 Image File Format (AVIF).
 const base::Feature kAVIF{"AVIF", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Make all pending 'display: auto' web fonts enter the failure period
+// immediately before reaching the LCP time limit (~2500ms), so that web fonts
+// do not become a source of bad LCP.
+const base::Feature kAlignFontDisplayAutoTimeoutWithLCPGoal{
+    "AlignFontDisplayAutoTimeoutWithLCPGoal",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
+// The amount of time allowed for 'display: auto' web fonts to load, counted
+// from navigation start.
+const base::FeatureParam<int> kAlignFontDisplayAutoTimeoutWithLCPGoalParam{
+    &kAlignFontDisplayAutoTimeoutWithLCPGoal, "lcp-limit-in-ms", 2000};
+
 }  // namespace features
 }  // namespace blink

third_party/blink/public/common/features.h

@@ -169,6 +169,11 @@ BLINK_COMMON_EXPORT extern const base::Feature kAppCache;
 
 BLINK_COMMON_EXPORT extern const base::Feature kAVIF;
 
+BLINK_COMMON_EXPORT extern const base::Feature
+    kAlignFontDisplayAutoTimeoutWithLCPGoal;
+BLINK_COMMON_EXPORT extern const base::FeatureParam<int>
+    kAlignFontDisplayAutoTimeoutWithLCPGoalParam;
+
 }  // namespace features
 }  // namespace blink
 

third_party/blink/renderer/core/BUILD.gn

@@ -1052,6 +1052,7 @@ jumbo_source_set("unit_tests") {
     "css/css_test_helpers.h",
     "css/css_uri_value_test.cc",
     "css/css_value_test_helper.h",
+    "css/font_display_auto_lcp_align_test.cc",
     "css/mock_css_paint_image_generator.h",
     "display_lock/display_lock_context_test.cc",
     "display_lock/display_lock_utilities_test.cc",

third_party/blink/renderer/core/css/css_font_face.cc

@@ -228,6 +228,13 @@ void CSSFontFace::SetLoadStatus(FontFace::LoadStatusType new_status) {
   }
 }
 
+void CSSFontFace::UpdatePeriod() {
+  if (LoadStatus() == FontFace::kLoaded)
+    return;
+  for (CSSFontFaceSource* source : sources_)
+    source->UpdatePeriod();
+}
+
 void CSSFontFace::Trace(Visitor* visitor) {
   visitor->Trace(segmented_font_faces_);
   visitor->Trace(sources_);

third_party/blink/renderer/core/css/css_font_face.h

@@ -80,6 +80,10 @@ class CORE_EXPORT CSSFontFace final : public GarbageCollected<CSSFontFace> {
   void Load();
   void Load(const FontDescription&);
 
+  // Recalculate the font loading timeline period for the font face.
+  // https://drafts.csswg.org/css-fonts-4/#font-display-timeline
+  void UpdatePeriod();
+
   bool HadBlankText() { return IsValid() && sources_.front()->HadBlankText(); }
 
   void Trace(Visitor*);

third_party/blink/renderer/core/css/css_font_face_source.h

@@ -70,6 +70,10 @@ class CORE_EXPORT CSSFontFaceSource
   virtual bool IsInBlockPeriod() const { return false; }
   virtual bool IsInFailurePeriod() const { return false; }
 
+  // Recalculate the font loading timeline period for the font face.
+  // https://drafts.csswg.org/css-fonts-4/#font-display-timeline
+  virtual void UpdatePeriod() {}
+
   // For UMA reporting
   virtual bool HadBlankText() { return false; }
   virtual void PaintRequested() {}

third_party/blink/renderer/core/css/font_display_auto_lcp_align_test.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/test/scoped_feature_list.h"
+#include "third_party/blink/public/common/features.h"
+#include "third_party/blink/renderer/core/css/font_face_set_document.h"
+#include "third_party/blink/renderer/core/dom/element.h"
+#include "third_party/blink/renderer/core/layout/layout_object.h"
+#include "third_party/blink/renderer/core/loader/document_loader.h"
+#include "third_party/blink/renderer/core/style/computed_style.h"
+#include "third_party/blink/renderer/core/testing/sim/sim_request.h"
+#include "third_party/blink/renderer/core/testing/sim/sim_test.h"
+#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
+
+namespace blink {
+
+class FontDisplayAutoLCPAlignTest : public SimTest {
+ public:
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(
+        features::kAlignFontDisplayAutoTimeoutWithLCPGoal);
+    SimTest::SetUp();
+  }
+
+  static Vector<char> ReadAhemWoff2() {
+    return test::ReadFromFile(test::CoreTestDataPath("Ahem.woff2"))
+        ->CopyAs<Vector<char>>();
+  }
+
+ protected:
+  Element* GetTarget() { return GetDocument().getElementById("target"); }
+
+  const Font& GetTargetFont() {
+    return GetTarget()->GetLayoutObject()->Style()->GetFont();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+TEST_F(FontDisplayAutoLCPAlignTest, FontFinishesBeforeLCPLimit) {
+  SimRequest main_resource("https://example.com", "text/html");
+  SimRequest font_resource("https://example.com/Ahem.woff2", "font/woff2");
+
+  LoadURL("https://example.com");
+  main_resource.Complete(R"HTML(
+    <!doctype html>
+    <style>
+      @font-face {
+        font-family: custom-font;
+        src: url(https://example.com/Ahem.woff2) format("woff2");
+      }
+      #target {
+        font: 25px/1 custom-font, monospace;
+      }
+    </style>
+    <span id=target style="position:relative">0123456789</span>
+  )HTML");
+
+  // The first frame is rendered with invisible fallback, as the web font is
+  // still loading, and is in the block display period.
+  Compositor().BeginFrame();
+  EXPECT_GT(250, GetTarget()->OffsetWidth());
+  EXPECT_TRUE(GetTargetFont().ShouldSkipDrawing());
+
+  font_resource.Complete(ReadAhemWoff2());
+
+  // The next frame is rendered with the web font.
+  Compositor().BeginFrame();
+  EXPECT_EQ(250, GetTarget()->OffsetWidth());
+  EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
+}
+
+TEST_F(FontDisplayAutoLCPAlignTest, FontFinishesAfterLCPLimit) {
+  SimRequest main_resource("https://example.com", "text/html");
+  SimRequest font_resource("https://example.com/Ahem.woff2", "font/woff2");
+
+  LoadURL("https://example.com");
+  main_resource.Complete(R"HTML(
+    <!doctype html>
+    <style>
+      @font-face {
+        font-family: custom-font;
+        src: url(https://example.com/Ahem.woff2) format("woff2");
+      }
+      #target {
+        font: 25px/1 custom-font, monospace;
+      }
+    </style>
+    <span id=target style="position:relative">0123456789</span>
+  )HTML");
+
+  // The first frame is rendered with invisible fallback, as the web font is
+  // still loading, and is in the block display period.
+  Compositor().BeginFrame();
+  EXPECT_GT(250, GetTarget()->OffsetWidth());
+  EXPECT_TRUE(GetTargetFont().ShouldSkipDrawing());
+
+  // Wait until we reach the LCP limit, and the relevant timeout fires.
+  test::RunDelayedTasks(base::TimeDelta::FromMilliseconds(
+      features::kAlignFontDisplayAutoTimeoutWithLCPGoalParam.Get()));
+
+  // After reaching the LCP limit, the web font should enter the failure
+  // display period. We should render visible fallback for it.
+  Compositor().BeginFrame();
+  EXPECT_GT(250, GetTarget()->OffsetWidth());
+  EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
+
+  font_resource.Complete(ReadAhemWoff2());
+
+  // We shouldn't use the web font even if it loads now. It's already in the
+  // failure display period.
+  Compositor().BeginFrame();
+  EXPECT_GT(250, GetTarget()->OffsetWidth());
+  EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
+}
+
+TEST_F(FontDisplayAutoLCPAlignTest, FontFaceAddedAfterLCPLimit) {
+  SimRequest main_resource("https://example.com", "text/html");
+  SimRequest font_resource("https://example.com/Ahem.woff2", "font/woff2");
+
+  LoadURL("https://example.com");
+  main_resource.Write("<!doctype html>");
+
+  // Wait until we reach the LCP limit, and the relevant timeout fires.
+  test::RunDelayedTasks(base::TimeDelta::FromMilliseconds(
+      features::kAlignFontDisplayAutoTimeoutWithLCPGoalParam.Get()));
+
+  main_resource.Complete(R"HTML(
+    <style>
+      @font-face {
+        font-family: custom-font;
+        src: url(https://example.com/Ahem.woff2) format("woff2");
+      }
+      #target {
+        font: 25px/1 custom-font, monospace;
+      }
+    </style>
+    <span id=target style="position:relative">0123456789</span>
+  )HTML");
+
+  font_resource.Complete(ReadAhemWoff2());
+
+  // Since the font face is added after the LCP limit and is not in the memory
+  // cache, we'll treated as already in the failure period to prevent any
+  // latency or layout shifting. We should rendering visible fallback for it.
+  Compositor().BeginFrame();
+  EXPECT_GT(250, GetTarget()->OffsetWidth());
+  EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
+}
+
+TEST_F(FontDisplayAutoLCPAlignTest, FontFaceInMemoryCacheAddedAfterLCPLimit) {
+  SimRequest main_resource("https://example.com", "text/html");
+  SimRequest font_resource("https://example.com/Ahem.woff2", "font/woff2");
+
+  LoadURL("https://example.com");
+  main_resource.Write(R"HTML(
+    <!doctype html>
+    <link rel="preload" as="font" type="font/woff2"
+          href="https://example.com/Ahem.woff2" crossorigin>
+  )HTML");
+
+  font_resource.Complete(ReadAhemWoff2());
+
+  // Wait until we reach the LCP limit, and the relevant timeout fires.
+  test::RunDelayedTasks(base::TimeDelta::FromMilliseconds(
+      features::kAlignFontDisplayAutoTimeoutWithLCPGoalParam.Get()));
+
+  main_resource.Complete(R"HTML(
+    <style>
+      @font-face {
+        font-family: custom-font;
+        src: url(https://example.com/Ahem.woff2) format("woff2");
+      }
+      #target {
+        font: 25px/1 custom-font, monospace;
+      }
+    </style>
+    <span id=target style="position:relative">0123456789</span>
+  )HTML");
+
+  // The font face is added after the LCP limit, but it's already preloaded and
+  // available from the memory cache. We'll render with it as it's immediate
+  // available.
+  Compositor().BeginFrame();
+  EXPECT_EQ(250, GetTarget()->OffsetWidth());
+  EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
+}
+
+}  // namespace blink

third_party/blink/renderer/core/css/font_face_set_document.cc

@@ -26,7 +26,9 @@
 #include "third_party/blink/renderer/core/css/font_face_set_document.h"
 
 #include "base/metrics/histogram_functions.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/renderer/bindings/core/v8/dictionary.h"
+#include "third_party/blink/renderer/core/css/css_font_face.h"
 #include "third_party/blink/renderer/core/css/css_font_selector.h"
 #include "third_party/blink/renderer/core/css/css_property_value_set.h"
 #include "third_party/blink/renderer/core/css/css_segmented_font_face.h"
@@ -41,6 +43,7 @@
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
+#include "third_party/blink/renderer/core/loader/document_loader.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
@@ -52,7 +55,10 @@ const char FontFaceSetDocument::kSupplementName[] = "FontFaceSetDocument";
 
 FontFaceSetDocument::FontFaceSetDocument(Document& document)
     : FontFaceSet(*document.GetExecutionContext()),
-      Supplement<Document>(document) {}
+      Supplement<Document>(document),
+      lcp_limit_timer_(document.GetTaskRunner(TaskType::kInternalLoading),
+                       this,
+                       &FontFaceSetDocument::LCPLimitReached) {}
 
 FontFaceSetDocument::~FontFaceSetDocument() = default;
 
@@ -76,8 +82,23 @@ void FontFaceSetDocument::DidLayout() {
   HandlePendingEventsAndPromisesSoon();
 }
 
+void FontFaceSetDocument::StartLCPLimitTimerIfNeeded() {
+  // Make sure the timer is started at most once for each document, and only
+  // when the feature is enabled
+  if (!base::FeatureList::IsEnabled(
+          features::kAlignFontDisplayAutoTimeoutWithLCPGoal) ||
+      has_reached_lcp_limit_ || lcp_limit_timer_.IsActive() ||
+      !GetDocument()->Loader()) {
+    return;
+  }
+
+  lcp_limit_timer_.StartOneShot(
+      GetDocument()->Loader()->RemainingTimeToLCPLimit(), FROM_HERE);
+}
+
 void FontFaceSetDocument::BeginFontLoading(FontFace* font_face) {
   AddToLoadingFonts(font_face);
+  StartLCPLimitTimerIfNeeded();
 }
 
 void FontFaceSetDocument::NotifyLoaded(FontFace* font_face) {
@@ -211,6 +232,18 @@ size_t FontFaceSetDocument::ApproximateBlankCharacterCount(Document& document) {
   return 0;
 }
 
+void FontFaceSetDocument::LCPLimitReached(TimerBase*) {
+  DCHECK(base::FeatureList::IsEnabled(
+      features::kAlignFontDisplayAutoTimeoutWithLCPGoal));
+  if (!GetDocument()->IsActive())
+    return;
+  has_reached_lcp_limit_ = true;
+  for (FontFace* font_face : CSSConnectedFontFaceList())
+    font_face->CssFontFace()->UpdatePeriod();
+  for (FontFace* font_face : non_css_connected_faces_)
+    font_face->CssFontFace()->UpdatePeriod();
+}
+
 void FontFaceSetDocument::Trace(Visitor* visitor) {
   Supplement<Document>::Trace(visitor);
   FontFaceSet::Trace(visitor);

third_party/blink/renderer/core/css/font_face_set_document.h

@@ -64,6 +64,13 @@ class CORE_EXPORT FontFaceSetDocument final : public FontFaceSet,
   void NotifyLoaded(FontFace*) override;
   void NotifyError(FontFace*) override;
 
+  // After flipping the flag to true, all 'font-display: auto' fonts that
+  // haven't finished loading will enter the failure period immediately (except
+  // for those already in the memory cache), so that they don't cause a bad
+  // Largest Contentful Paint (https://wicg.github.io/largest-contentful-paint/)
+  bool HasReachedLCPLimit() const { return has_reached_lcp_limit_; }
+  void LCPLimitReached(TimerBase*);
+
   size_t ApproximateBlankCharacterCount() const;
 
   static FontFaceSetDocument* From(Document&);
@@ -87,6 +94,8 @@ class CORE_EXPORT FontFaceSetDocument final : public FontFaceSet,
   const HeapLinkedHashSet<Member<FontFace>>& CSSConnectedFontFaceList()
       const override;
 
+  void StartLCPLimitTimerIfNeeded();
+
   class FontLoadHistogram {
     DISALLOW_NEW();
 
@@ -100,6 +109,11 @@ class CORE_EXPORT FontFaceSetDocument final : public FontFaceSet,
     Status status_;
   };
   FontLoadHistogram histogram_;
+
+  TaskRunnerTimer<FontFaceSetDocument> lcp_limit_timer_;
+
+  bool has_reached_lcp_limit_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(FontFaceSetDocument);
 };
 

third_party/blink/renderer/core/css/remote_font_face_source.cc

@@ -11,6 +11,7 @@
 #include "third_party/blink/public/platform/web_effective_connection_type.h"
 #include "third_party/blink/renderer/core/css/css_custom_font_data.h"
 #include "third_party/blink/renderer/core/css/css_font_face.h"
+#include "third_party/blink/renderer/core/css/font_face_set_document.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_frame_client.h"
@@ -33,6 +34,21 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod()
     const {
   switch (display_) {
     case kFontDisplayAuto:
+      if (base::FeatureList::IsEnabled(
+              features::kAlignFontDisplayAutoTimeoutWithLCPGoal)) {
+        // If a 'display: auto' font hasn't finished loading by the LCP limit,
+        // it should enter the failure period immediately, so that it doesn't
+        // become a source of bad LCP. The only exception is when the font is
+        // immediately available from the memory cache, in which case it can be
+        // used right away without any latency.
+        if (GetDocument() &&
+            FontFaceSetDocument::From(*GetDocument())->HasReachedLCPLimit()) {
+          if (!IsLoaded() ||
+              (!FinishedFromMemoryCache() && !finished_before_lcp_limit_)) {
+            return kFailurePeriod;
+          }
+        }
+      }
       if (is_intervention_triggered_)
         return kSwapPeriod;
       FALLTHROUGH;
@@ -106,7 +122,8 @@ RemoteFontFaceSource::RemoteFontFaceSource(CSSFontFace* css_font_face,
                                                ReportOptions::kDoNotReport)),
       phase_(kNoLimitExceeded),
       is_intervention_triggered_(ShouldTriggerWebFontsIntervention()),
-      finished_before_document_rendering_begin_(false) {
+      finished_before_document_rendering_begin_(false),
+      finished_before_lcp_limit_(false) {
   DCHECK(face_);
   period_ = ComputePeriod();
 }
@@ -171,9 +188,11 @@ void RemoteFontFaceSource::NotifyFinished(Resource* resource) {
 
   PruneTable();
 
-  if (GetDocument() &&
-      !GetDocument()->GetFontPreloadManager().RenderingHasBegun()) {
-    finished_before_document_rendering_begin_ = true;
+  if (GetDocument()) {
+    if (!GetDocument()->GetFontPreloadManager().RenderingHasBegun())
+      finished_before_document_rendering_begin_ = true;
+    if (!FontFaceSetDocument::From(*GetDocument())->HasReachedLCPLimit())
+      finished_before_lcp_limit_ = true;
   }
 
   if (FinishedFromMemoryCache())

third_party/blink/renderer/core/css/remote_font_face_source.h

@@ -127,7 +127,7 @@ class RemoteFontFaceSource final : public CSSFontFaceSource,
   Document* GetDocument() const;
 
   DisplayPeriod ComputePeriod() const;
-  void UpdatePeriod();
+  void UpdatePeriod() override;
   bool ShouldTriggerWebFontsIntervention();
   bool IsLowPriorityLoadingAllowedForRemoteFont() const override;
   FontDisplay GetFontDisplayWithFeaturePolicyCheck(FontDisplay,
@@ -147,6 +147,7 @@ class RemoteFontFaceSource final : public CSSFontFaceSource,
   FontLoadHistograms histograms_;
   bool is_intervention_triggered_;
   bool finished_before_document_rendering_begin_;
+  bool finished_before_lcp_limit_;
 };
 
 }  // namespace blink

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

@@ -36,6 +36,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/time/default_tick_clock.h"
 #include "services/network/public/cpp/features.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/origin_policy/origin_policy.h"
 #include "third_party/blink/public/mojom/commit_result/commit_result.mojom-blink.h"
 #include "third_party/blink/public/platform/modules/service_worker/web_service_worker_network_provider.h"
@@ -1922,6 +1923,19 @@ DocumentLoader::GetPrefetchedSignedExchangeManager() const {
   return prefetched_signed_exchange_manager_;
 }
 
+base::TimeDelta DocumentLoader::RemainingTimeToLCPLimit() const {
+  // We shouldn't call this function before navigation start
+  DCHECK(!document_load_timing_.NavigationStart().is_null());
+  base::TimeTicks lcp_limit =
+      document_load_timing_.NavigationStart() +
+      base::TimeDelta::FromMilliseconds(
+          features::kAlignFontDisplayAutoTimeoutWithLCPGoalParam.Get());
+  base::TimeTicks now = clock_->NowTicks();
+  if (now < lcp_limit)
+    return lcp_limit - now;
+  return base::TimeDelta();
+}
+
 DEFINE_WEAK_IDENTIFIER_MAP(DocumentLoader)
 
 }  // namespace blink

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

@@ -324,6 +324,11 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
 
   bool NavigationScrollAllowed() const { return navigation_scroll_allowed_; }
 
+  // We want to make sure that the largest content is painted before the "LCP
+  // limit", so that we get a good LCP value. This returns the remaining time to
+  // the LCP limit. See crbug.com/1065508 for details.
+  base::TimeDelta RemainingTimeToLCPLimit() const;
+
  protected:
   Vector<KURL> redirect_chain_;