Closed Bug 820707 Opened 7 years ago Closed 7 years ago

crash in gfxGDIFont::Initialize @ gfxFontFamily::HasItalicFace

Categories

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

20 Branch
All
Windows 7
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla20
Tracking Status
firefox20 - verified

People

(Reporter: scoobidiver, Assigned: jfkthame)

References

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(1 file, 1 obsolete file)

It began spiking in 20.0a1/20121211 and is currently #4 top crasher in this build. The regression range for the spike is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=725eb8792d27&tochange=4dfe323a663d
It might be a regression from bug 816483.

Stack traces are various:
Frame 	Module 	Signature 	Source
0 	xul.dll 	gfxFontFamily::HasItalicFace 	gfx/thebes/gfxFont.h:665
1 	xul.dll 	gfxGDIFont::Initialize 	gfx/thebes/gfxGDIFont.cpp:293
2 	xul.dll 	gfxGDIFont::ShapeWord 	gfx/thebes/gfxGDIFont.cpp:114
3 	xul.dll 	gfxFont::GetShapedWord<unsigned char> 	gfx/thebes/gfxFont.cpp:2402
4 	xul.dll 	gfxFont::SplitAndInitTextRun<unsigned char> 	gfx/thebes/gfxFont.cpp:2594
5 	xul.dll 	gfxFontGroup::InitScriptRun<unsigned char> 	gfx/thebes/gfxFont.cpp:3754
6 	xul.dll 	gfxFontGroup::InitTextRun<unsigned char> 	gfx/thebes/gfxFont.cpp:3651
7 	xul.dll 	gfxFontGroup::MakeTextRun 	gfx/thebes/gfxFont.cpp:3572
8 	xul.dll 	BuildTextRunsScanner::BuildTextRunForFrames 	layout/generic/nsTextFrameThebes.cpp:2052
...

Frame 	Module 	Signature 	Source
0 	xul.dll 	gfxFontFamily::HasItalicFace 	gfx/thebes/gfxFont.h:665
1 	xul.dll 	gfxGDIFont::Initialize 	gfx/thebes/gfxGDIFont.cpp:293
2 	xul.dll 	nsLayoutUtils::GetFontMetricsForFrame 	layout/base/nsLayoutUtils.cpp:2163
3 	xul.dll 	nsFontMetrics::MaxAscent 	gfx/src/nsFontMetrics.cpp:222
4 	xul.dll 	nsInlineFrame::ReflowFrames 	layout/generic/nsInlineFrame.cpp:646
5 	xul.dll 	nsInlineFrame::Reflow 	layout/generic/nsInlineFrame.cpp:395
6 	xul.dll 	nsLineLayout::ReflowFrame 	layout/generic/nsLineLayout.cpp:782
7 	xul.dll 	nsLineLayout::BeginLineReflow 	layout/generic/nsLineLayout.cpp:215
8 	xul.dll 	nsBlockFrame::ReflowInlineFrames 	layout/generic/nsBlockFrame.cpp:3377
...

Frame 	Module 	Signature 	Source
0 	xul.dll 	gfxFontFamily::HasItalicFace 	gfx/thebes/gfxFont.h:665
1 	xul.dll 	gfxGDIFont::Initialize 	gfx/thebes/gfxGDIFont.cpp:293
2 	xul.dll 	nsStyleContext::DoGetStyleFont 	layout/style/nsStyleStructList.h:47
3 	xul.dll 	ComputeLineHeight 	layout/generic/nsHTMLReflowState.cpp:2390
4 	xul.dll 	xul.dll@0x10f272f 	
5 	xul.dll 	nsBlockReflowState::nsBlockReflowState 	layout/generic/nsBlockReflowState.cpp:113
6 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:994
...

Frame 	Module 	Signature 	Source
0 	xul.dll 	gfxFontFamily::HasItalicFace 	gfx/thebes/gfxFont.h:665
1 	xul.dll 	gfxGDIFont::Initialize 	gfx/thebes/gfxGDIFont.cpp:293
2 	mozglue.dll 	arena_bin_nonfull_run_get 	memory/mozjemalloc/jemalloc.c:3882
3 	xul.dll 	gfxFont::GetShapedWord<unsigned char> 	gfx/thebes/gfxFont.cpp:2402
4 	xul.dll 	gfxFont::SplitAndInitTextRun<unsigned char> 	gfx/thebes/gfxFont.cpp:2594
5 	gkmedias.dll 	SkDataSet::CreateProc 	gfx/skia/include/core/SkDataSet.h:74
...

Frame 	Module 	Signature 	Source
0 	xul.dll 	gfxFontFamily::HasItalicFace 	gfx/thebes/gfxFont.h:665
1 	xul.dll 	gfxGDIFont::Initialize 	gfx/thebes/gfxGDIFont.cpp:293
2 	xul.dll 	base::Histogram::Accumulate 	ipc/chromium/src/base/histogram.cc:562
3 	xul.dll 	gfxFont::GetShapedWord<unsigned char> 	gfx/thebes/gfxFont.cpp:2402
4 	xul.dll 	BuildTextRunsScanner::BuildTextRunForFrames 	layout/generic/nsTextFrameThebes.cpp:2092
5 	mozjs.dll 	MakeJITScript 	js/src/methodjit/Compiler.cpp:959
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=gfxFontFamily%3A%3AHasItalicFace%28%29
Hmm... it's clearly been happening with earlier builds as well; I see crash reports across a range of Firefox versions. But a spike in 20.0a1/20121211 strongly suggests that bug 816483 has made it more common.

The crashes/stacks suggest we're encountering a font entry with NULL family pointer during GDIFont::Initialize, which means we crash when trying to find out whether the family has any italic faces. I can imagine how bug 816483 has made this more common, because the same font entry might now be used by multiple user font families. If the last-created of those families is deleted, it will clear the entry's family pointer - but other, earlier families might still be alive and try to use it. So basically, we can't rely on the family pointer in user font entries being valid/meaningful any more.

We can avoid the crash here by consistently setting the mFamilyHasItalicFace as needed in all GDI font entries (not only for user fonts), so that Initialize() won't need to refer back to the family. (And it makes sense, actually, that instantiating a gfxFont from its associated font entry should be a "standalone" process that doesn't require knowledge of the family, if any, to which that entry belonged.)

This also has the benefit of eliminating the gfxFontFamily::HasItalicFace() method, which only existed to support GDI's needs, from the general gfxFontFamily class and confining the special italic-management logic to the GDI-specific files, where it more properly belongs.
Attachment #691271 - Flags: review?(roc)
Marking this as blocking 816483 because of the spike in crashiness, even though the signature existed at a low level prior to that. I believe the patch here should eliminate it altogether.
Assignee: nobody → jfkthame
Blocks: 816483
And now with appropriate typecasts to make it actually compile. :) Carrying forward r=roc.
Attachment #691271 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/0f6f0585d019
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
No post-fix crashes in Socorro.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.