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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@250983 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 76c2b7a..953287f 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2019-10-10 Myles C. Maxfield <mmaxfield@apple.com>
+
+ 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 <wenson_hsieh@apple.com>
pointerevents/ios/touch-action-pinch-zoom-allows-zooming.html is failing after r250361
diff --git a/LayoutTests/fast/text/fontfaceset-ready-not-fired-2-expected.txt b/LayoutTests/fast/text/fontfaceset-ready-not-fired-2-expected.txt
new file mode 100644
index 0000000..48c2350
--- /dev/null
+++ b/LayoutTests/fast/text/fontfaceset-ready-not-fired-2-expected.txt
@@ -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
diff --git a/LayoutTests/fast/text/fontfaceset-ready-not-fired-2.html b/LayoutTests/fast/text/fontfaceset-ready-not-fired-2.html
new file mode 100644
index 0000000..1454d5f
--- /dev/null
+++ b/LayoutTests/fast/text/fontfaceset-ready-not-fired-2.html
@@ -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="../../resources/js-test-pre.js"></script>
+</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="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/text/fontfaceset-ready-not-fired-expected.txt b/LayoutTests/fast/text/fontfaceset-ready-not-fired-expected.txt
new file mode 100644
index 0000000..166747f
--- /dev/null
+++ b/LayoutTests/fast/text/fontfaceset-ready-not-fired-expected.txt
@@ -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
diff --git a/LayoutTests/fast/text/fontfaceset-ready-not-fired.html b/LayoutTests/fast/text/fontfaceset-ready-not-fired.html
new file mode 100644
index 0000000..bc322b3
--- /dev/null
+++ b/LayoutTests/fast/text/fontfaceset-ready-not-fired.html
@@ -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="../../resources/js-test-pre.js"></script>
+</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="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 4b7db5b..a55e099 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2019-10-10 Myles C. Maxfield <mmaxfield@apple.com>
+
+ 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 <youenn@apple.com>
MockRealtimeVideoSource::setFrameRateWithPreset should not use preset after moving it
diff --git a/Source/WebCore/css/CSSFontFace.cpp b/Source/WebCore/css/CSSFontFace.cpp
index 646c14a..8d21b3ab 100644
--- a/Source/WebCore/css/CSSFontFace.cpp
+++ b/Source/WebCore/css/CSSFontFace.cpp
@@ -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;
}
