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

RESOLVED FIXED in Firefox 51

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: philipp, Assigned: bobowen)

Tracking

({crash, regression})

51 Branch
mozilla53
x86
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 wontfix, firefox51 fixed, firefox52 fixed, firefox53 fixed)

Details

(crash signature)

Attachments

(2 attachments)

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
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:
https://hg.mozilla.org/releases/mozilla-beta/annotate/9afe68360fa8/gfx/2d/RecordedEvent.cpp#l1647

Bug 1320863 might also be related to these issues.
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
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 bobowencode@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fe29922e32c
Add null check and logging on fontResource in RecordedScaledFontCreation::PlayEvent. r=milan
https://hg.mozilla.org/mozilla-central/rev/7fe29922e32c
Status: ASSIGNED → RESOLVED
Closed: 3 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?]:
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.
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)
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 ?
Flags: needinfo?(bobowencode)
Flags: needinfo?(bobowencode)
You need to log in before you can comment on or make changes to this bug.