Bug 173962

Summary: REGRESSION(r216944): Font loads can cause Chinese characters to draw as .notdef
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, dino, jonlee, rniwa, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 174213    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Patch
none
Patch simon.fraser: review+

Description Myles C. Maxfield 2017-06-28 22:32:24 PDT
Font loads can cause Chinese characters to draw as .notdef
Comment 1 Myles C. Maxfield 2017-06-28 22:36:39 PDT
Created attachment 314116 [details]
Patch
Comment 2 Myles C. Maxfield 2017-06-28 22:39:28 PDT
<rdar://problem/32925318>
Comment 3 Myles C. Maxfield 2017-06-28 22:41:38 PDT
Comment on attachment 314116 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Font loads can cause Chinese characters to draw as .notdef

REGRESSION(r216944):

> LayoutTests/ChangeLog:3
> +        Font loads can cause Chinese characters to draw as .notdef

REGRESSION(r216944):
Comment 4 Build Bot 2017-06-29 00:02:49 PDT
Comment on attachment 314116 [details]
Patch

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

New failing tests:
fast/text/font-load-fallback-chinese.html
Comment 5 Build Bot 2017-06-29 00:02:50 PDT
Created attachment 314120 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 6 Myles C. Maxfield 2017-07-02 02:12:34 PDT
Created attachment 314412 [details]
Patch
Comment 7 Build Bot 2017-07-02 13:57:04 PDT
Comment on attachment 314412 [details]
Patch

Attachment 314412 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4039266

New failing tests:
workers/bomb.html
Comment 8 Build Bot 2017-07-02 13:57:05 PDT
Created attachment 314426 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 9 Myles C. Maxfield 2017-07-02 15:21:24 PDT
Failure is unrelated.
Comment 10 Build Bot 2017-07-03 13:20:11 PDT
Comment on attachment 314412 [details]
Patch

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

New failing tests:
fast/text/font-loading-system-fallback.html
fast/text/font-loading-system-fallback-visibility.html
Comment 11 Build Bot 2017-07-03 13:20:12 PDT
Created attachment 314511 [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.12.5
Comment 12 Myles C. Maxfield 2017-07-05 21:51:55 PDT
Created attachment 314695 [details]
Patch
Comment 13 Build Bot 2017-07-05 23:20:50 PDT
Comment on attachment 314695 [details]
Patch

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

New failing tests:
fast/text/font-loading-system-fallback.html
Comment 14 Build Bot 2017-07-05 23:20:52 PDT
Created attachment 314699 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 15 Myles C. Maxfield 2017-07-06 09:56:58 PDT
Created attachment 314728 [details]
Patch
Comment 16 Myles C. Maxfield 2017-07-06 14:14:59 PDT
Created attachment 314756 [details]
Patch
Comment 17 Simon Fraser (smfr) 2017-07-06 14:27:20 PDT
Comment on attachment 314756 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.cpp:289
> +    if (!m_derivedFontData)
> +        m_derivedFontData = std::make_unique<DerivedFonts>();

This is repeated 3 times. Maybe make an ensureDerivedFontData() function?

> Source/WebCore/platform/graphics/Font.h:137
> +    const Font& invisibleFont() const;

This is a little mysterious. Is this the font you use if you don't want the characters to display?

> Source/WebCore/platform/graphics/Font.h:284
> +        RefPtr<Font> invisible;

I would have said "Font" after the name. "invisibleFont". Same for the preceding.

> Source/WebCore/platform/graphics/FontCascadeFonts.cpp:333
> +

remove this blank line.
Comment 18 Myles C. Maxfield 2017-07-06 14:59:59 PDT
Comment on attachment 314756 [details]
Patch

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

>> Source/WebCore/platform/graphics/Font.h:137
>> +    const Font& invisibleFont() const;
> 
> This is a little mysterious. Is this the font you use if you don't want the characters to display?

Yep! For loading purposes.
Comment 19 Myles C. Maxfield 2017-07-06 15:09:16 PDT
Committed r219221: <http://trac.webkit.org/changeset/219221>