bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

crash [@ gfxAtsuiFont::SetupCairoFont]




10 years ago
7 years ago


(Reporter: Gavin, Assigned: roc)


({crash, testcase})

Mac OS X
crash, testcase
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [needs review], crash signature)


(4 attachments)

aContext is null. I see:

WARNING: Could not get glyph extents: file /Users/gavin/moz/mozilla/gfx/thebes/src/gfxFont.cpp, line 538

before crashing.
Flags: blocking1.9?
Created attachment 292438 [details]
testcase (crashes on load)
Attachment #292438 - Attachment mime type: text/plain → text/html
Severity: normal → critical
Keywords: crash

Comment 3

10 years ago
JDaggett this going to get fixed by your font work?
Assignee: nobody → jdaggett
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3

Comment 4

10 years ago
No, I don't think this will be fixed by any changes in the font matching code.  This one started showing up around 11/9 in crash reports so I'm guessing it's tied to some of the changes for by 385417 by roc/reed.  The crash reports only show it occurring on 10.5.


Comment 5

10 years ago
Confirmed works-for-me on 10.4.11 with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2007121204 Minefield/3.0b3pre

Will test this under 10.5 tomorrow.
I can't get this to crash in the latest Minefield nightly or my CmTrunk build from tonight on 10.5.1.

After a slew of the warnings from comment 0, I also see this:
<Error>: ATSFontGetPostScriptName failed to return valid ASCII PostScript name


10 years ago
Duplicate of this bug: 408701

Comment 8

10 years ago
(In reply to comment #7)
> *** Bug 408701 has been marked as a duplicate of this bug. ***

Good test sites to reproduce (From Bug 408701)
This bug appears for me on the latest nightly. :/
I do crash with attachment https://bugzilla.mozilla.org/attachment.cgi?id=291259 with this stack and Gavin's warning on 10.5.1 ;)
Duplicate of this bug: 409185
Marcia says over in bug 396137 this is the #3 crasher for b2.
This is crashing because http://mxr.mozilla.org/seamonkey/source/layout/generic/nsTextFrameThebes.cpp#5690 calls MeasureText with a nsnull refContext -- however, it also specifies PR_FALSE to tight metrics, and the refContext is only supposed to be used when tight metrics are requested.

The problem is, one of the glyphs is not a simple glyph, so in http://mxr.mozilla.org/seamonkey/source/gfx/thebes/src/gfxFont.cpp#404 we fail that test, and we drop into the else clause which unconditionally does GetTightGlyphExtents... and then boom.

roc, what's supposed to happen here?  I'm guessing we're seeing this a lot more because we dropped the min font size for using tight glyph extents a good amount.  Are we supposed to just not request the tight glyph metrics and use the bounding box like we do in the simple glyph case?
There should have been a FetchGlyphExtents call on that textrun which should have preloaded all necessary glyph extents, right here:
Using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007122004 Minefield/3.0b3pre, I do not crash with Gavin's test case, but I crash instantly when I visit http://www.deviantart.com/deviation/70742546/.
Created attachment 294142 [details]
testcase that triggers

Ok, so what's happening is that there's a bogus glyph (0x3232) in here that, for some reason, we're assinging to the same text run as the other normal ascii glyphs.  That glyph doesn't exist in the font, so the initial caching of metrics fails (this is the assertion).  But later since there's no entry for it, we end up blowing up because we go through the same path expecting that everything was cached.

I'm inclined to leave this be until John's patch lands -- John, could you test this out if you get a chance with your patch?  The deviantart link crashes every time for me, but here's a testcase for at least the assertion (it doesn't cause a crash, though, but the assertion should also go away).

(Note that the first testcase only sporadically crashes for me.)

The "bogus glyph" is actually the character code; this is a missing-glyph situation and we shouldn't be trying get the extents for such "glyphs". I'll fix.
Assignee: jdaggett → roc

Comment 17

10 years ago
As of the current nightly, this no longer triggers with either of testcases..

But the deviantart link I provided still does.  I am curious if Bug 408701 is really a duplicate, or is this a related but different problem?

Created attachment 294151 [details] [diff] [review]

1) Add better comments to FetchGlyphExtents and elsewhere
2) FetchGlyphExtents should not try to fetch extents for missing glyphs
3) gfxFont::Measure should not try to get extents for missing glyphs; the rectangle given by the glyph advance and the font ascent and descent covers the missing-glyph box
4) gfxFont::Measure should gracefully handle the cases where glyph extents are not available. They might not be available due to OOM when we were setting up the textrun.
Attachment #294151 - Flags: review?(vladimir)
Whiteboard: [needs review]
Comment on attachment 294151 [details] [diff] [review]

Looks good to me!
Attachment #294151 - Flags: review?(vladimir) → review+

Comment 20

10 years ago
Do we understand why this only occurs on 10.5?  With Vlad's testcase on 10.5 I see warnings "Could not get glyph extents" which I don't see on 10.4.  In both cases, there are no fonts for the missing character U+0CA0 (unless you have fonts for Kannada installed), so the missing extents should be the same.  Are we just unlucky on 10.5 where something failed benignly on 10.4?
checked in, with crashtest
Last Resolved: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(In reply to comment #20)
> Do we understand why this only occurs on 10.5?

To be honest, I don't. It's possible ATSUGlyphGetScreenMetrics always returns noErr on 10.4 for an unknown glyph ID and just gives us bogus metrics, that would hide this bug.

A similar explanation probably holds for Windows and Pango: the default gfxFont::SetupGlyphExtents calls cairo_glyph_extents without checking for errors, so we always appear to get some extents even for bogus glyph IDs, also hiding this bug.
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007123104 Minefield/3.0b3pre and the testcase from this bug - no crash on testcase

-> Verified fixed
Crash Signature: [@ gfxAtsuiFont::SetupCairoFont]
You need to log in before you can comment on or make changes to this bug.