1308394 - [e10s] OOM crash when trying to print SVG file

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Alex]

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.

Comment 1

[Loic]

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.

Comment 2

[Marco Castelluccio [:marco]]

I believe this is a regression in 50: https://crash-stats.mozilla.com/search/?signature=%3Dmemmove%20%7C%20std%3A%3Avector%3CT%3E%3A%3A_Reallocate&product=Firefox&date=%3E%3D2016-07-01T18%3A59%3A00.000Z&date=%3C2016-10-07T18%3A59%3A00.000Z&_sort=-date&_facets=signature&_facets=version&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-version.

Comment 4

[Marco Castelluccio [:marco]]

Loic, could you verify if 49 is affected?

Comment 5

[Marco Castelluccio [:marco]]

Perhaps you or Alice could run mozregression and find out the regression range?

Comment 6

[Alice0775 White]

Setting following prefs fixes the crash

print.print_via_parent = false
security.sandbox.content.level = 0

Comment 7

[Alice0775 White]

I cannot print web page to file until landing Bug 1156742.
And the crash is since Bug 1156742.

Comment 8

[Loic]

(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

Comment 9

[Marco Castelluccio [:marco]]

(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?

Comment 10

[Loic]

No crash with print.print_via_parent=false.

Comment 11

[Ethan Lin[:ethlin]]

Attachment: use PathBuilder's backend type

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.

Comment 12

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

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

Comment 13

[Marco Castelluccio [:marco]]

[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.

Comment 14

[Ethan Lin[:ethlin]]

Attachment: use PathBuilder's backend type (carry r+: bas)

Rebase the patch.

Comment 15

[Ethan Lin[:ethlin]]

try result looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=43ccebf1be5d&selectedJob=29907731

Comment 16

[Ethan Lin[:ethlin]]

Attachment: add crashtest

I put the svg to crashtest with reftest-print tag.

Comment 17

[Ethan Lin[:ethlin]]

Attachment: add crashtest

Sorry, I forgot to modify crashtest.list. This patch should be correct.

Comment 18

[Ethan Lin[:ethlin]]

try looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ec1ff30f366&selectedJob=30002887

Comment 19

[Pulsebot]

Pushed by [e-mail]:
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

Comment 20

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5aea6f637102
https://hg.mozilla.org/mozilla-central/rev/128a3d8cea98

Comment 21

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

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!

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Do you think this would be good to uplift to aurora 51? We could still take the patch in aurora if you would like.

Comment 23

[Ethan Lin[:ethlin]]

It would be good to uplift to aurora 51. I'll upload a patch for aurora.

Comment 24

[Ethan Lin[:ethlin]]

Attachment: patch for aurora

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.

Comment 25

[Gerry Chang [:gchang]]

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.

Comment 26

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f10cd5ee7c4e

Comment 27

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

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!

Comment 28

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

We could take this in 50.1.0 since it's a crash fix that was verified on 51. Tracked for 50+

Comment 29

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 30

[Ethan Lin[:ethlin]]

I think it's fine to let it ride the 51 train to release since the crash volume is low.