Closed
Bug 407761
Opened 17 years ago
Closed 17 years ago
crash [@ gfxAtsuiFont::SetupCairoFont]
Categories
(Core :: Graphics, defect, P3)
Tracking
()
VERIFIED
FIXED
People
(Reporter: Gavin, Assigned: roc)
References
Details
(Keywords: crash, testcase, Whiteboard: [needs review])
Crash Data
Attachments
(4 files)
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?
Reporter | ||
Comment 1•17 years ago
|
||
Reporter | ||
Comment 2•17 years ago
|
||
Reporter | ||
Updated•17 years ago
|
Attachment #292438 -
Attachment mime type: text/plain → text/html
Comment 3•17 years ago
|
||
JDaggett this going to get fixed by your font work?
Assignee: nobody → jdaggett
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Comment 4•17 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. http://crash-stats.mozilla.com/report/list?range_unit=months&query_search=signature&query_type=contains&platform=mac&signature=gfxAtsuiFont%3A%3ASetupCairoFont%28gfxContext%2A%29&range_value=2
Comment 5•17 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
(In reply to comment #7) > *** Bug 408701 has been marked as a duplicate of this bug. *** > Good test sites to reproduce (From Bug 408701) http://-coey-.deviantart.com or http://www.deviantart.com/deviation/70742546/ 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 ;)
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?
Assignee | ||
Comment 13•17 years ago
|
||
There should have been a FetchGlyphExtents call on that textrun which should have preloaded all necessary glyph extents, right here: http://mxr.mozilla.org/seamonkey/source/gfx/thebes/src/gfxFont.cpp#1948
Comment 14•17 years ago
|
||
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/.
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.)
Assignee | ||
Comment 16•17 years ago
|
||
Taking. 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•17 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? Thoughts?
Assignee | ||
Comment 18•17 years ago
|
||
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)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [needs review]
Comment on attachment 294151 [details] [diff] [review] fix Looks good to me!
Attachment #294151 -
Flags: review?(vladimir) → review+
Comment 20•17 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?
Assignee | ||
Comment 21•17 years ago
|
||
checked in, with crashtest
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Assignee | ||
Comment 22•17 years ago
|
||
(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.
Comment 23•17 years ago
|
||
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
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
Crash Signature: [@ gfxAtsuiFont::SetupCairoFont]
You need to log in
before you can comment on or make changes to this bug.
Description
•