1460543 - Crash @ gfxFontEntry::GetVariationsForStyle when loading @font face html

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect, P2)

Description

[roxleitan]

Attachment: test font face.html

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID:20180509220105

[Affected versions]:
Nightly 62.0a1

[Affected platforms]:
Windows 10 x64

[Steps to reproduce]:
1.Launch Nightly 62.0a1 with a new profile
2.Open the attached Html

[Expected result]:
The Html should be displayed correctly

[Actual result]:
Crash:
https://crash-stats.mozilla.com/report/index/fb17dabd-99ac-4b5c-9654-b881d0180510

Comment 1

[Jonathan Kew [:jfkthame]]

Confirmed this crashes on Win10 for me, too. On macOS, it doesn't crash, though the rendering is incorrect: it uses the bold weight even for the body text that should be regular.

Comment 2

[Jonathan Kew [:jfkthame]]

Oh, wait... the second @font-face rule doesn't have a font-weight descriptor, so it essentially overrides the first. So the bold rendering is expected, as all the content uses the rule with the font-variation-settings descriptor.

Crashing is still just as bad, though.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Check for null (default) style parameter in gfxDWriteFontEntry::CreateFontFace before dereferencing it

The DW code uses gfxDWriteFontEntry::CreateFontFace without passing a specific gfxFontStyle when it just needs to instantiate a face to read font tables, etc., so we need to handle this safely.

Comment 4

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Kew (:jfkthame) from comment #1)
> On macOS, it doesn't crash,
> though the rendering is incorrect: it uses the bold weight even for the body
> text that should be regular.

Will you file a separate bug for that?

Comment 5

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Kew (:jfkthame) from comment #2)
> Oh, wait...

Doh, jumped the gun there. ;)

Comment 6

[Jonathan Watt [:jwatt]]

Comment on attachment 8974680 [details] [diff] [review]
Check for null (default) style parameter in gfxDWriteFontEntry::CreateFontFace before dereferencing it

Review of attachment 8974680 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxDWriteFontList.cpp
@@ +784,5 @@
>  
>          // Get the variation settings needed to instantiate the fontEntry
>          // for a particular fontStyle.
>          AutoTArray<gfxFontVariation,4> vars;
> +        GetVariationsForStyle(vars, aFontStyle ? *aFontStyle : gfxFontStyle());

Maybe expand the comment above these lines.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1819372b068a
Check for null (default) style parameter in gfxDWriteFontEntry::CreateFontFace before dereferencing it. r=jwatt

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/1819372b068a

Comment 9

[roxleitan]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID:20180510220127

I cannot reproduce the crash on the latest Nightly 62.0a1 on Windows 10 x64.