Closed Bug 1203610 Opened 5 years ago Closed 5 years ago

Crashes at nsLayoutUtils::ShouldUseNoScriptSheet(nsIDocument*) printing very large documents

Categories

(Core :: Printing: Output, defect)

41 Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla44
Tracking Status
firefox41 --- wontfix
firefox42 + verified
firefox43 + verified
firefox44 --- verified

People

(Reporter: smichaud, Assigned: heycam)

References

()

Details

(Keywords: crash, regression, reproducible)

Crash Data

Attachments

(1 file)

Example:

bp-944dbb40-1f33-4034-9e6f-94bee2150910

I find these crashes 100% reproducible, on both OS X and Windows, trying to print https://html.spec.whatwg.org/, which is the WHATWG HTML spec, "one-page version".  (See also https://html.spec.whatwg.org/multipage/.)  The crashes happen with e10s on or off, before any print dialog appears.

Regression range:

firefox-2015-06-16-03-02-01-mozilla-central
firefox-2015-06-17-03-02-05-mozilla-central

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ce863f9d8864&tochange=d7c148c84594
Crash Signature: [@ nsLayoutUtils::ShouldUseNoScriptSheet(nsIDocument*) ]
I'd bet one of the patches for bug 1169514 is the cause/trigger here.
Blocks: 1169514
Keywords: crash
[Tracking Requested - why for this release]:

This isn't a topcrasher on either OS X or Windows.  But it *is* 100% reproducible.
:heycam, want to take a look? 
Not high volume but recent regression from 41.
Flags: needinfo?(cam)
Thanks Steven and Liz; I will look at this in a week and a bit.
Assignee: nobody → cam
Duplicate of this bug: 1207960
(In reply to Loic from comment #8)
> Simple STR: printing this SVG
> https://bugzilla.mozilla.org/attachment.cgi?id=8665305
> 
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=4a71f48a4e2ca3a7452d37c7aa55f9b209d3c161&tochange=7ae3
> e54ebb633e8895ce987a5934613303360eaa

The issue here is that we're under nsIDocument::CreateStaticClone, cloning the SVG document for printing, and we call SVGDocument::EnsureNonSVGUserAgentStyleSheetsLoaded from SVGDocument::InsertChildAt.  After bug 1169514, EnsureNonSVGUserAgentStyleSheetsLoaded calls nsLayoutUtils::ShouldUseNoScriptSheet which wants to get the document's mOriginalDocument, but that is only set after the entire document cloning process has finished, back up in CreateStaticClone.
Flags: needinfo?(cam)
We should be able to skip loading the on-demand non-SVG UA sheets here, since CreateStaticClone will clone the original document's loaded sheets after the document has been cloned.  There is a comment in SVGDocument::InsertChildAt about needing to do this now, but I tried printing some SVG documents (with a non-SVG root, like attachment 8665305 [details], and with an SVG root but with foreignObject HTML content in it) and it seemed to work.
Attached patch patchSplinter Review
Attachment #8672431 - Flags: review?(jwatt)
This fixes the HTML spec printing too.
Status: NEW → ASSIGNED
Crash Signature: [@ nsLayoutUtils::ShouldUseNoScriptSheet(nsIDocument*) ] → [@ nsLayoutUtils::ShouldUseNoScriptSheet(nsIDocument*) ] [@ nsLayoutUtils::ShouldUseNoScriptSheet ]
Cameron, do you know someone else who could review the patch? Thanks
Flags: needinfo?(cam)
Comment on attachment 8672431 [details] [diff] [review]
patch

Sorry for the delay.
Flags: needinfo?(cam)
Attachment #8672431 - Flags: review?(jwatt) → review+
Cameron, do you think this could be uplifted ? (probably in beta 9 on Thursday)
Flags: needinfo?(cam)
Keywords: checkin-needed
Comment on attachment 8672431 [details] [diff] [review]
patch

Yes I think we should uplift.

Approval Request Comment
[Feature/regressing bug #]: bug 1169514
[User impact if declined]: crashes when printing SVG documents
[Describe test coverage new/current, TreeHerder]: manually tested only; just landed on inbound
[Risks and why]: low; we just skip loading some style sheets that we know we will be loaded soon elsewhere
[String/UUID change made/needed]: N/A
Flags: needinfo?(cam)
Attachment #8672431 - Flags: approval-mozilla-beta?
Attachment #8672431 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d61b658a2a1a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Comment on attachment 8672431 [details] [diff] [review]
patch

Fix a crash, taking it.
Should be in 42 beta 9.
Attachment #8672431 - Flags: approval-mozilla-beta?
Attachment #8672431 - Flags: approval-mozilla-beta+
Attachment #8672431 - Flags: approval-mozilla-aurora?
Attachment #8672431 - Flags: approval-mozilla-aurora+
Duplicate of this bug: 1216052
Flags: qe-verify+
I was able to reproduce this issue on Firefox 44.0a1 (2015-09-25) using Windows 7 64-bit.

Verified fixed on Firefox 44.0a1 (2015-10-22/23), Firefox 43.0a2 (2015-10-22/23) and Firefox 42 Beta 9 under Windows 7 64-bit and Mac OS X 10.10.4.

This issue could not be verified on Firefox 44.0a1 with e10s enabled under Mac OS X because of https://bugzilla.mozilla.org/show_bug.cgi?id=1217190
Status: RESOLVED → VERIFIED
Reproduced using Firefox 41.0.1.

Verified as fixed with Firefox 44 beta 8 under Win 10 (x64) and Mac OS X 10.10.5.
Also, there are no new reports in crash-stats, so marking as verified.
You need to log in before you can comment on or make changes to this bug.