Changeset 223576 in webkit


Ignore:
Timestamp:
Oct 17, 2017 2:16:42 PM (6 years ago)
Author:
mmaxfield@apple.com
Message:

[CSS Font Loading] Fonts are erroneously invisible when the policy says they should be visible
https://bugs.webkit.org/show_bug.cgi?id=178238

Reviewed by Simon Fraser.

When implementing font-display, I added testing infrastructure (so we don't have to wait for
3 second timeouts to occur). This testing infrastructure covered up a real bug where the wrong
font would be reported to CSSFontAccessor. This patch reverts the erroneous testing
infrastructure and replaces it with a real fix to the problem. The replacement fix is covered
by the same tests that I wrote when implementing the feature.

Covered by existing tests.

  • css/CSSFontFace.cpp:

(WebCore::CSSFontFace::pump):
(WebCore::visibility):
(WebCore::CSSFontFace::font):

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r223574 r223576  
     12017-10-17  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        [CSS Font Loading] Fonts are erroneously invisible when the policy says they should be visible
     4        https://bugs.webkit.org/show_bug.cgi?id=178238
     5
     6        Reviewed by Simon Fraser.
     7
     8        When implementing font-display, I added testing infrastructure (so we don't have to wait for
     9        3 second timeouts to occur). This testing infrastructure covered up a real bug where the wrong
     10        font would be reported to CSSFontAccessor. This patch reverts the erroneous testing
     11        infrastructure and replaces it with a real fix to the problem. The replacement fix is covered
     12        by the same tests that I wrote when implementing the feature.
     13
     14        Covered by existing tests.
     15
     16        * css/CSSFontFace.cpp:
     17        (WebCore::CSSFontFace::pump):
     18        (WebCore::visibility):
     19        (WebCore::CSSFontFace::font):
     20
    1212017-10-16  Sam Weinig  <sam@webkit.org>
    222
  • trunk/Source/WebCore/css/CSSFontFace.cpp

    r222969 r223576  
    715715                    setStatus(Status::Loading);
    716716                source->load(m_fontSelector.get());
    717             } else if (fontLoadTimingOverride(m_fontSelector.get()) != Settings::FontLoadTimingOverride::None && m_status == Status::Pending) {
    718                 // Similar to above, if a test that has set fontLoadTimingOverride() needs to fail, this needs to happen
    719                 // eagerly so the FontRanges sees a consistent view of the CSSFontFace.
    720                 setStatus(Status::Loading);
    721717            }
    722718        }
     
    756752}
    757753
     754static Font::Visibility visibility(CSSFontFace::Status status, CSSFontFace::FontLoadTiming timing)
     755{
     756    switch (status) {
     757    case CSSFontFace::Status::Pending:
     758        return timing.blockPeriod == 0_s ? Font::Visibility::Visible : Font::Visibility::Invisible;
     759    case CSSFontFace::Status::Loading:
     760        return Font::Visibility::Invisible;
     761    case CSSFontFace::Status::TimedOut:
     762    case CSSFontFace::Status::Failure:
     763    case CSSFontFace::Status::Success:
     764    default:
     765        return Font::Visibility::Visible;
     766    }
     767}
     768
    758769RefPtr<Font> CSSFontFace::font(const FontDescription& fontDescription, bool syntheticBold, bool syntheticItalic, ExternalResourceDownloadPolicy policy)
    759770{
     
    778789        case CSSFontFaceSource::Status::Pending:
    779790        case CSSFontFaceSource::Status::Loading: {
    780             Font::Visibility visibility = status() == Status::TimedOut || status() == Status::Failure ? Font::Visibility::Visible : Font::Visibility::Invisible;
     791            Font::Visibility visibility = WebCore::visibility(status(), fontLoadTiming());
    781792            return Font::create(FontCache::singleton().lastResortFallbackFont(fontDescription)->platformData(), Font::Origin::Local, Font::Interstitial::Yes, visibility);
    782793        }
Note: See TracChangeset for help on using the changeset viewer.