[e10s] OOM crash when trying to print SVG file

RESOLVED FIXED in Firefox 51

Status

()

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

People

(Reporter: the.decryptor, Assigned: ethlin)

Tracking

({crash, regression})

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 unaffected, firefox50+ wontfix, firefox51 verified, firefox52 verified)

Details

(crash signature, )

Attachments

(3 attachments, 2 obsolete attachments)

This bug was filed from the Socorro interface and is 
report bp-22375822-8a92-4bcd-a701-319eb2161007.
=============================================================

Trying to print (or preview) the linked SVG file in the latest Nightly build crashes the content process instantly with a OOM error.
I can reproduce it with FF52 on Win 7 when printing the SVG with MS XPS Document Writer.
https://crash-stats.mozilla.com/report/index/02faf51a-2e87-4eec-939b-d4dce2161007

The crash is present in old versions of Firefox with e10s enabled, sometimes it crashes the Print driver host.
Crash Signature: [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | void std::__1::vector<T>::__push_back_slow_path<T>] → [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | void std::__1::vector<T>::__push_back_slow_path<T>] [@ memmove | std::vector<T>::_Reallocate ]
Keywords: crash
OS: Mac OS X → Unspecified
Summary: [OS X] OOM crash when trying to print SVG file → [e10s] OOM crash when trying to print SVG file
Duplicate of this bug: 1308530
Loic, could you verify if 49 is affected?
Perhaps you or Alice could run mozregression and find out the regression range?
Flags: needinfo?(epinal99-bugzilla2)
Flags: needinfo?(alice0775)
Setting following prefs fixes the crash

print.print_via_parent = false
security.sandbox.content.level = 0
Flags: needinfo?(alice0775)
I cannot print web page to file until landing Bug 1156742.
And the crash is since Bug 1156742.
Blocks: 1156742
(In reply to Marco Castelluccio [:marco] from comment #4)
> Loic, could you verify if 49 is affected?

Yes, with prefs:
browser.tabs.remote.autostart=true
print.print_via_parent=true
https://crash-stats.mozilla.com/report/index/cb431e85-bd50-49c8-bae3-643472161008
Flags: needinfo?(epinal99-bugzilla2)
(In reply to Loic from comment #8)
> (In reply to Marco Castelluccio [:marco] from comment #4)
> > Loic, could you verify if 49 is affected?
> 
> Yes, with prefs:
> browser.tabs.remote.autostart=true
> print.print_via_parent=true
> https://crash-stats.mozilla.com/report/index/cb431e85-bd50-49c8-bae3-
> 643472161008

What about with the default prefs?
No crash with print.print_via_parent=false.
See Also: → 1308516
Assignee: nobody → ethlin
The drawtarget is 'DrawTargetRecording', but we'll get real backend(skia/cairo) if we call dt->GetBackendType(). CopyGlyphsToBuilder will convert the PathBuilder to PathBuilderSkia/PathBuilderCairo according to the drawtarget's backend though the PathBuilder is PathBuilderRecording. 
So my fix is getting the backend type from PathBuilder directly. I'm not sure if we should handle Backend::RECORDING in CopyGlyphsToBuilder.
Attachment #8800167 - Flags: review?(mstange)
I encountered this bug while I wanted to print a SVG file( e10s was activated). I used Fx50.0b7 on Windows 7 x32 and Windows 10 x64.



https://crash-stats.mozilla.com/report/index/00131818-0ee9-4924-b75a-8b9342161014
[Tracking Requested - why for this release]: Regression in 50. There aren't many crash reports, but if the fix turns out to be simple we might want to uplift it anyway.
Attachment #8800167 - Flags: review?(mstange) → review?(bas)
Attachment #8800167 - Flags: review?(bas) → review+
Rebase the patch.
Attachment #8800167 - Attachment is obsolete: true
Posted patch add crashtest (obsolete) — Splinter Review
I put the svg to crashtest with reftest-print tag.
Attachment #8805018 - Flags: review?(bas)
Posted patch add crashtestSplinter Review
Sorry, I forgot to modify crashtest.list. This patch should be correct.
Attachment #8805018 - Attachment is obsolete: true
Attachment #8805018 - Flags: review?(bas)
Attachment #8805019 - Flags: review?(bas)
Attachment #8805019 - Flags: review?(bas) → review+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5aea6f637102
In CopyGlyphsToBuilder, use PathBuilder's backend instead of DrawTarget's. r=bas
https://hg.mozilla.org/integration/mozilla-inbound/rev/128a3d8cea98
Add crashtest. r=bas
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5aea6f637102
https://hg.mozilla.org/mozilla-central/rev/128a3d8cea98
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
I reproduced this issue with Fx Fx50.0b7 on Windows 7 x32 and Windows 10 x64.
I can confirm this issue is fixed on nightly. I verified using Fx 52.0a2 (20161103030205) on Windows 10 x64. (both e10s + and e10s -)

Cheers!
Do you think this would be good to uplift to aurora 51? We could still take the patch in aurora if you would like.
Flags: needinfo?(ethlin)
It would be good to uplift to aurora 51. I'll upload a patch for aurora.
Flags: needinfo?(ethlin)
Approval Request Comment
[Feature/regressing bug #]:Bug 1156742 (according to comment 7)
[User impact if declined]: Browser may crash while printing or previewing a page.
[Describe test coverage new/current, TreeHerder]: tested on locally and try server
[Risks and why]: Low. The patch is just changing a way to get correct backend type.
[String/UUID change made/needed]: None.
Attachment #8807440 - Flags: approval-mozilla-aurora?
Comment on attachment 8807440 [details] [diff] [review]
patch for aurora

Fix a crash related to printing or previewing a page. Take it in 51 aurora.
Attachment #8807440 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
See Also: 1308516
I reproduced this issue with Fx Fx51.0a2 (20161102004003) on Windows 10 x64.
I can confirm this issue is fixed on aurora. I verified using Fx 51.0a2 (20161105004017) on Windows 10 x64. (both e10s + and e10s -)

Cheers!
We could take this in 50.1.0 since it's a crash fix that was verified on 51. Tracked for 50+
Hi Ethan, should we uplift this fix in 50.1.0? Or should we let it ride the 51 train to release? The crash volume is low so we can easily skip the 50 train.
Flags: needinfo?(ethlin)
I think it's fine to let it ride the 51 train to release since the crash volume is low.
Flags: needinfo?(ethlin)
Duplicate of this bug: 1323769
You need to log in before you can comment on or make changes to this bug.