1109201 - crash in gfxFontGroup::FamilyFace::SetFont(gfxFont*)

Status: VERIFIED FIXED

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

Description

[Tracy Walker [:tracy]]

[Tracking Requested - why for this release]:

This startup crash has exploded to #2 in volume on 35.0b1.  At this time, it appears to only affect Fx35.  

There were some scattered reports in the Aurora time frame. But as you can see, it has risen sharply in volume on beta1
2014110804 	4 
2014111300 	1 	
2014111900 	1 
2014112500 	1 
2014112800 	63 
2014120116 	2629

This bug was filed from the Socorro interface and is 
report bp-eb0b3277-c492-48d7-b78a-4904d2141203.
=============================================================

Frame 	Module 	Signature 	Source
0 	xul.dll 	gfxFontGroup::FamilyFace::SetFont(gfxFont*) 	gfx/thebes/gfxTextRun.h
1 	xul.dll 	gfxFontGroup::GetFontAt(int) 	gfx/thebes/gfxTextRun.cpp
2 	xul.dll 	gfxFontGroup::GetFirstValidFont() 	gfx/thebes/gfxTextRun.cpp
3 	xul.dll 	gfxFontGroup::InitScriptRun<unsigned char>(gfxContext*, gfxTextRun*, unsigned char const*, unsigned int, unsigned int, int) 	gfx/thebes/gfxTextRun.cpp
4 	xul.dll 	gfxFontGroup::InitTextRun<unsigned char>(gfxContext*, gfxTextRun*, unsigned char const*, unsigned int) 	gfx/thebes/gfxTextRun.cpp
5 	xul.dll 	gfxFontGroup::MakeTextRun(unsigned char const*, unsigned int, gfxTextRunFactory::Parameters const*, unsigned int) 	gfx/thebes/gfxTextRun.cpp
6 	xul.dll 	MakeTextRun<unsigned char>(unsigned char const*, unsigned int, gfxFontGroup*, gfxTextRunFactory::Parameters const*, unsigned int) 	layout/generic/nsTextFrame.cpp
7 	xul.dll 	BuildTextRunsScanner::BuildTextRunForFrames(void*) 	layout/generic/nsTextFrame.cpp
8 	xul.dll 	BuildTextRunsScanner::FlushFrames(bool, bool) 	layout/generic/nsTextFrame.cpp
9 	xul.dll 	BuildTextRuns 	layout/generic/nsTextFrame.cpp

Comment 1

[Lukas Blakk [:lsblakk] use ?needinfo]

Milan - can you help us get some eyes on this?

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Jonathan, something you can help with?

Comment 3

[Jonathan Kew [:jfkthame]]

jdaggett has been working on this code more recently than me, I believe; I think we should ask him to take a look first.

Comment 4

[Robert Kaiser]

This is a startup crash and it's been almost a week, meaning that it probably stranded a lot of beta users with non-working Firefox already. We are losing those people from our testing community!

What's the status here, jdaggett?

Comment 5

[alex_mayorga]

FWIW, my SO's Firefox has this on about:crashes

bp-4823bb7d-24f1-4ed8-b90d-abdfe2141215	15/12/2014	12:42 p.m.

Report ID 	Date Submitted
Let me know if there's anything worth collecting from an affected system.

Comment 6

[John Daggett (:jtd)]

We really need a testcase for this. If the crash is occurring close to startup then it must be caused by trying to create a gfxFont for a system font. But to hit this path the font must somehow be invalid, which is sort of puzzling.

Comment 7

[John Daggett (:jtd)]

Attachment: patch, null-check the font before calling SetFont

Add a simple null-check, which will cause this font to be simply marked as invalid and skipped.

Comment 8

[alex_mayorga]

FWIW 35 crashed like this when loading http://live.nicovideo.jp/watch/lv203465449

Report ID 	Date Submitted
bp-18c157a4-5689-4945-9786-92ac62141223	23/12/2014	01:54 a.m.

Comment 9

[John Daggett (:jtd)]

Pushed to inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/073b4ec79de9

Please leave this bug open as I'm not sure yet whether this will fix the problem or not.

Comment 10

[John Daggett (:jtd)]

Tryserver build including patch:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-9b3c720c5055

Comment 11

[John Daggett (:jtd)]

(In reply to alex_mayorga from comment #8)
> FWIW 35 crashed like this when loading
> http://live.nicovideo.jp/watch/lv203465449

Alex, can you always reproduce this on your machine? If so, could you try with the tryserver build to see if the crash does not occur?

Comment 12

[Robert Kaiser]

(In reply to John Daggett (:jtd) from comment #6)
> We really need a testcase for this. If the crash is occurring close to
> startup then it must be caused by trying to create a gfxFont for a system
> font. But to hit this path the font must somehow be invalid, which is sort
> of puzzling.

I remember from the past that we every now and then had issues with some people having corrupted or damaged fonts installed in the system. It's wise to harden us against that when possible.

Comment 13

[Tracy Walker [:tracy]]

How can one tell if they have damaged/corrupted fonts?

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/073b4ec79de9

Comment 15

[John Daggett (:jtd)]

User reports reproducible testcase:

https://crash-stats.mozilla.com/report/index/42b309d6-caa6-4259-9d72-80d972141223

Reproducable crash (even with clean profile): Firefox ver >= 35
0. Windows Vista 64bit SP2
1. Download and install Google Noto Fonts named "Noto Sans CJK JP Regular". The file name is "NotoSansCJKjp-Regular.otf"(MD5: 96E4076155CD5D760B012360A850BFC0) from (URL removed) .
2. Open "(URL removed)
3. Select "Contents" tab.
4. Select "Default font" as "Noto Sans CJK JP" and apply it.
5. Crash...
6. You cannot start Firefox except for Firefox Safe Mode...

Comment 16

[Alice0775 White]

STR in Bug 1097626
Crash on Aurora36.0a2, Firefox35.0b6.

Comment 18

[Alice0775 White]

Please uplift this to Aurora36.0a2, Firefox35beta, ASAP.

Comment 19

[John Daggett (:jtd)]

Comment on attachment 8540507 [details] [diff] [review]
patch, null-check the font before calling SetFont

Approval Request Comment
[Feature/regressing bug #]: Crash caused by changes made in bug 998869.
[User impact if declined]: in situations where something is wrong with a font, a crash may occur. This occurs at startup so it has the potential to prevent users from using Firefox.
[Describe test coverage new/current, TBPL]: don't have a reproducible testcase yet.
[Risks and why]: the fix is a simple null-check so very minor risk.
[String/UUID change made/needed]: none

Comment 20

[John Daggett (:jtd)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/40c8e653429f
https://hg.mozilla.org/releases/mozilla-beta/rev/5829fa2b0234

Comment 21

[John Daggett (:jtd)]

No crashes in beta/aurora channel builds after patch landed on aurora/beta trees.

Comment 22

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Socorro shows no crashes (with either of the two signatures) with Firefox 35 Beta 8 or older, Firefox 36 Aurora (builds after December 28), or Firefox 37 Nightly (Builds after December 24).

Reproduced the crash in an older Firefox 35 Beta with steps from bug 1097626. Could not reproduce the crash with the same steps for Firefox 35 RC build2, latest Firefox 36 Aurora, or latest Firefox 37 Nightly.