RESOLVED FIXED in Firefox 61

Status

()

defect
--
major
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: laszlo.bialis, Assigned: jfkthame)

Tracking

61 Branch
mozilla61
Unspecified
macOS
Points:
---

Firefox Tracking Flags

(firefox61 fixed)

Details

(crash signature)

Attachments

(2 attachments)

Reporter

Description

a year ago
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180329222832

[Affected versions]:
Nightly 61.0a1

[Affected platforms]:
Mac 10.13

1.Launch Nightly 61.0a1 with a new profile
2.Set pref gfx.downloadable_fonts.otl_validation = false
3.Restart
4.Open https://developer.microsoft.com/en-us/microsoft-edge/testdrive/demos/variable-fonts/

Expected result:
The page should be displayed correctly.

Actual result:
Crash: https://crash-stats.mozilla.com/report/index/ea6ac35a-cfb0-4f10-8e32-33b8a0180330

Note: Could not repro on Win 10
Assignee

Comment 1

a year ago
If you set gfx.downloadable_fonts.keep_variation_tables = false before loading the page, does that prevent the crash?

If you set layout.css.font-variations.enabled = false before loading the page, does that prevent the crash?

Does the page load OK in Safari on the same system?
Reporter

Comment 2

a year ago
(In reply to Jonathan Kew (:jfkthame) from comment #1)

If Set pref gfx.downloadable_fonts.otl_validation = false and
> If you set gfx.downloadable_fonts.keep_variation_tables = false before
> loading the page, does that prevent the crash? ---> Works

> If you set layout.css.font-variations.enabled = false before loading the
> page, does that prevent the crash? ---> Works
> 
> Does the page load OK in Safari on the same system? ---> Safari looks good, no crash, on basic settings.
Assignee

Comment 3

a year ago
One more test: I've pushed a try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe355ea1e3f12a013b375d75372d784b3e485398. When this is ready, could you please see if the page still crashes that build (with default prefs)? Thanks!
Flags: needinfo?(laszlo.bialis)
Assignee

Comment 4

a year ago
BTW, what exact version of macOS is this? (10.13.???)

FWIW, the microsoft demo page doesn't crash, and seems to work fine for me on 10.12.6.
Reporter

Comment 5

a year ago
(In reply to Jonathan Kew (:jfkthame) from comment #3)
> One more test: I've pushed a try build at
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=fe355ea1e3f12a013b375d75372d784b3e485398. When this
> is ready, could you please see if the page still crashes that build (with
> default prefs)? Thanks!

Tried with Build ID:20180330121558, but the crash is reproduced.
(In reply to Jonathan Kew (:jfkthame) from comment #4)
> BTW, what exact version of macOS is this? (10.13.???). 
  10.13.1 (17B1003)
 
> FWIW, the microsoft demo page doesn't crash, and seems to work fine for me
> on 10.12.6. 
The crash occurs when opening the exact link https://developer.microsoft.com/en-us/microsoft-edge/testdrive/demos/variable-fonts/ the page is displayed for max one second before the crash happens.
Flags: needinfo?(laszlo.bialis)
Assignee

Comment 6

a year ago
On 10.12.6, I can open that link with no problem. Is there someone who could test with the latest update (10.13.4) to see if the crash still occurs there?
Assignee

Updated

a year ago
Duplicate of this bug: 1451117
Assignee

Comment 8

a year ago
:mstange confirmed this still crashes on 10.13.4. Another site that crashes similarly is https://v-fonts.com/.
Summary: Crash when opening variable https://developer.microsoft.com fonts page → Crash when opening some variable-fonts demo pages [including https://developer.microsoft.com/en-us/microsoft-edge/testdrive/demos/variable-fonts/, https://v-fonts.com/]
I caught this in the debugger.

The glyph ID is 4 and the font is "Sitka FV Regular".

The crash happens with this stack:
> GetUnscaledAdvances(TFont const&, unsigned short const*, CGSize*, long, CTFontOrientation) + 76
> TFont::GetAdvancesForGlyphs(unsigned short const*, CGSize*, long, CTFontOrientation) const + 96
> CTFontGetAdvancesForGlyphs + 213
> gfxMacFont::GetGlyphWidth(this=0x000000015e4eaae0, aDrawTarget=0x000000010887bb80, aGID=4) at gfxMacFont.cpp:469

CTFontGetAdvancesForGlyphs gets a non-null TFont* from a field in the CTFont object at offset 0x28.
GetUnscaledAdvances gets a pointer from a field in the TFont object at offset 0x148. GetUnscaledAdvances dereferences this pointer without null-checking, but the pointer is null, so we crash.

So my guess is that either something goes wrong during the construction of the TFont, leaving it with a null field in a place that should always be non-null, or some code that is supposed to lazy-initialize a field in the TFont isn't being called in time for our call to CTFontGetAdvancesForGlyphs.

Out of curiousity, I set a breakpoint on GetUnscaledAdvances to check whether it was being called for other fonts with non-null values in the relevant field, and yes, there were other fonts for which this field was non-null.
Assignee

Comment 10

a year ago
Thanks for looking into this, Markus. It looks to me very much like a Core Text bug; if we could create a reduced testcase, it would be worth reporting to Apple. If I update my machine to 10.13, I may take a stab at that.

Meanwhile, though, we'll need to work around this, probably by avoiding the use of CTFontGetAdvancesForGlyphs. :( I'm looking into a couple of possible approaches.
Assignee

Comment 11

a year ago
I've pushed try jobs with a couple of different patches that I hope might avoid the issue here:

(a) https://treeherder.mozilla.org/#/jobs?repo=try&revision=694413ba9c087640b8d75efdde247fa187bcf4cf

(b) https://treeherder.mozilla.org/#/jobs?repo=try&revision=7be668e991aad283415d738ec47ae81439afdea9

As I don't currently have a 10.13.x machine, these are untested (the crash doesn't occur on 10.12). Laszlo, Markus: if you could try these builds and confirm whether either/both of them do resolve the crash, that would be really helpful - thanks!
Flags: needinfo?(mstange)
Flags: needinfo?(laszlo.bialis)
Reporter

Comment 12

a year ago
(In reply to Jonathan Kew (:jfkthame) from comment #11)
> I've pushed try jobs with a couple of different patches that I hope might
> avoid the issue here:
> 
> (a)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=694413ba9c087640b8d75efdde247fa187bcf4cf
> 
> (b)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=7be668e991aad283415d738ec47ae81439afdea9
> 
> As I don't currently have a 10.13.x machine, these are untested (the crash
> doesn't occur on 10.12). Laszlo, Markus: if you could try these builds and
> confirm whether either/both of them do resolve the crash, that would be
> really helpful - thanks!

Tried with both patches and couldn't reproduce the crash on either of the pages, both links open successfully in both patches (a)and(b).

However in case of both patches and in case of the link https://v-fonts.com/ although no crash occurs, still some fonts are not displayed (ex. the second font on the page Venn VF). I've also tried to open the link in Safari/Chrome and in both cases all the fonts from the page are displayed correctly. 
Is this intended? 

Tried also on Win 10 with https://v-fonts.com/ in Nightly (Version 61.0a1 and Build ID 20180401220058) same fonts are missing as on MacOS 10.13.
Flags: needinfo?(laszlo.bialis)
Assignee

Updated

a year ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee

Comment 14

a year ago
(In reply to laszlo.bialis from comment #12)
> However in case of both patches and in case of the link https://v-fonts.com/
> although no crash occurs, still some fonts are not displayed (ex. the second
> font on the page Venn VF). I've also tried to open the link in Safari/Chrome
> and in both cases all the fonts from the page are displayed correctly. 
> Is this intended? 
> 
> Tried also on Win 10 with https://v-fonts.com/ in Nightly (Version 61.0a1
> and Build ID 20180401220058) same fonts are missing as on MacOS 10.13.

I think this must be an unrelated problem. All the fonts on https://v-fonts.com/ show up as expected for me in Nightly (both with and without patches here) on macOS 10.12, and also on Win10.

If the same fonts failed for you on both macOS and Win10, where the font back-end in firefox is entirely different, that suggests maybe a temporary issue with the site or with your connection, such that they failed to download correctly.
Attachment #8965267 - Flags: review?(mstange) → review+

Comment 15

a year ago
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95342072369b
Avoid use of CTFontGetAdvancesForGlyphs with variation fonts due to possible Core Text crash. r=mstange

Comment 16

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/95342072369b
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Flags: needinfo?(mstange)
Reporter

Comment 17

a year ago
The issue still reproduces on the latest Nightly, accessing https://v-fonts.com/ causes an instant crash.

Report: https://crash-stats.mozilla.com/report/index/1775fa57-2d08-4a21-983d-3be260180416

Should I reopen this issue?
Flags: needinfo?(jfkthame)
Assignee

Comment 18

a year ago
We fixed the issue at the original location, but it's now crashing within a Core Text call made by Skia. :(  We have bug 1454094 about that, so no need to re-open this.
Flags: needinfo?(jfkthame)
You need to log in before you can comment on or make changes to this bug.