Closed Bug 1004202 Opened 11 years ago Closed 11 years ago

crash in nsPrintEngine::ReconstructAndReflow(bool)

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Version:
30 Branch
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla32
Tracking Flags:
Tracking Status
firefox29 --- unaffected
firefox30 --- verified
firefox31 --- verified
firefox32 --- verified
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: martijn.martijn, Assigned: Ms2ger)

References

Details

(4 keywords)

Crash Data

Attachments

(4 files)

parentframe2.htm
1.30 KB, text/html
Details
crash1.htm
757 bytes, text/html
Details
testcase
286 bytes, text/html
Details
Stop calling PrepareToStartLoad in nsDocumentViewer::LoadStart;
1.45 KB, patch
smaug
: review+
lsblakk
: approval-mozilla-aurora+
lsblakk
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached file parentframe2.htm —
See testcase, load the testcase, the print dialog comes up, click on the print button. Result, crash. The iframe consists of this source: <html> <head> <script> setTimeout(function() {window.print();window.location.href=window.location.href;}, 1000); </script> </head> <body onbeforeunload="document.write('test')"> <style> @font-face { font-family: "cutabovetherest"; src: url(""); } * { font-family: "cutabovetherest"; } </style> mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm <div style="page-break-before: avoid;page-break-after: right;"></div> mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm mmmmm </body> </html> This bug was filed from the Socorro interface and is report bp-60ffc46d-d59f-47d7-96e8-176da2140430. ============================================================= 0 XUL nsPrintEngine::ReconstructAndReflow(bool) layout/base/nsPresContext.h 1 XUL nsPrintEngine::SetupToPrintContent() layout/printing/nsPrintEngine.cpp 2 XUL nsPrintEngine::AfterNetworkPrint(bool) layout/printing/nsPrintEngine.cpp 3 XUL nsPrintEngine::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult) layout/printing/nsPrintEngine.cpp 4 XUL _ZThn8_N13nsPrintEngine13OnStateChangeEP14nsIWebProgressP10nsIRequestj12tag_nsresult layout/printing/nsPrintEngine.cpp 5 XUL nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, tag_nsresult) uriloader/base/nsDocLoader.cpp 6 XUL nsDocLoader::FireOnStateChange(nsIWebProgress*, nsIRequest*, int, tag_nsresult) uriloader/base/nsDocLoader.cpp 7 XUL nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) uriloader/base/nsDocLoader.cpp 8 XUL _ZThn8_N11nsDocLoader13OnStopRequestEP10nsIRequestP11nsISupports12tag_nsresult obj-firefox/x86_64/uriloader/base/Unified_cpp_uriloader_base0.cpp 9 XUL nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, tag_nsresult) netwerk/base/src/nsLoadGroup.cpp 10 XUL nsLoadGroup::Cancel(tag_nsresult) netwerk/base/src/nsLoadGroup.cpp
Attached file crash1.htm —
Iframe for testcase
Attached file testcase —
Ok, the first testcase didn't crash, but the last testcase I attached does crash.
Last testcase crashes for me (in linux nightly) with signature [@ nsPrintEngine::InitPrintDocConstruction(bool) ] bp-30b5ac0a-d38a-4055-a93a-047e02140430
Crash: bp-33165a7b-eeae-4cf8-8cff-d1bd92140430 Regression window(m-c) Good: https://hg.mozilla.org/mozilla-central/rev/c71a1f6f6f2f Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140208192902 Bad: https://hg.mozilla.org/mozilla-central/rev/c8cd1f6b6d2d Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140209000603 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c71a1f6f6f2f&tochange=c8cd1f6b6d2d
status-firefox29: --- → unaffected
status-firefox30: --- → affected
status-firefox31: --- → affected
status-firefox32: --- → affected
Keywords: regression, reproducible
OS: Mac OS X → All
Version: Trunk → 30 Branch
In local build, Last Good: 7bec1b07e94f First Bad: d30a9c28e77e Triggered by: d30a9c28e77e Ms2ger — Bug 968805 - Pass nsIDocument to nsIContentViewer.loadStart; r=smaug
Blocks: 968805
Oh, I think I see it... cv->LoadStart(static_cast<nsIHTMLDocument *>(this)); doesn't play well with the v nsIHTMLDocument mDocument == aDocument ^ nsIDocument* check. Not sure how to best solve it; add an aDontPrepareToStartLoad argument?
Flags: needinfo?(Ms2ger)
OTOH, I'm pretty sure the PrepareToStartLoad call was never reached before.
Before bug 968805, none of the callers would reach this branch. The callers in nsContentDLF.cpp call LoadStart immediately after constructing the nsDocumentViewer, so they hit the !mDocument case, and the comparison was always false for the caller in nsHTMLDocument::Open, as it passed an nsIHTMLDocument pointer, so the nsISupports pointers being compared were always different. (XPCOM rules require SameCOMIdentity in this case, to avoid exactly this issue.)
Attachment #8415889 - Flags: review?(bugs)
Assignee: nobody → Ms2ger
Status: NEW → ASSIGNED
With changeset d30a9c28e77e backed out, the removed code isn't reached by attachment 8415595 [details] or anything we run on try.
Attachment #8415889 - Flags: review?(bugs) → review+
Comment on attachment 8415889 [details] [diff] [review] Stop calling PrepareToStartLoad in nsDocumentViewer::LoadStart; https://hg.mozilla.org/integration/mozilla-inbound/rev/e50544d1d32d [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 968805 User impact if declined: crashes when printing Testing completed (on m-c, etc.): full try run + manual testing that the removed code was indeed dead; on mozilla-inbound Risk to taking this patch (and alternatives if risky): fairly low risk; it should revert our behaviour to what it was before bug 968805. Alternative is a backout of the regressing patch, which does change a uuid. String or IDL/UUID changes made by this patch: none
Attachment #8415889 - Flags: approval-mozilla-beta?
Attachment #8415889 - Flags: approval-mozilla-aurora?
Comment on attachment 8415889 [details] [diff] [review] Stop calling PrepareToStartLoad in nsDocumentViewer::LoadStart; Approving for uplift as I see this landed on central: http://hg.mozilla.org/mozilla-central/rev/e50544d1d32d
Attachment #8415889 - Flags: approval-mozilla-beta?
Attachment #8415889 - Flags: approval-mozilla-beta+
Attachment #8415889 - Flags: approval-mozilla-aurora?
Attachment #8415889 - Flags: approval-mozilla-aurora+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-firefox32: affected → fixed
Resolution: --- → FIXED
Keywords: checkin-needed
Keywords: checkin-needed
Target Milestone: --- → mozilla32
Reproduced in Nightly 2014-05-01, Win 7 x64. Verified fixed 32.0a1 (2014-05-06).
Status: RESOLVED → VERIFIED
status-firefox32: fixed → verified
Reproduced the crash using old Nightly (2014-04-30) on Windows 7 64bit, verified that the issue is fixed using Firefox 30 beta 3 and latest Aurora on Mac OS X 10.9.2, Windows 7 64bit and Ubuntu 13.10 32bit.
status-firefox30: fixed → verified
status-firefox31: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: