Bug 158339 – Show fallback text during font loads

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: New Bugs

[Myles C. Maxfield]

Show fallback text during font loads

[Myles C. Maxfield]

Created attachment 280424 [details]
WIP

[Myles C. Maxfield]

Created attachment 280432 [details]
Needs tests

[Myles C. Maxfield]

Created attachment 280449 [details]
Patch

[Build Bot]

Comment on attachment 280449 [details]
Patch

Attachment 280449 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1429928

New failing tests:
fast/text/web-font-load-fallback-during-loading.html

[Build Bot]

Created attachment 280453 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.4

[Myles C. Maxfield]

*** This bug has been marked as a duplicate of bug 25207 ***

[Darin Adler]

Comment on attachment 280449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=280449&action=review

> Source/WebCore/css/CSSFontFace.cpp:354
> +    ASSERT(m_fontSelector);
> +    m_fontSelector->fontLoaded();
> +
> +    iterateClients(m_clients, [&](Client& client) {
> +        client.fontLoaded(*this);
> +    });

Seems confusing that fontLoaded will be called twice. Are all the callers prepared for this?

> Source/WebCore/css/CSSFontFace.cpp:442
> +        double timeout = 3; // seconds

Doesn’t seem good for this 3 second value to be here in the setStatus function. Can we put it at the top of the file with a comment explaining the reason we chose 3 seconds?

> Source/WebCore/css/CSSFontFace.h:160
> +    Timer m_timeoutTimer;

Timers are pretty big objects. Does each CSSFontFace really need to have its own timer just for this? Maybe use a std::unique_ptr<Timer> instead? Or maybe CSSFontFace is really huge already and this isn’t a significant additional stuff.

Is there some set of other data members that are only relevant for web fonts? Seems unfortunate the things about font loading are all mixed in with the things about fonts “at rest”.