Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 171823002: Make text visible when font loading takes longer than 3 seconds (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 7 months ago by Kunihiko Sakamoto
Modified:
3 years, 6 months ago
CC:
blink-reviews, jamesr, krit, jbroman, dsinclair, rwlbuis, ed+blinkwatch_opera.com, fs, danakj, dglazkov+blink, Rik, apavlov+blink_chromium.org, gavinp+loader_chromium.org, gyuyoung.kim_webkit.org, darktears, f(malita), Stephen Chennney, Nate Chapin, pdr., rune+blink
Visibility:
Public.

Description

Make text visible when font loading takes longer than 3 seconds When a font resource takes a long time to download, currently Blink keeps the text invisible until the font is ready. After this patch, Blink behaves similar to FireFox; keeps the text invisible until 3 seconds, then fallback to the next available font in the fallback list. When the webfont finally loads, change the text to use it. - Add a Timer to FontResource that fires after 3 seconds since load start. - fontLoadWaitLimitExceeded() callback invalidates the fallback font and triggers style recalc. - Fallback fonts have m_skipDrawing boolean in CustomFontData, that is false if the font load exceeds the wait limit. - FontFallbackList::shouldSkipDrawing() returns true if it contains at least one FontData with m_skipDrawing == true. BUG=235303 TEST=http/tests/webfont/slow-loading.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168258

Patch Set 1 : #

Total comments: 4

Patch Set 2 : Use enum parameter #

Patch Set 3 : rebase #

Patch Set 4 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -26 lines) Patch
M LayoutTests/http/tests/webfont/slow-ahem-loading.cgi View 1 chunk +7 lines, -1 line 0 comments Download
A LayoutTests/http/tests/webfont/slow-loading.html View 1 chunk +74 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/webfont/slow-loading-expected.html View 1 chunk +45 lines, -0 lines 0 comments Download
M Source/core/css/CSSCustomFontData.h View 1 2 chunks +4 lines, -4 lines 0 comments Download
M Source/core/css/CSSFontFace.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSFontFace.cpp View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download
M Source/core/css/CSSFontFaceSource.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSFontFaceSource.cpp View 1 2 2 chunks +8 lines, -1 line 0 comments Download
M Source/core/css/CSSSegmentedFontFace.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSSegmentedFontFace.cpp View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/fetch/FontResource.h View 4 chunks +7 lines, -0 lines 0 comments Download
M Source/core/fetch/FontResource.cpp View 1 2 4 chunks +16 lines, -0 lines 0 comments Download
M Source/platform/fonts/CustomFontData.h View 1 3 chunks +8 lines, -4 lines 0 comments Download
M Source/platform/fonts/Font.h View 1 chunk +5 lines, -0 lines 0 comments Download
M Source/platform/fonts/Font.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/platform/fonts/FontData.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/platform/fonts/FontFallbackList.h View 2 chunks +2 lines, -1 line 0 comments Download
M Source/platform/fonts/FontFallbackList.cpp View 1 2 3 4 chunks +20 lines, -9 lines 0 comments Download
M Source/platform/fonts/SegmentedFontData.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/platform/fonts/SegmentedFontData.cpp View 2 chunks +14 lines, -4 lines 0 comments Download
M Source/platform/fonts/SimpleFontData.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 14 (0 generated)
Kunihiko Sakamoto
3 years, 7 months ago (2014-02-20 08:32:09 UTC) #1
eae
The logic and tests looks good to me I'm not sure what our policy on ...
3 years, 7 months ago (2014-02-20 16:51:46 UTC) #2
dglazkov
I am not up to speed on the latest thinking in regard to timers, but ...
3 years, 7 months ago (2014-02-20 17:45:40 UTC) #3
Kunihiko Sakamoto
On 2014/02/20 17:45:40, Dimitri Glazkov wrote: > Can we not simply treat this limit as ...
3 years, 7 months ago (2014-02-21 03:38:05 UTC) #4
Kunihiko Sakamoto
https://codereview.chromium.org/171823002/diff/130001/LayoutTests/http/tests/webfont/slow-loading.html File LayoutTests/http/tests/webfont/slow-loading.html (right): https://codereview.chromium.org/171823002/diff/130001/LayoutTests/http/tests/webfont/slow-loading.html#newcode67 LayoutTests/http/tests/webfont/slow-loading.html:67: displayElementAfter("span2", 0); On 2014/02/20 16:51:47, eae wrote: > What ...
3 years, 7 months ago (2014-02-21 03:38:24 UTC) #5
Kunihiko Sakamoto
James and Adam, any advice on the use of timers?
3 years, 7 months ago (2014-02-24 06:58:27 UTC) #6
abarth-chromium
On 2014/02/24 06:58:27, Kunihiko Sakamoto wrote: > James and Adam, any advice on the use ...
3 years, 7 months ago (2014-02-24 08:28:23 UTC) #7
Kunihiko Sakamoto
On 2014/02/24 08:28:23, abarth wrote: > On 2014/02/24 06:58:27, Kunihiko Sakamoto wrote: > > James ...
3 years, 7 months ago (2014-02-25 01:19:09 UTC) #8
Kunihiko Sakamoto
On 2014/02/25 01:19:09, Kunihiko Sakamoto wrote: > On 2014/02/24 08:28:23, abarth wrote: > > On ...
3 years, 7 months ago (2014-02-27 15:14:38 UTC) #9
Kunihiko Sakamoto
ping...
3 years, 6 months ago (2014-03-01 00:19:28 UTC) #10
dglazkov
lgtm.
3 years, 6 months ago (2014-03-01 03:44:23 UTC) #11
Kunihiko Sakamoto
The CQ bit was checked by ksakamoto@chromium.org
3 years, 6 months ago (2014-03-03 00:49:47 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ksakamoto@chromium.org/171823002/500001
3 years, 6 months ago (2014-03-03 00:49:52 UTC) #13
commit-bot: I haz the power
3 years, 6 months ago (2014-03-03 03:15:16 UTC) #14
Message was sent while issue was closed.
Change committed as 168258
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b