856784 - crash in gfxUserFontSet::UserFontCache::ForgetFont

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Brendan Dahl [:bdahl]]

This bug was filed from the Socorro interface and is 
report bp-95c5442e-d6bf-4874-9a73-d74c72130401 .
============================================================= 

Multiple crashes while viewing the pdf https://bugzilla.mozilla.org/attachment.cgi?id=731131 from bug 855679

Comment 1

[John Daggett (:jtd)]

Attachment: patch, null-check mUserFontData before use

Simple null-check needed for the bad font case.

In this case the cmap contains funky data, the 19th segment of the format-4 cmap subtable has startCount = 0xffff and endCount = 0xfffe.  Not sure whether this comes from the data embedded within the PDF or not.  If not, may also need to log a bug against pdf.js.

Comment 2

[Jonathan Kew [:jfkthame]]

Comment on attachment 732196 [details] [diff] [review]
patch, null-check mUserFontData before use

Review of attachment 732196 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this makes sense to prevent the potential crash.

Brendan, it does mean there's a font that isn't being loaded (because of the cmap error John describes), and therefore the PDF won't be rendered as intended. So on the pdf.js side, you may want to correct it before trying to stuff it into @font-face.

Comment 3

[Brendan Dahl [:bdahl]]

We'll leave bug 855679 open as the font conversion problem to fix on the pdf.js side of things.

Comment 4

[John Daggett (:jtd)]

Pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/7201a22e3c44

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7201a22e3c44

Comment 6

[Scoobidiver (away)]

Is it possible to uplift this null check to Aurora and Beta?

Comment 7

[John Daggett (:jtd)]

Comment on attachment 732196 [details] [diff] [review]
patch, null-check mUserFontData before use

[Approval Request Comment]
Bug caused by (feature/regressing bug #): unloading downloadable fonts
User impact if declined: crash
Testing completed (on m-c, etc.): patch includes crashtest
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none

Comment 8

[bhavana bajaj [:bajaj]]

Comment on attachment 732196 [details] [diff] [review]
patch, null-check mUserFontData before use

low risk, null check.Approving for uplift.

Thanks for the test!

Comment 9

[John Daggett (:jtd)]

Pushed to aurora/beta:
https://hg.mozilla.org/releases/mozilla-aurora/rev/b5d85c32501f
https://hg.mozilla.org/releases/mozilla-beta/rev/f61bd1da1a5e

Comment 10

[Mihai Morar, (:MihaiMorar)]

I'm not able to reproduce this issue on FF 21b3 on Mac OS 18.8:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0
(20130416200523)

There are only 6 crashes on 21b2 brach via soccoro and probably they appeared after 21b2 was landed and before push from Comment 9 was done.

Product	Version  	Percentage   Number Of Crashes
Firefox	 20.0	        32.075 %	   17
Firefox	 22.0a2	        32.075 %	   17
Firefox	 22.0a1	        11.321 %	    6
Firefox	 21.0b2	        11.321 %	    6

Comment 11

[Ioana (away)]

This issue is either intermittent, or it can only be reproduced under conditions not specified here - I couldn't reproduce it on Mac 10.7.5 and Mac 10.8.3 with Firefox 20 (affected version with no fix).

There are four post-fix crashes in Socorro (all on Fx 22.0a2):
https://crash-stats.mozilla.com/report/list?query_search=signature&query_type=contains&reason_type=contains&range_value=1&range_unit=weeks&hang_type=any&process_type=any&signature=gfxUserFontSet%3A%3AUserFontCache%3A%3AForgetFont%28gfxFontEntry%2A%29

If anyone can reproduce this bug, please verify it on Firefox 21 beta 3 or later.

Comment 12

[John Daggett (:jtd)]

(In reply to Ioana Budnar [QA] from comment #11)
> This issue is either intermittent, or it can only be reproduced under
> conditions not specified here - I couldn't reproduce it on Mac 10.7.5 and
> Mac 10.8.3 with Firefox 20 (affected version with no fix).

The patch includes a crashtest that will produce the desired crash.

Comment 13

[Ioana (away)]

(In reply to John Daggett (:jtd) from comment #12)
> The patch includes a crashtest that will produce the desired crash.

Thanks for the hint. I can reproduce the crash on Fx20 with your test case and I could verify this fix on Fx21 beta 3 - Mac OSX 10.8.3.

I won't mark the bug back as verifyme though, considering that it will be verified by an automated test.