Closed Bug 1609184 Opened 2 years ago Closed 2 years ago

Crash in [@ TFont::FindColourBitmapForGlyph]

Categories

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

72 Branch
Desktop
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox-esr68 --- unaffected
firefox72 --- fixed
firefox73 --- fixed
firefox74 --- fixed

People

(Reporter: philipp, Assigned: jfkthame)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

+++ 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
...

Pinging Triage Owner for a priority -- thanks!

Flags: needinfo?(lsalzman)

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

Flags: needinfo?(madperson)
Flags: needinfo?(madperson)
Priority: -- → P3

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.

Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Pushed by jkew@mozilla.com:
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

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 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:
Attachment #9121190 - Flags: approval-mozilla-beta?
Flags: needinfo?(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...

Is it worth restricting this workaround to older macOS versions?

Flags: needinfo?(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.

Flags: needinfo?(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 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.

Attachment #9121190 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

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.

Flags: needinfo?(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

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
Attachment #9121190 - Flags: approval-mozilla-release?

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.

Flags: needinfo?(ryanvm)
Attachment #9121190 - Flags: approval-mozilla-release? → approval-mozilla-release+
You need to log in before you can comment on or make changes to this bug.