- Revision
- 218733
- Author
- mmaxfi...@apple.com
- Date
- 2017-06-22 19:17:21 -0700 (Thu, 22 Jun 2017)
Log Message
@font-face rules with invalid primary fonts never download their secondary fonts
https://bugs.webkit.org/show_bug.cgi?id=173138
<rdar://problem/32554450>
Reviewed by Simon Fraser.
Source/WebCore:
We have logic in CSSFontAccessor::font() which disallows downloading a CSSFontFace if that CSSFontFace
is already in the Succeeded state. However, it was possible for a succeeded CSSFontFace to still fail
to create a font. In this situation, we wouldn't be able to use the downloaded font, and we wouldn't
try to download the next item in the src: list because the CSSFontFace is succeeded.
This patch strengthens the meaning of the Succeeded state. Previously, it just meant that the bytes
in the file were downloaded successfully. This patch extends this to also mean that the bytes in the
file can be successfully interpreted as a font. This way, the CSSFontFace in the example above won't be
set to the Succeeded state, so we will continue follow the src: list and download the secondary fonts.
This has an added benefit that the CSS Font Loading API's promises will be called more appropriately.
The transition to the Succeeded state will trigger a resolve of the promise. Now, these promises will
only be resolved if the fonts are actually parsed and understood by our text system.
Test: fast/text/font-fallback-invalid-load.html
* css/CSSFontFaceSource.cpp:
(WebCore::CSSFontFaceSource::fontLoaded): Move to the failed state if we can't understand the font
data. This is the crux of this patch.
(WebCore::CSSFontFaceSource::font): This function should only be called if we are in the Succeeded
state, which means now we know we should always be able to understand the bytes of the file. Therefore,
we can change some if statements into ASSERT()s.
* loader/cache/CachedSVGFont.cpp:
(WebCore::CachedSVGFont::createFont): Ditto.
(WebCore::CachedSVGFont::ensureCustomFontData): Similarly to CSSFontFaceSource::fontLoaded(), this
adds another check to our criteria for transitioning into the Succeeded state, which will guarantee that
later we will always be able to create the font object.
LayoutTests:
* fast/text/font-fallback-invalid-load-expected.html: Added.
* fast/text/font-fallback-invalid-load.html: Added.
* fast/text/resources/bogus.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (218732 => 218733)
--- trunk/LayoutTests/ChangeLog 2017-06-23 02:13:42 UTC (rev 218732)
+++ trunk/LayoutTests/ChangeLog 2017-06-23 02:17:21 UTC (rev 218733)
@@ -1,3 +1,15 @@
+2017-06-22 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ @font-face rules with invalid primary fonts never download their secondary fonts
+ https://bugs.webkit.org/show_bug.cgi?id=173138
+ <rdar://problem/32554450>
+
+ Reviewed by Simon Fraser.
+
+ * fast/text/font-fallback-invalid-load-expected.html: Added.
+ * fast/text/font-fallback-invalid-load.html: Added.
+ * fast/text/resources/bogus.svg: Added.
+
2017-06-22 Youenn Fablet <you...@apple.com>
Add a test for multi data channel peer connection
Added: trunk/LayoutTests/fast/text/font-fallback-invalid-load-expected.html (0 => 218733)
--- trunk/LayoutTests/fast/text/font-fallback-invalid-load-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-fallback-invalid-load-expected.html 2017-06-23 02:17:21 UTC (rev 218733)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that an @font-face with an invalid primary font file is rendered. The test passes if you see something other than this text on the page.
+<div style="font: 48px 'WebFont';">Hello</div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/font-fallback-invalid-load.html (0 => 218733)
--- trunk/LayoutTests/fast/text/font-fallback-invalid-load.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-fallback-invalid-load.html 2017-06-23 02:17:21 UTC (rev 218733)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+ testRunner.waitUntilDone();
+</script>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("resources/bogus.svg") format("svg"), url("../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that an @font-face with an invalid primary font file is rendered. The test passes if you see something other than this text on the page.
+<div style="font: 48px 'WebFont', Helvetica;">Hello</div>
+<div id="error" style="font-size: 48px; color: red;"></div>
+<script>
+// We're waiting for Ahem to be loaded. Unfortunately, the WK API says that the load is complete when the number of concurrent in-flight subresources
+// hits 0, which occurs before we hit our second layout and realize we need to load Ahem. So, without this, the test would complete before Ahem is
+// requested.
+if (window.testRunner) {
+ document.fonts.keys().next().value.loaded.then(function() {
+ testRunner.notifyDone();
+ }, function() {
+ document.getElementById("error").textContent = "Error loading font.";
+ testRunner.notifyDone();
+ });
+}
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/resources/bogus.svg (0 => 218733)
--- trunk/LayoutTests/fast/text/resources/bogus.svg (rev 0)
+++ trunk/LayoutTests/fast/text/resources/bogus.svg 2017-06-23 02:17:21 UTC (rev 218733)
@@ -0,0 +1 @@
+bogus
Modified: trunk/Source/WebCore/ChangeLog (218732 => 218733)
--- trunk/Source/WebCore/ChangeLog 2017-06-23 02:13:42 UTC (rev 218732)
+++ trunk/Source/WebCore/ChangeLog 2017-06-23 02:17:21 UTC (rev 218733)
@@ -1,3 +1,39 @@
+2017-06-22 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ @font-face rules with invalid primary fonts never download their secondary fonts
+ https://bugs.webkit.org/show_bug.cgi?id=173138
+ <rdar://problem/32554450>
+
+ Reviewed by Simon Fraser.
+
+ We have logic in CSSFontAccessor::font() which disallows downloading a CSSFontFace if that CSSFontFace
+ is already in the Succeeded state. However, it was possible for a succeeded CSSFontFace to still fail
+ to create a font. In this situation, we wouldn't be able to use the downloaded font, and we wouldn't
+ try to download the next item in the src: list because the CSSFontFace is succeeded.
+
+ This patch strengthens the meaning of the Succeeded state. Previously, it just meant that the bytes
+ in the file were downloaded successfully. This patch extends this to also mean that the bytes in the
+ file can be successfully interpreted as a font. This way, the CSSFontFace in the example above won't be
+ set to the Succeeded state, so we will continue follow the src: list and download the secondary fonts.
+
+ This has an added benefit that the CSS Font Loading API's promises will be called more appropriately.
+ The transition to the Succeeded state will trigger a resolve of the promise. Now, these promises will
+ only be resolved if the fonts are actually parsed and understood by our text system.
+
+ Test: fast/text/font-fallback-invalid-load.html
+
+ * css/CSSFontFaceSource.cpp:
+ (WebCore::CSSFontFaceSource::fontLoaded): Move to the failed state if we can't understand the font
+ data. This is the crux of this patch.
+ (WebCore::CSSFontFaceSource::font): This function should only be called if we are in the Succeeded
+ state, which means now we know we should always be able to understand the bytes of the file. Therefore,
+ we can change some if statements into ASSERT()s.
+ * loader/cache/CachedSVGFont.cpp:
+ (WebCore::CachedSVGFont::createFont): Ditto.
+ (WebCore::CachedSVGFont::ensureCustomFontData): Similarly to CSSFontFaceSource::fontLoaded(), this
+ adds another check to our criteria for transitioning into the Succeeded state, which will guarantee that
+ later we will always be able to create the font object.
+
2017-06-22 Andreas Kling <akl...@apple.com>
Rename MemoryPressureHandler::setTabCount to setPageCount
Modified: trunk/Source/WebCore/css/CSSFontFaceSource.cpp (218732 => 218733)
--- trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2017-06-23 02:13:42 UTC (rev 218732)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2017-06-23 02:17:21 UTC (rev 218733)
@@ -108,6 +108,8 @@
{
ASSERT_UNUSED(loadedFont, &loadedFont == m_font.get());
+ Ref<CSSFontFace> protectedFace(m_face);
+
// If the font is in the cache, this will be synchronously called from CachedFont::addClient().
if (m_status == Status::Pending)
setStatus(Status::Loading);
@@ -120,7 +122,7 @@
if (m_face.webFontsShouldAlwaysFallBack())
return;
- if (m_font->errorOccurred())
+ if (m_font->errorOccurred() || !m_font->ensureCustomFontData(m_familyNameOrURI))
setStatus(Status::Failure);
else
setStatus(Status::Success);
@@ -193,10 +195,13 @@
}
if (m_font) {
- if (!m_font->ensureCustomFontData(m_familyNameOrURI))
- return nullptr;
+ auto success = m_font->ensureCustomFontData(m_familyNameOrURI);
+ ASSERT_UNUSED(success, success);
- return m_font->createFont(fontDescription, m_familyNameOrURI, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities);
+ ASSERT(status() == Status::Success);
+ auto result = m_font->createFont(fontDescription, m_familyNameOrURI, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities);
+ ASSERT(result);
+ return result;
}
// In-Document SVG Fonts
Modified: trunk/Source/WebCore/loader/cache/CachedSVGFont.cpp (218732 => 218733)
--- trunk/Source/WebCore/loader/cache/CachedSVGFont.cpp 2017-06-23 02:13:42 UTC (rev 218732)
+++ trunk/Source/WebCore/loader/cache/CachedSVGFont.cpp 2017-06-23 02:17:21 UTC (rev 218733)
@@ -54,9 +54,8 @@
RefPtr<Font> CachedSVGFont::createFont(const FontDescription& fontDescription, const AtomicString& remoteURI, bool syntheticBold, bool syntheticItalic, const FontFeatureSettings& fontFaceFeatures, const FontVariantSettings& fontFaceVariantSettings, FontSelectionSpecifiedCapabilities fontFaceCapabilities)
{
- if (firstFontFace(remoteURI))
- return CachedFont::createFont(fontDescription, remoteURI, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities);
- return nullptr;
+ ASSERT(firstFontFace(remoteURI));
+ return CachedFont::createFont(fontDescription, remoteURI, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities);
}
FontPlatformData CachedSVGFont::platformDataFromCustomData(const FontDescription& fontDescription, bool bold, bool italic, const FontFeatureSettings& fontFaceFeatures, const FontVariantSettings& fontFaceVariantSettings, FontSelectionSpecifiedCapabilities fontFaceCapabilities)
@@ -86,7 +85,7 @@
m_externalSVGDocument = nullptr;
if (m_externalSVGDocument)
maybeInitializeExternalSVGFontElement(remoteURI);
- if (!m_externalSVGFontElement)
+ if (!m_externalSVGFontElement || !firstFontFace(remoteURI))
return false;
if (auto convertedFont = convertSVGToOTFFont(*m_externalSVGFontElement))
m_convertedFont = SharedBuffer::create(WTFMove(convertedFont.value()));
