398265 - Crash [@ nsPrintPreviewListener::RemoveListeners()] with <frame> on print preview

Status: VERIFIED FIXED

(Core :: Print Preview, defect, P2)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, when doing print preview on that testcase, I crash with current trunk build after clicking away the "An unknown error occured while printing" dialogs.

This seems to have regressed between 2007-07-01 and 2007-07-02:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-07-01+03&maxdate=2007-07-02+06&cvsroot=%2Fcvsroot
I have no idea what might have caused this.

http://crash-stats.mozilla.com/report/index/9640c868-70e5-11dc-bcee-001a4bd43ed6
0  	nsPrintPreviewListener::RemoveListeners()  	 mozilla/layout/printing/nsPrintPreviewListener.cpp:100
1 	nsPrintData::~nsPrintData() 	mozilla/layout/printing/nsPrintData.cpp:96
2 	nsPrintEngine::CommonPrint(int, nsIPrintSettings*, nsIWebProgressListener*) 	mozilla/layout/printing/nsPrintEngine.cpp:429
3 	nsPrintEngine::PrintPreview(nsIPrintSettings*, nsIDOMWindow*, nsIWebProgressListener*) 	mozilla/layout/printing/nsPrintEngine.cpp:720
4 	DocumentViewerImpl::PrintPreview(nsIPrintSettings*, nsIDOMWindow*, nsIWebProgressListener*) 	mozilla/layout/base/nsDocumentViewer.cpp:3513
5 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
6 	AutoJSSuspendRequest::SuspendRequest() 	mozilla/js/src/xpconnect/src/xpcprivate.h:3318
7 	RtlpAllocateFromHeapLookaside

Comment 1

[Stuart Parmenter]

Attachment: keep us from crashing...

This will fix the crash from happening.  We still end up with an "Unknown print error"

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Where is the mPrintEngine being dropped? If it's because the nsDocumentViewer is going away, then we have more serious problems that could make us crash in other ways...

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I can investigate this.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: proposed patch

I think we want this.
The problem is that we're cleaning up things twice.
One crash goes like this: 
DocumentReadyForPrinting() fails, so FinishPrintPreview() calls 
CleanupOnFailure, which fires nsPrintCompletionEvent/nsRunnable. Then before 
the event is handled, DoCommonPrint returns to CommonPrint which open notices 
the error and opens a dialog. Events get processed, and so 
nsPrintCompletionEvent calls mDocViewerPrint->OnDonePrinting(), which deletes 
the printengine. So next time printengine is used, it is a deleted object.

The patch changes things so that FinishPrintPreview() doesn't call 
CleanupOnFailure. It is up to caller to handle clean-up.
FinishPrintPreview() is called 3 times. One is the case here, where we don't 
want CleanupOnFailure() to be called, one is in ::Observe(), which I'm changing 
in the patch and one is in nsDocumentViewerImpl::Destroy, where printengine is 
anyway destroyed, so doesn't make much sense to clean-up (and possibly show a dialog).
Not sure, but this might be actually a regression from Bug 370966.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 288040 [details] [diff] [review]
proposed patch

Document that callers to FinishPrintPreview are expected to clean up.

Comment 6

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112905 Minefield/3.0b2pre. I don't crash, but I still get the unknown print error on Win XP.

Comment 7

[Martijn Wargers (dead)]

I filed bug 468843 for the unknown print error thing.