Closed Bug 1203610 Opened 5 years ago Closed 5 years ago
Crashes at ns
Layout Utils::Should Use No Script Sheet(ns IDocument*) printing very large documents
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*) ]
Windows example: bp-efdcc488-5725-4748-a572-d82952150910
[Tracking Requested - why for this release]: This isn't a topcrasher on either OS X or Windows. But it *is* 100% reproducible.
Mac non-e10s example: bp-d3538e87-990e-4caa-8c0d-b2a4d2150910
Windows non-e10s example: bp-5fc6de0a-8605-42cf-841a-3225d2150910
:heycam, want to take a look? Not high volume but recent regression from 41.
Thanks Steven and Liz; I will look at this in a week and a bit.
Assignee: nobody → cam
(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.
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.
This fixes the HTML spec printing too.
Crash Signature: [@ nsLayoutUtils::ShouldUseNoScriptSheet(nsIDocument*) ] → [@ nsLayoutUtils::ShouldUseNoScriptSheet(nsIDocument*) ] [@ nsLayoutUtils::ShouldUseNoScriptSheet ]
Cameron, do you know someone else who could review the patch? Thanks
Comment on attachment 8672431 [details] [diff] [review] patch Sorry for the delay.
Attachment #8672431 - Flags: review?(jwatt) → review+
Cameron, do you think this could be uplifted ? (probably in beta 9 on Thursday)
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
Comment on attachment 8672431 [details] [diff] [review] patch Fix a crash, taking it. Should be in 42 beta 9.
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
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.