Skip to content

GitLab

T
tangled
Commit d338750b authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot
Browse files
Options

Allow using an 'optional' font as long as it loads before first use 

After launching crbug.com/1040632, we disallow a 'font-display:optional'
font from being used if it doesn't load before the first rendering cycle
begins, so that it won't cause any relayout.

This turns out to be too stringent. This patches allows using the font
as long as we have never rendered fallback for it. In this way, we still
guarantee no layout shifting, and the font can be used in more
circumstances.

Bug: 1114314
Change-Id: Id521e97b8882e2e3d1fb13c09e040c8e2fd3d47e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343643
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: default avatarDominik Röttsches <drott@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797076}
parent 2c02957d
Hide whitespace changes
Inline Side-by-side
Showing with 59 additions and 4 deletions
third_party/blink/renderer/core/css/remote_font_face_source.cc
View file @ d338750b
...@@ -124,7 +124,8 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod() ...@@ -124,7 +124,8 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod()
if (GetDocument()->GetFontPreloadManager().RenderingHasBegun()) { if (GetDocument()->GetFontPreloadManager().RenderingHasBegun()) {
if (FinishedFromMemoryCache() || if (FinishedFromMemoryCache() ||
finished_before_document_rendering_begin_) finished_before_document_rendering_begin_ ||
!has_been_requested_while_pending_)
return kSwapPeriod; return kSwapPeriod;
return kFailurePeriod; return kFailurePeriod;
} }
...@@ -152,6 +153,7 @@ RemoteFontFaceSource::RemoteFontFaceSource(CSSFontFace* css_font_face, ...@@ -152,6 +153,7 @@ RemoteFontFaceSource::RemoteFontFaceSource(CSSFontFace* css_font_face,
phase_(kNoLimitExceeded), phase_(kNoLimitExceeded),
is_intervention_triggered_(ShouldTriggerWebFontsIntervention()), is_intervention_triggered_(ShouldTriggerWebFontsIntervention()),
finished_before_document_rendering_begin_(false), finished_before_document_rendering_begin_(false),
has_been_requested_while_pending_(false),
finished_before_lcp_limit_(false) { finished_before_lcp_limit_(false) {
DCHECK(face_); DCHECK(face_);
period_ = ComputePeriod(); period_ = ComputePeriod();
...@@ -357,6 +359,7 @@ RemoteFontFaceSource::CreateLoadingFallbackFontData( ...@@ -357,6 +359,7 @@ RemoteFontFaceSource::CreateLoadingFallbackFontData(
scoped_refptr<CSSCustomFontData> css_font_data = CSSCustomFontData::Create( scoped_refptr<CSSCustomFontData> css_font_data = CSSCustomFontData::Create(
this, period_ == kBlockPeriod ? CSSCustomFontData::kInvisibleFallback this, period_ == kBlockPeriod ? CSSCustomFontData::kInvisibleFallback
: CSSCustomFontData::kVisibleFallback); : CSSCustomFontData::kVisibleFallback);
has_been_requested_while_pending_ = true;
return SimpleFontData::Create(temporary_font->PlatformData(), css_font_data); return SimpleFontData::Create(temporary_font->PlatformData(), css_font_data);
} }
......
third_party/blink/renderer/core/css/remote_font_face_source.h
View file @ d338750b
...@@ -157,6 +157,14 @@ class RemoteFontFaceSource final : public CSSFontFaceSource, ...@@ -157,6 +157,14 @@ class RemoteFontFaceSource final : public CSSFontFaceSource,
FontLoadHistograms histograms_; FontLoadHistograms histograms_;
bool is_intervention_triggered_; bool is_intervention_triggered_;
bool finished_before_document_rendering_begin_; bool finished_before_document_rendering_begin_;
// Indicates whether FontData has been requested while the font is still being
// loaded, in which case a fallback FontData is returned and used. If true, we
// will render contents with fallback font, and later if we would switch to
// the web font after it loads, there will be a layout shifting. Therefore, we
// don't need to worry about layout shifting when it's false.
bool has_been_requested_while_pending_;
bool finished_before_lcp_limit_; bool finished_before_lcp_limit_;
}; };
......
third_party/blink/renderer/core/loader/font_preload_manager_test.cc
View file @ d338750b
...@@ -235,7 +235,7 @@ TEST_F(FontPreloadManagerTest, OptionalFontWithoutPreloading) { ...@@ -235,7 +235,7 @@ TEST_F(FontPreloadManagerTest, OptionalFontWithoutPreloading) {
"font/woff2"); "font/woff2");
LoadURL("https://example.com"); LoadURL("https://example.com");
main_resource.Complete(R"HTML( main_resource.Write(R"HTML(
<!doctype html> <!doctype html>
<style> <style>
@font-face { @font-face {
...@@ -248,6 +248,7 @@ TEST_F(FontPreloadManagerTest, OptionalFontWithoutPreloading) { ...@@ -248,6 +248,7 @@ TEST_F(FontPreloadManagerTest, OptionalFontWithoutPreloading) {
} }
</style> </style>
<span id=target>0123456789</span> <span id=target>0123456789</span>
<script>document.fonts.load('25px/1 custom-font');</script>
)HTML"); )HTML");
// Now rendering has started, as there's no blocking resources. // Now rendering has started, as there's no blocking resources.
...@@ -256,11 +257,54 @@ TEST_F(FontPreloadManagerTest, OptionalFontWithoutPreloading) { ...@@ -256,11 +257,54 @@ TEST_F(FontPreloadManagerTest, OptionalFontWithoutPreloading) {
font_resource.Complete(ReadAhemWoff2()); font_resource.Complete(ReadAhemWoff2());
// The 'optional' web font isn't used, as it didn't finish loading before // Although the optional web font isn't preloaded, it finished loading before
// rendering started. Text is rendered in visible fallback. // the first time we try to render with it. Therefore it's used.
Compositor().BeginFrame().Contains(SimCanvas::kText); Compositor().BeginFrame().Contains(SimCanvas::kText);
EXPECT_EQ(250, GetTarget()->OffsetWidth());
EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
main_resource.Finish();
}
TEST_F(FontPreloadManagerTest, OptionalFontMissingFirstFrame) {
SimRequest main_resource("https://example.com", "text/html");
SimSubresourceRequest font_resource("https://example.com/Ahem.woff2",
"font/woff2");
LoadURL("https://example.com");
main_resource.Write(R"HTML(
<!doctype html>
<style>
@font-face {
font-family: custom-font;
src: url(https://example.com/Ahem.woff2) format("woff2");
font-display: optional;
}
#target {
font: 25px/1 custom-font, monospace;
}
</style>
<span id=target>0123456789</span>
)HTML");
// Now rendering has started, as there's no blocking resources.
EXPECT_FALSE(Compositor().DeferMainFrameUpdate());
EXPECT_EQ(State::kUnblocked, GetState());
// We render visible fallback as the 'optional' web font hasn't loaded.
Compositor().BeginFrame();
EXPECT_GT(250, GetTarget()->OffsetWidth());
EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
font_resource.Complete(ReadAhemWoff2());
// Since we have rendered fallback for the 'optional' font, even after it
// finishes loading, we shouldn't use it, as otherwise there's a relayout.
Compositor().BeginFrame();
EXPECT_GT(250, GetTarget()->OffsetWidth()); EXPECT_GT(250, GetTarget()->OffsetWidth());
EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing()); EXPECT_FALSE(GetTargetFont().ShouldSkipDrawing());
main_resource.Finish();
} }
TEST_F(FontPreloadManagerTest, OptionalFontRemoveAndReadd) { TEST_F(FontPreloadManagerTest, OptionalFontRemoveAndReadd) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment