Bug 158339 - Show fallback text during font loads
Summary: Show fallback text during font loads
Status: RESOLVED DUPLICATE of bug 25207
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-02 21:42 PDT by Myles C. Maxfield
Modified: 2017-07-07 04:04 PDT (History)
6 users (show)

See Also:


Attachments
WIP (6.05 KB, patch)
2016-06-02 21:42 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Needs tests (7.19 KB, patch)
2016-06-03 00:48 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (18.87 KB, patch)
2016-06-03 11:35 PDT, Myles C. Maxfield
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (689.89 KB, application/zip)
2016-06-03 12:35 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2016-06-02 21:42:44 PDT
Show fallback text during font loads
Comment 1 Myles C. Maxfield 2016-06-02 21:42:59 PDT
Created attachment 280424 [details]
WIP
Comment 2 Myles C. Maxfield 2016-06-03 00:48:03 PDT
Created attachment 280432 [details]
Needs tests
Comment 3 Myles C. Maxfield 2016-06-03 11:35:50 PDT
Created attachment 280449 [details]
Patch
Comment 4 Build Bot 2016-06-03 12:35:17 PDT
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
Comment 5 Build Bot 2016-06-03 12:35:19 PDT
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
Comment 6 Myles C. Maxfield 2016-06-03 18:12:49 PDT

*** This bug has been marked as a duplicate of bug 25207 ***
Comment 7 Darin Adler 2016-06-04 13:47:54 PDT
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”.