Changeset 216079 in webkit


Ignore:
Timestamp:
May 2, 2017 11:02:50 AM (7 years ago)
Author:
mmaxfield@apple.com
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:
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r216075 r216079  
     12017-05-02  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        Font Loading API specifies font is loaded but sizing of font after load reports inconsistent values
     4        https://bugs.webkit.org/show_bug.cgi?id=168533
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        * fast/text/font-loading-local-expected.txt: Added.
     9        * fast/text/font-loading-local.html: Added.
     10        * fast/text/web-font-load-fallback-during-loading.html:
     11
    1122017-05-02  Youenn Fablet  <youenn@apple.com>
    213

  • trunk/LayoutTests/fast/text/web-font-load-fallback-during-loading.html

    r201676 r216079  
    5454<body>
    5555This 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.
     56<!-- FIXME: This test is racey. The font may complete downloading before the 0-delay timer fires,
     57in which case the "Test Complete" text will not be added. -->
    5658<p id="console"></p>
    5759<p><span id="reference" style="font-family: Helvetica">This is rendered with Helvetica.</span></p>

  • trunk/Source/WebCore/ChangeLog

    r216075 r216079  
     12017-05-02  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        Font Loading API specifies font is loaded but sizing of font after load reports inconsistent values
     4        https://bugs.webkit.org/show_bug.cgi?id=168533
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        Previously, we were marking all local() fonts as immediately successful,
     9        regardless of whether or not they were present on the system. Instead, we
     10        should use the load() function to make this determination and mark the font
     11        as failed if it doesn't exist. (This is, after all, the whole point of the
     12        load() function). This brings us in-line with Firefox's and Chrome's
     13        behavior.
     14
     15        Test: fast/text/font-loading-local.html
     16
     17        * css/CSSFontFace.cpp:
     18        (WebCore::CSSFontFace::pump): Remote loading requires the FontSelector,
     19        but it isn't available for local fonts. Now that load() is called for both
     20        local and remote fonts, the ASSERT() should be lowered into the load()
     21        function and scoped to just the case where we have a remote font.
     22        (WebCore::CSSFontFace::font): Ditto.
     23        * css/CSSFontFaceSource.cpp:
     24        (WebCore::CSSFontFaceSource::CSSFontFaceSource): Don't immediatley set
     25        the success/failure state for local fonts.
     26        (WebCore::CSSFontFaceSource::load): Move loading logic from font() to
     27        load(). None of this code is new; it just is moved.
     28        (WebCore::CSSFontFaceSource::font): Delete code moved to load().
     29        * css/CSSFontFaceSource.h:
     30        * css/FontFace.cpp:
     31        (WebCore::FontFace::create):
     32
    1332017-05-02  Youenn Fablet  <youenn@apple.com>
    234

  • trunk/Source/WebCore/css/CSSFontFace.cpp

    r215287 r216079  
    559559        if (source->status() == CSSFontFaceSource::Status::Pending) {
    560560            ASSERT(m_status == Status::Pending || m_status == Status::Loading || m_status == Status::TimedOut);
    561             ASSERT(m_fontSelector);
    562561            if (m_status == Status::Pending)
    563562                setStatus(Status::Loading);
    564             source->load(*m_fontSelector);
     563            source->load(m_fontSelector.get());
    565564        }
    566565
    … …  
    613612        auto& source = m_sources[i];
    614613        if (source->status() == CSSFontFaceSource::Status::Pending) {
    615             ASSERT(m_fontSelector);
    616             if (fontIsLoading)
     614            if (fontIsLoading && source->requiresExternalResource())
    617615                continue;
    618             source->load(*m_fontSelector);
     616            source->load(m_fontSelector.get());
    619617        }
    620618

  • trunk/Source/WebCore/css/CSSFontFaceSource.cpp

    r215287 r216079  
    9090        m_font->addClient(*this);
    9191
    92     if (status() == Status::Pending && (!m_font || m_font->isLoaded())) {
     92    if (status() == Status::Pending && m_font && m_font->isLoaded()) {
    9393        setStatus(Status::Loading);
    9494        if (m_font && m_font->errorOccurred())
    … …  
    129129}
    130130
    131 void CSSFontFaceSource::load(CSSFontSelector& fontSelector)
     131void CSSFontFaceSource::load(CSSFontSelector* fontSelector)
    132132{
    133133    setStatus(Status::Loading);
    134134
    135     ASSERT(m_font);
    136     fontSelector.beginLoadingFontSoon(*m_font);
     135    if (m_font) {
     136        ASSERT(fontSelector);
     137        fontSelector->beginLoadingFontSoon(*m_font);
     138    } else {
     139        bool success = false;
     140        if (m_svgFontFaceElement) {
     141            if (is<SVGFontElement>(m_svgFontFaceElement->parentNode())) {
     142                ASSERT(!m_inDocumentCustomPlatformData);
     143                SVGFontElement& fontElement = downcast<SVGFontElement>(*m_svgFontFaceElement->parentNode());
     144                if (auto otfFont = convertSVGToOTFFont(fontElement))
     145                    m_generatedOTFBuffer = SharedBuffer::create(WTFMove(otfFont.value()));
     146                if (m_generatedOTFBuffer) {
     147                    m_inDocumentCustomPlatformData = createFontCustomPlatformData(*m_generatedOTFBuffer);
     148                    success = static_cast<bool>(m_inDocumentCustomPlatformData);
     149                }
     150            }
     151        } else if (m_immediateSource) {
     152            ASSERT(!m_immediateFontCustomPlatformData);
     153            bool wrapping;
     154            RefPtr<SharedBuffer> buffer = SharedBuffer::create(static_cast<const char*>(m_immediateSource->baseAddress()), m_immediateSource->byteLength());
     155            ASSERT(buffer);
     156            m_immediateFontCustomPlatformData = CachedFont::createCustomFontData(*buffer, wrapping);
     157            success = static_cast<bool>(m_immediateFontCustomPlatformData);
     158        } else {
     159            // We are only interested in whether or not fontForFamily() returns null or not. Luckily, none of
     160            // the values in the FontDescription other than the family name can cause the function to return
     161            // null if it wasn't going to otherwise (and vice-versa).
     162            FontCascadeDescription fontDescription;
     163            fontDescription.setOneFamily(m_familyNameOrURI);
     164            fontDescription.setComputedSize(1);
     165            success = FontCache::singleton().fontForFamily(fontDescription, m_familyNameOrURI, nullptr, nullptr, FontSelectionSpecifiedCapabilities(), true);
     166        }
     167        setStatus(success ? Status::Success : Status::Failure);
     168    }
    137169}
    138170
    … …  
    148180    if (!m_font && !fontFaceElement) {
    149181        if (m_immediateSource) {
    150             if (!m_immediateFontCustomPlatformData) {
    151                 bool wrapping;
    152                 RefPtr<SharedBuffer> buffer = SharedBuffer::create(static_cast<const char*>(m_immediateSource->baseAddress()), m_immediateSource->byteLength());
    153                 ASSERT(buffer);
    154                 m_immediateFontCustomPlatformData = CachedFont::createCustomFontData(*buffer, wrapping);
    155             } if (!m_immediateFontCustomPlatformData)
     182            if (!m_immediateFontCustomPlatformData)
    156183                return nullptr;
    157184            return Font::create(CachedFont::platformDataFromCustomData(*m_immediateFontCustomPlatformData, fontDescription, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities), true);
    … …  
    177204    if (!is<SVGFontElement>(m_svgFontFaceElement->parentNode()))
    178205        return nullptr;
    179     if (!m_inDocumentCustomPlatformData) {
    180         SVGFontElement& fontElement = downcast<SVGFontElement>(*m_svgFontFaceElement->parentNode());
    181         if (auto otfFont = convertSVGToOTFFont(fontElement))
    182             m_generatedOTFBuffer = SharedBuffer::create(WTFMove(otfFont.value()));
    183         if (!m_generatedOTFBuffer)
    184             return nullptr;
    185         m_inDocumentCustomPlatformData = createFontCustomPlatformData(*m_generatedOTFBuffer);
    186     }
    187206    if (!m_inDocumentCustomPlatformData)
    188207        return nullptr;

  • trunk/Source/WebCore/css/CSSFontFaceSource.h

    r215287 r216079  
    6767    const AtomicString& familyNameOrURI() const { return m_familyNameOrURI; }
    6868
    69     void load(CSSFontSelector&);
     69    void load(CSSFontSelector*);
    7070    RefPtr<Font> font(const FontDescription&, bool syntheticBold, bool syntheticItalic, const FontFeatureSettings&, const FontVariantSettings&, FontSelectionSpecifiedCapabilities);
     71
     72    bool requiresExternalResource() const { return m_font; }
    7173
    7274#if ENABLE(SVG_FONTS)

  • trunk/Source/WebCore/css/FontFace.cpp

    r214433 r216079  
    108108    if (!dataRequiresAsynchronousLoading) {
    109109        result->backing().load();
    110         ASSERT(result->backing().status() == CSSFontFace::Status::Success);
     110        auto status = result->backing().status();
     111        ASSERT_UNUSED(status, status == CSSFontFace::Status::Success || status == CSSFontFace::Status::Failure);
    111112    }
    112113
Note: See TracChangeset for help on using the changeset viewer.