1321522 - Crash in mozilla::gfx::RecordedScaledFontCreation::PlayEvent

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-bd0892e6-2a5c-42cf-afe3-0eaf82161201.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::gfx::RecordedScaledFontCreation::PlayEvent(mozilla::gfx::Translator*) 	gfx/2d/RecordedEvent.cpp:1647
1 	xul.dll 	mozilla::layout::PrintTranslator::TranslateRecording(std::basic_istream<char, std::char_traits<char> >&) 	layout/printing/PrintTranslator.cpp:60
2 	xul.dll 	mozilla::layout::RemotePrintJobParent::PrintPage(nsCString const&) 	layout/printing/ipc/RemotePrintJobParent.cpp:128

the signature has been around for a while but is jumping up in numbers in 51.0b5, where it's accounting for ~2% of browser crashes in early crash data.

there have only been few changes between 51.0b4 & beta 5:
https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FIREFOX_51_0b4_RELEASE&tochange=FIREFOX_51_0b5_RELEASE
out of it bug 1279699 looks like the most likely cause

Comment 1

[Bob Owen (:bobowen)]

Yes, looks like the creation of the NativeFontResource must be failing earlier on and that is giving us a null fontResource here:
https://hg.mozilla.org/releases/mozilla-beta/annotate/9afe68360fa8/gfx/2d/RecordedEvent.cpp#l1647

Bug 1320863 might also be related to these issues.

Comment 2

[Bob Owen (:bobowen)]

Attachment: Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent

I can't really see how the NativeFontResource is not getting stored without the print aborting, but this should prevent the crash.

Also, not sure if I'm using the gfx logging correctly.


MozReview-Commit-ID: 3nM28xgGLPO

Comment 3

[Bob Owen (:bobowen)]

Even though bug 1279699 was backed out, this was still happening at a lower level before, so it would good to get it fixed and I don't think I need to change any of the flags.

Comment 4

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

Comment on attachment 8816200 [details] [diff] [review]
Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent

The caller is ready for a false return value, so I think this is worth landing.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fe29922e32c
Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent. r=milan

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7fe29922e32c

Comment 7

[Gerry Chang [:gchang]]

Hi :bobowen, 
Do you think this is worth uplifting to Beta51 & Aurora52?

Comment 8

[Bob Owen (:bobowen)]

Comment on attachment 8816200 [details] [diff] [review]
Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent

Yes, just giving it a bit of time on Nightly, but thanks for the reminder.

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 792207 originally, but not actually triggered until bug 1156742.

[User impact if declined]:
Firefox will continue to crash for people who hit this printing issue.

[Is this code covered by automated tests?]:
No.

[Has the fix been verified in Nightly?]:
No.

[Needs manual test from QE? If yes, steps to reproduce]: 
We don't have STR unfortunately.

[List of other uplifts needed for the feature/fix]:
None.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Simple null check with logging.

[String changes made/needed]:
None.

Comment 9

[Gerry Chang [:gchang]]

Comment on attachment 8816200 [details] [diff] [review]
Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent

Fix a crash. Beta51+ & Aurora52+. Should be in 51 beta 10.

Comment 10

[Carsten Book [:Tomcat]]

needs rebasing for uplifts

Comment 11

[Bob Owen (:bobowen)]

Attachment: Aurora/Beta - Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent

MozReview-Commit-ID: 3nM28xgGLPO

Comment 12

[Bob Owen (:bobowen)]

(In reply to Carsten Book [:Tomcat] from comment #10)
> needs rebasing for uplifts

Uplift patch uploaded, thanks.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6735296aff71
Bob, next time, please share the url
and update the status flags.
Btw, I don't see the change on m-b ?

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6735296aff71

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ffbf2dcc32bb