1609184 - Crash in [@ TFont::FindColourBitmapForGlyph]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

[[:philipp]]

+++ This bug was initially created as a clone of Bug #1591432 +++

This bug is for crash report bp-cd3d4836-a0b0-4f32-987b-faf500200114.

Top 10 frames of crashing thread:

0 CoreText TFont::FindColourBitmapForGlyph const 
1 CoreText TFont::CreatePathForGlyph const 
2 CoreText CTFontCreatePathForGlyph 
3 XUL SkScalerContext_Mac::generatePath gfx/skia/skia/src/ports/SkFontHost_mac.cpp:1561
4 XUL SkScalerContext::internalGetPath gfx/skia/skia/src/core/SkScalerContext.cpp:630
5 XUL SkScalerContext::getMetrics gfx/skia/skia/src/core/SkScalerContext.cpp:192
6 libmozglue.dylib BaseAllocator::malloc memory/build/mozjemalloc.cpp:3956
7 libmozglue.dylib operator new[] memory/mozalloc/cxxalloc.h:42
8 XUL SkGlyph::setPath gfx/skia/skia/src/core/SkGlyph.cpp:166
9 XUL SkTextBlob::getIntercepts const gfx/skia/skia/src/core/SkStrike.cpp:86

after bug 1591432 seems to have fixed the problem in firefox 71, these tab crashes are spiking up again in 72.0 release and later. all the crashes are tab crashes coming from users on os x 10.9.

comments indicate that tabs are crashing repeatedly in some situations and some webmail urls are showing up frequently in submitted urls among other sites:
https://webmail.plus.net/squirrelmail/src/webmail.php
https://orange.safe-mail.net/cgi-bin/Safe-mail.net/display?N1P-yweVUujk&frames/frames.html
https://webmail.cybermesa.com/src/webmail.php
https://webmail.earthlink.net/wam/index.jsp?folder=INBOX.Trash
https://webmail.lmi.net/src/webmail.php
...

Comment 1

[Bert Peers [:bpeers]]

Pinging Triage Owner for a priority -- thanks!

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:philipp, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 3

[Jonathan Kew [:jfkthame]]

This looks like it's related to trying to implement text-decoration-skip-ink when the font is a color-bitmap font such as Apple Color Emoji, and Skia ends up hitting a Core Text bug trying to get the glyph outlines. Given that it's apparently 10.9-only, I think we can assume this is a CT bug that has been fixed in newer OS versions, but as Firefox still runs on 10.9 we should try to work around it.

We're not going to be able to retrieve paths and compute intercepts for such a font anyway, so maybe we can just check for the presence of an 'sbix' table and bail out of trying to get intercepts (so no ink-skipping will occur with such fonts -- but it wasn't going to work anyhow). AFAICS this should avoid the crash.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1609184 - Don't try to apply skip-ink to 'sbix' fonts like Apple Color Emoji, to avoid possible Core Text crash. r=lsalzman

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af66de41d253
Don't try to apply skip-ink to 'sbix' fonts like Apple Color Emoji, to avoid possible Core Text crash. r=lsalzman

Comment 6

[Jonathan Kew [:jfkthame]]

The Nightly crash rate is low enough that it may be hard to tell whether this really has solved the issue, but assuming no unexpected side-effects, I think we should uplift it to beta73, and also consider uplifting to 72 in the event of a further dot release being planned.

Comment 7

[Jonathan Kew [:jfkthame]]

Comment on attachment 9121190 [details]
Bug 1609184 - Don't try to apply skip-ink to 'sbix' fonts like Apple Color Emoji, to avoid possible Core Text crash. r=lsalzman

Beta/Release Uplift Approval Request

• User impact if declined: Possible tab crashes on older macOS versions
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: (unable to specifically test as no known STR, all we can do is watch crash-stats)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): No new functionality or tricky code; just trivial patch to skip applying skip-ink behavior to Apple emoji font.
• String changes made/needed:

Comment 8

[Lee Salzman [:lsalzman]]

(In reply to Jonathan Kew (:jfkthame) from comment #6)

The Nightly crash rate is low enough that it may be hard to tell whether this really has solved the issue, but assuming no unexpected side-effects, I think we should uplift it to beta73, and also consider uplifting to 72 in the event of a further dot release being planned.

I would expect given the crash reports all peg skip-ink as the culprit, that this should mostly eliminate the crashes...

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Is it worth restricting this workaround to older macOS versions?

Comment 10

[Jonathan Kew [:jfkthame]]

I considered doing that, but it didn't seem worth it given that skip-ink isn't expected to actually do anything useful for a bitmap font anyhow - and checking the crash reports, I saw a couple from 10.10 so it's possible the issue isn't quite as strictly limited to 10.9 as we thought.

Comment 11

[Jonathan Kew [:jfkthame]]

...though having said that, the 10.10 crash reports aren't the same, actually; they look like they're related to actually painting glyphs, not to the skip-ink feature. So they're probably not relevant here.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9121190 [details]
Bug 1609184 - Don't try to apply skip-ink to 'sbix' fonts like Apple Color Emoji, to avoid possible Core Text crash. r=lsalzman

Hopefully avoids a crash on older macOS versions by not applying skip-ink to certain fonts. Approved for 73.0b6 so we can see what affect it has over the weekend.

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/eb708b61ff55

Comment 14

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/af66de41d253

Comment 15

[Jonathan Kew [:jfkthame]]

I understand we may be doing a 72.0.2 release .... in that case I think we should consider taking this patch, given how safe it looks.

Comment 16

[Jonathan Kew [:jfkthame]]

Comment on attachment 9121190 [details]
Bug 1609184 - Don't try to apply skip-ink to 'sbix' fonts like Apple Color Emoji, to avoid possible Core Text crash. r=lsalzman

Beta/Release Uplift Approval Request

• User impact if declined: Possible tab crashes on older macOS versions
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: (unable to specifically test as no known STR, all we can do is watch crash-stats)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Minimal-risk patch to just skip applying skip-ink behavior to Apple emoji font.
• String changes made/needed: None

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9121190 [details]
Bug 1609184 - Don't try to apply skip-ink to 'sbix' fonts like Apple Color Emoji, to avoid possible Core Text crash. r=lsalzman

Not a ton of crash data to look at on Beta so far, but the initial results look promising. Low-risk enough that I agree it's worth taking for 72.0.2.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/25e0edbb0a613c3bf794c93ba3aa0985d29d5ef4