Title: [250983] trunk
- Revision
- 250983
- Author
- [email protected]
- Date
- 2019-10-10 12:42:15 -0700 (Thu, 10 Oct 2019)
Log Message
FontFaceSet's ready promise is not always resolved
https://bugs.webkit.org/show_bug.cgi?id=202548
Reviewed by Youenn Fablet.
Source/WebCore:
When we do layout on an element, FontRanges::glyphDataForCharacter() will cause the first
available font to start loading, but will continue looking at subsequent fonts to see if
there's a good one we can render while the load is happening. When looking for a fallback
font, it calls FontRanges::Range::font() with a ExternalResourceDownloadPolicy set to
Forbid. This is fine, except that a side effect of calling this function is that the
CSSFontFace marks itself as Loading, which means document.fonts.ready is deferred. Then,
the load finishes, and the subsequent CSSFontFace is never actually used, meaning it never
exits the Loading state, which means document.fonts.ready never fires.
The solution to this is to just only allow the font to enter the Loading state if it's not
one of these "subsequent" fonts.
Test: fast/text/fontfaceset-ready-not-fired.html
* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::pump):
LayoutTests:
* fast/text/fontfaceset-ready-not-fired-expected.txt: Added.
* fast/text/fontfaceset-ready-not-fired.html: Added.
* fast/text/fontfaceset-ready-not-fired-2-expected.txt: Added.
* fast/text/fontfaceset-ready-not-fired-2.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (250982 => 250983)
--- trunk/LayoutTests/ChangeLog 2019-10-10 19:40:50 UTC (rev 250982)
+++ trunk/LayoutTests/ChangeLog 2019-10-10 19:42:15 UTC (rev 250983)
@@ -1,3 +1,15 @@
+2019-10-10 Myles C. Maxfield <[email protected]>
+
+ FontFaceSet's ready promise is not always resolved
+ https://bugs.webkit.org/show_bug.cgi?id=202548
+
+ Reviewed by Youenn Fablet.
+
+ * fast/text/fontfaceset-ready-not-fired-expected.txt: Added.
+ * fast/text/fontfaceset-ready-not-fired.html: Added.
+ * fast/text/fontfaceset-ready-not-fired-2-expected.txt: Added.
+ * fast/text/fontfaceset-ready-not-fired-2.html: Added.
+
2019-10-10 Wenson Hsieh <[email protected]>
pointerevents/ios/touch-action-pinch-zoom-allows-zooming.html is failing after r250361
Added: trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-2-expected.txt (0 => 250983)
--- trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-2-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-2-expected.txt 2019-10-10 19:42:15 UTC (rev 250983)
@@ -0,0 +1,9 @@
+This test makes sure the ready promise is fired when a local font is loaded successfully with a policy of ExternalResourceDownloadPolicy::Forbid.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Hello
Added: trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-2.html (0 => 250983)
--- trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-2.html (rev 0)
+++ trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-2.html 2019-10-10 19:42:15 UTC (rev 250983)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "WebFont";
+ font-weight: 400;
+ src: url("../../resources/Ahem.otf");
+}
+@font-face {
+ font-family: "WebFont";
+ font-weight: 500;
+ src: local("Helvetica");
+}
+</style>
+<script src=""
+</head>
+<body>
+<div id="target" style="font: 48px 'WebFont';">Hello</div>
+<script>
+window.jsTestIsAsync = true;
+description("This test makes sure the ready promise is fired when a local font is loaded successfully with a policy of ExternalResourceDownloadPolicy::Forbid.");
+let target = document.getElementById("target");
+target.offsetWidth;
+document.fonts.ready.then(function() {
+ finishJSTest();
+});
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-expected.txt (0 => 250983)
--- trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired-expected.txt 2019-10-10 19:42:15 UTC (rev 250983)
@@ -0,0 +1,9 @@
+This test makes sure the ready promise is resolved even when we interrogate an @font-face block to see if it can be used as an interstitial font.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Hello
Added: trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired.html (0 => 250983)
--- trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired.html (rev 0)
+++ trunk/LayoutTests/fast/text/fontfaceset-ready-not-fired.html 2019-10-10 19:42:15 UTC (rev 250983)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "WebFont";
+ font-weight: 400;
+ src: url("../../resources/Ahem.otf");
+}
+@font-face {
+ font-family: "WebFont";
+ font-weight: 500;
+ src: local("asdfasdf"), url("../../resources/Ahem.ttf");
+}
+</style>
+<script src=""
+</head>
+<body>
+<div id="target" style="font: 48px 'WebFont', 'Helvetica';">Hello</div>
+<script>
+window.jsTestIsAsync = true;
+description("This test makes sure the ready promise is resolved even when we interrogate an @font-face block to see if it can be used as an interstitial font.");
+let target = document.getElementById("target");
+target.offsetWidth;
+document.fonts.ready.then(function() {
+ finishJSTest();
+});
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (250982 => 250983)
--- trunk/Source/WebCore/ChangeLog 2019-10-10 19:40:50 UTC (rev 250982)
+++ trunk/Source/WebCore/ChangeLog 2019-10-10 19:42:15 UTC (rev 250983)
@@ -1,3 +1,27 @@
+2019-10-10 Myles C. Maxfield <[email protected]>
+
+ FontFaceSet's ready promise is not always resolved
+ https://bugs.webkit.org/show_bug.cgi?id=202548
+
+ Reviewed by Youenn Fablet.
+
+ When we do layout on an element, FontRanges::glyphDataForCharacter() will cause the first
+ available font to start loading, but will continue looking at subsequent fonts to see if
+ there's a good one we can render while the load is happening. When looking for a fallback
+ font, it calls FontRanges::Range::font() with a ExternalResourceDownloadPolicy set to
+ Forbid. This is fine, except that a side effect of calling this function is that the
+ CSSFontFace marks itself as Loading, which means document.fonts.ready is deferred. Then,
+ the load finishes, and the subsequent CSSFontFace is never actually used, meaning it never
+ exits the Loading state, which means document.fonts.ready never fires.
+
+ The solution to this is to just only allow the font to enter the Loading state if it's not
+ one of these "subsequent" fonts.
+
+ Test: fast/text/fontfaceset-ready-not-fired.html
+
+ * css/CSSFontFace.cpp:
+ (WebCore::CSSFontFace::pump):
+
2019-10-10 youenn fablet <[email protected]>
MockRealtimeVideoSource::setFrameRateWithPreset should not use preset after moving it
Modified: trunk/Source/WebCore/css/CSSFontFace.cpp (250982 => 250983)
--- trunk/Source/WebCore/css/CSSFontFace.cpp 2019-10-10 19:40:50 UTC (rev 250982)
+++ trunk/Source/WebCore/css/CSSFontFace.cpp 2019-10-10 19:42:15 UTC (rev 250983)
@@ -737,7 +737,7 @@
// but it seems that this behavior is a requirement of the design of FontRanges. FIXME: Perhaps rethink
// this design.
if (policy == ExternalResourceDownloadPolicy::Allow || !source->requiresExternalResource()) {
- if (m_status == Status::Pending)
+ if (policy == ExternalResourceDownloadPolicy::Allow && m_status == Status::Pending)
setStatus(Status::Loading);
source->load(m_fontSelector.get());
}
@@ -749,7 +749,7 @@
return i;
case CSSFontFaceSource::Status::Loading:
ASSERT(m_status == Status::Pending || m_status == Status::Loading || m_status == Status::TimedOut || m_status == Status::Failure);
- if (m_status == Status::Pending)
+ if (policy == ExternalResourceDownloadPolicy::Allow && m_status == Status::Pending)
setStatus(Status::Loading);
return i;
case CSSFontFaceSource::Status::Success:
@@ -760,7 +760,7 @@
setStatus(Status::Success);
return i;
case CSSFontFaceSource::Status::Failure:
- if (m_status == Status::Pending)
+ if (policy == ExternalResourceDownloadPolicy::Allow && m_status == Status::Pending)
setStatus(Status::Loading);
break;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
