Closed Bug 1321522 Opened 7 years ago Closed 7 years ago

Crash in mozilla::gfx::RecordedScaledFontCreation::PlayEvent


(Core :: Graphics: Text, defect)

51 Branch
Not set



Tracking Status
firefox50 --- wontfix
firefox51 --- fixed
firefox52 --- fixed
firefox53 --- fixed


(Reporter: philipp, Assigned: bobowen)



(Keywords: crash, regression)

Crash Data


(2 files)

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:
out of it bug 1279699 looks like the most likely cause
Flags: needinfo?(bobowencode)
Yes, looks like the creation of the NativeFontResource must be failing earlier on and that is giving us a null fontResource here:

Bug 1320863 might also be related to these issues.
Assignee: nobody → bobowencode
Flags: needinfo?(bobowencode)
See Also: → 1320863
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
Attachment #8816200 - Flags: review?(nical.bugzilla)
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 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.
Attachment #8816200 - Flags: review?(nical.bugzilla) → review+
Pushed by
Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent. r=milan
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Hi :bobowen, 
Do you think this is worth uplifting to Beta51 & Aurora52?
Flags: needinfo?(bobowencode)
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?]:

[Has the fix been verified in Nightly?]:

[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]:

[Is the change risky?]:

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

[String changes made/needed]:
Flags: needinfo?(bobowencode)
Attachment #8816200 - Flags: approval-mozilla-beta?
Attachment #8816200 - Flags: approval-mozilla-aurora?
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.
Attachment #8816200 - Flags: approval-mozilla-beta?
Attachment #8816200 - Flags: approval-mozilla-beta+
Attachment #8816200 - Flags: approval-mozilla-aurora?
Attachment #8816200 - Flags: approval-mozilla-aurora+
needs rebasing for uplifts
Flags: needinfo?(bobowencode)
(In reply to Carsten Book [:Tomcat] from comment #10)
> needs rebasing for uplifts

Uplift patch uploaded, thanks.
Flags: needinfo?(bobowencode)
Bob, next time, please share the url
and update the status flags.
Btw, I don't see the change on m-b ?
Flags: needinfo?(bobowencode)
Flags: needinfo?(bobowencode)
You need to log in before you can comment on or make changes to this bug.