- Revision
- 216079
- Author
- mmaxfi...@apple.com
- Date
- 2017-05-02 11:02:50 -0700 (Tue, 02 May 2017)
Log Message
Font Loading API specifies font is loaded but sizing of font after load reports inconsistent values
https://bugs.webkit.org/show_bug.cgi?id=168533
Reviewed by Zalan Bujtas.
Source/WebCore:
Previously, we were marking all local() fonts as immediately successful,
regardless of whether or not they were present on the system. Instead, we
should use the load() function to make this determination and mark the font
as failed if it doesn't exist. (This is, after all, the whole point of the
load() function). This brings us in-line with Firefox's and Chrome's
behavior.
Test: fast/text/font-loading-local.html
* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::pump): Remote loading requires the FontSelector,
but it isn't available for local fonts. Now that load() is called for both
local and remote fonts, the ASSERT() should be lowered into the load()
function and scoped to just the case where we have a remote font.
(WebCore::CSSFontFace::font): Ditto.
* css/CSSFontFaceSource.cpp:
(WebCore::CSSFontFaceSource::CSSFontFaceSource): Don't immediatley set
the success/failure state for local fonts.
(WebCore::CSSFontFaceSource::load): Move loading logic from font() to
load(). None of this code is new; it just is moved.
(WebCore::CSSFontFaceSource::font): Delete code moved to load().
* css/CSSFontFaceSource.h:
* css/FontFace.cpp:
(WebCore::FontFace::create):
LayoutTests:
* fast/text/font-loading-local-expected.txt: Added.
* fast/text/font-loading-local.html: Added.
* fast/text/web-font-load-fallback-during-loading.html:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (216078 => 216079)
--- trunk/LayoutTests/ChangeLog 2017-05-02 18:00:23 UTC (rev 216078)
+++ trunk/LayoutTests/ChangeLog 2017-05-02 18:02:50 UTC (rev 216079)
@@ -1,3 +1,14 @@
+2017-05-02 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Font Loading API specifies font is loaded but sizing of font after load reports inconsistent values
+ https://bugs.webkit.org/show_bug.cgi?id=168533
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/text/font-loading-local-expected.txt: Added.
+ * fast/text/font-loading-local.html: Added.
+ * fast/text/web-font-load-fallback-during-loading.html:
+
2017-05-02 Youenn Fablet <you...@apple.com>
Allow media stream based videos with sound autoplay if the page is already playing sound
Added: trunk/LayoutTests/fast/text/font-loading-local-expected.txt (0 => 216079)
--- trunk/LayoutTests/fast/text/font-loading-local-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/text/font-loading-local-expected.txt 2017-05-02 18:02:50 UTC (rev 216079)
@@ -0,0 +1,11 @@
+This test makes sure that the CSS Font Loading API works properly with local() fonts.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Local font loaded correctly
+PASS Garbage local font didn't load
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/text/font-loading-local.html (0 => 216079)
--- trunk/LayoutTests/fast/text/font-loading-local.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-loading-local.html 2017-05-02 18:02:50 UTC (rev 216079)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description("This test makes sure that the CSS Font Loading API works properly with local() fonts.");
+window.jsTestIsAsync = true;
+var fontFace = new FontFace("WebFont", "local('Helvetica')");
+fontFace.load().then(function() {
+ testPassed("Local font loaded correctly");
+ fontFace = new FontFace("WebFont", "local('garbage')");
+ return fontFace.load();
+}, function() {
+ testFailed("Should not fail");
+ finishJSTest();
+}).then(function() {
+ testFailed("Should not succeed");
+ finishJSTest();
+}, function() {
+ testPassed("Garbage local font didn't load");
+ finishJSTest();
+});
+</script>
+<script src=""
+</body>
+</html>
\ No newline at end of file
Modified: trunk/LayoutTests/fast/text/web-font-load-fallback-during-loading.html (216078 => 216079)
--- trunk/LayoutTests/fast/text/web-font-load-fallback-during-loading.html 2017-05-02 18:00:23 UTC (rev 216078)
+++ trunk/LayoutTests/fast/text/web-font-load-fallback-during-loading.html 2017-05-02 18:02:50 UTC (rev 216079)
@@ -53,6 +53,8 @@
</head>
<body>
This test makes sure that fallback fonts are used during the time when fonts are loading. The test passes if the next line below reads "Test complete" and all the subsequent statements below are legible and true.
+<!-- FIXME: This test is racey. The font may complete downloading before the 0-delay timer fires,
+in which case the "Test Complete" text will not be added. -->
<p id="console"></p>
<p><span id="reference" style="font-family: Helvetica">This is rendered with Helvetica.</span></p>
<p><span class="test" style="font-family: WebFont">This is rendered with Helvetica.</span></p>
Modified: trunk/Source/WebCore/ChangeLog (216078 => 216079)
--- trunk/Source/WebCore/ChangeLog 2017-05-02 18:00:23 UTC (rev 216078)
+++ trunk/Source/WebCore/ChangeLog 2017-05-02 18:02:50 UTC (rev 216079)
@@ -1,3 +1,35 @@
+2017-05-02 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Font Loading API specifies font is loaded but sizing of font after load reports inconsistent values
+ https://bugs.webkit.org/show_bug.cgi?id=168533
+
+ Reviewed by Zalan Bujtas.
+
+ Previously, we were marking all local() fonts as immediately successful,
+ regardless of whether or not they were present on the system. Instead, we
+ should use the load() function to make this determination and mark the font
+ as failed if it doesn't exist. (This is, after all, the whole point of the
+ load() function). This brings us in-line with Firefox's and Chrome's
+ behavior.
+
+ Test: fast/text/font-loading-local.html
+
+ * css/CSSFontFace.cpp:
+ (WebCore::CSSFontFace::pump): Remote loading requires the FontSelector,
+ but it isn't available for local fonts. Now that load() is called for both
+ local and remote fonts, the ASSERT() should be lowered into the load()
+ function and scoped to just the case where we have a remote font.
+ (WebCore::CSSFontFace::font): Ditto.
+ * css/CSSFontFaceSource.cpp:
+ (WebCore::CSSFontFaceSource::CSSFontFaceSource): Don't immediatley set
+ the success/failure state for local fonts.
+ (WebCore::CSSFontFaceSource::load): Move loading logic from font() to
+ load(). None of this code is new; it just is moved.
+ (WebCore::CSSFontFaceSource::font): Delete code moved to load().
+ * css/CSSFontFaceSource.h:
+ * css/FontFace.cpp:
+ (WebCore::FontFace::create):
+
2017-05-02 Youenn Fablet <you...@apple.com>
Allow media stream based videos with sound to autoplay if the page is already playing sound
Modified: trunk/Source/WebCore/css/CSSFontFace.cpp (216078 => 216079)
--- trunk/Source/WebCore/css/CSSFontFace.cpp 2017-05-02 18:00:23 UTC (rev 216078)
+++ trunk/Source/WebCore/css/CSSFontFace.cpp 2017-05-02 18:02:50 UTC (rev 216079)
@@ -558,10 +558,9 @@
if (source->status() == CSSFontFaceSource::Status::Pending) {
ASSERT(m_status == Status::Pending || m_status == Status::Loading || m_status == Status::TimedOut);
- ASSERT(m_fontSelector);
if (m_status == Status::Pending)
setStatus(Status::Loading);
- source->load(*m_fontSelector);
+ source->load(m_fontSelector.get());
}
switch (source->status()) {
@@ -612,10 +611,9 @@
for (size_t i = startIndex; i < m_sources.size(); ++i) {
auto& source = m_sources[i];
if (source->status() == CSSFontFaceSource::Status::Pending) {
- ASSERT(m_fontSelector);
- if (fontIsLoading)
+ if (fontIsLoading && source->requiresExternalResource())
continue;
- source->load(*m_fontSelector);
+ source->load(m_fontSelector.get());
}
switch (source->status()) {
Modified: trunk/Source/WebCore/css/CSSFontFaceSource.cpp (216078 => 216079)
--- trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2017-05-02 18:00:23 UTC (rev 216078)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2017-05-02 18:02:50 UTC (rev 216079)
@@ -89,7 +89,7 @@
if (m_font)
m_font->addClient(*this);
- if (status() == Status::Pending && (!m_font || m_font->isLoaded())) {
+ if (status() == Status::Pending && m_font && m_font->isLoaded()) {
setStatus(Status::Loading);
if (m_font && m_font->errorOccurred())
setStatus(Status::Failure);
@@ -128,12 +128,44 @@
m_face.fontLoaded(*this);
}
-void CSSFontFaceSource::load(CSSFontSelector& fontSelector)
+void CSSFontFaceSource::load(CSSFontSelector* fontSelector)
{
setStatus(Status::Loading);
- ASSERT(m_font);
- fontSelector.beginLoadingFontSoon(*m_font);
+ if (m_font) {
+ ASSERT(fontSelector);
+ fontSelector->beginLoadingFontSoon(*m_font);
+ } else {
+ bool success = false;
+ if (m_svgFontFaceElement) {
+ if (is<SVGFontElement>(m_svgFontFaceElement->parentNode())) {
+ ASSERT(!m_inDocumentCustomPlatformData);
+ SVGFontElement& fontElement = downcast<SVGFontElement>(*m_svgFontFaceElement->parentNode());
+ if (auto otfFont = convertSVGToOTFFont(fontElement))
+ m_generatedOTFBuffer = SharedBuffer::create(WTFMove(otfFont.value()));
+ if (m_generatedOTFBuffer) {
+ m_inDocumentCustomPlatformData = createFontCustomPlatformData(*m_generatedOTFBuffer);
+ success = static_cast<bool>(m_inDocumentCustomPlatformData);
+ }
+ }
+ } else if (m_immediateSource) {
+ ASSERT(!m_immediateFontCustomPlatformData);
+ bool wrapping;
+ RefPtr<SharedBuffer> buffer = SharedBuffer::create(static_cast<const char*>(m_immediateSource->baseAddress()), m_immediateSource->byteLength());
+ ASSERT(buffer);
+ m_immediateFontCustomPlatformData = CachedFont::createCustomFontData(*buffer, wrapping);
+ success = static_cast<bool>(m_immediateFontCustomPlatformData);
+ } else {
+ // We are only interested in whether or not fontForFamily() returns null or not. Luckily, none of
+ // the values in the FontDescription other than the family name can cause the function to return
+ // null if it wasn't going to otherwise (and vice-versa).
+ FontCascadeDescription fontDescription;
+ fontDescription.setOneFamily(m_familyNameOrURI);
+ fontDescription.setComputedSize(1);
+ success = FontCache::singleton().fontForFamily(fontDescription, m_familyNameOrURI, nullptr, nullptr, FontSelectionSpecifiedCapabilities(), true);
+ }
+ setStatus(success ? Status::Success : Status::Failure);
+ }
}
RefPtr<Font> CSSFontFaceSource::font(const FontDescription& fontDescription, bool syntheticBold, bool syntheticItalic, const FontFeatureSettings& fontFaceFeatures, const FontVariantSettings& fontFaceVariantSettings, FontSelectionSpecifiedCapabilities fontFaceCapabilities)
@@ -147,12 +179,7 @@
if (!m_font && !fontFaceElement) {
if (m_immediateSource) {
- if (!m_immediateFontCustomPlatformData) {
- bool wrapping;
- RefPtr<SharedBuffer> buffer = SharedBuffer::create(static_cast<const char*>(m_immediateSource->baseAddress()), m_immediateSource->byteLength());
- ASSERT(buffer);
- m_immediateFontCustomPlatformData = CachedFont::createCustomFontData(*buffer, wrapping);
- } if (!m_immediateFontCustomPlatformData)
+ if (!m_immediateFontCustomPlatformData)
return nullptr;
return Font::create(CachedFont::platformDataFromCustomData(*m_immediateFontCustomPlatformData, fontDescription, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities), true);
}
@@ -176,14 +203,6 @@
#if ENABLE(SVG_FONTS)
if (!is<SVGFontElement>(m_svgFontFaceElement->parentNode()))
return nullptr;
- if (!m_inDocumentCustomPlatformData) {
- SVGFontElement& fontElement = downcast<SVGFontElement>(*m_svgFontFaceElement->parentNode());
- if (auto otfFont = convertSVGToOTFFont(fontElement))
- m_generatedOTFBuffer = SharedBuffer::create(WTFMove(otfFont.value()));
- if (!m_generatedOTFBuffer)
- return nullptr;
- m_inDocumentCustomPlatformData = createFontCustomPlatformData(*m_generatedOTFBuffer);
- }
if (!m_inDocumentCustomPlatformData)
return nullptr;
#if PLATFORM(COCOA)
Modified: trunk/Source/WebCore/css/CSSFontFaceSource.h (216078 => 216079)
--- trunk/Source/WebCore/css/CSSFontFaceSource.h 2017-05-02 18:00:23 UTC (rev 216078)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.h 2017-05-02 18:02:50 UTC (rev 216079)
@@ -66,9 +66,11 @@
const AtomicString& familyNameOrURI() const { return m_familyNameOrURI; }
- void load(CSSFontSelector&);
+ void load(CSSFontSelector*);
RefPtr<Font> font(const FontDescription&, bool syntheticBold, bool syntheticItalic, const FontFeatureSettings&, const FontVariantSettings&, FontSelectionSpecifiedCapabilities);
+ bool requiresExternalResource() const { return m_font; }
+
#if ENABLE(SVG_FONTS)
bool isSVGFontFaceSource() const;
#endif
Modified: trunk/Source/WebCore/css/FontFace.cpp (216078 => 216079)
--- trunk/Source/WebCore/css/FontFace.cpp 2017-05-02 18:00:23 UTC (rev 216078)
+++ trunk/Source/WebCore/css/FontFace.cpp 2017-05-02 18:02:50 UTC (rev 216079)
@@ -107,7 +107,8 @@
if (!dataRequiresAsynchronousLoading) {
result->backing().load();
- ASSERT(result->backing().status() == CSSFontFace::Status::Success);
+ auto status = result->backing().status();
+ ASSERT_UNUSED(status, status == CSSFontFace::Status::Success || status == CSSFontFace::Status::Failure);
}
return WTFMove(result);
