423345 - Crash [@ nsPrintEngine::SetupToPrintContent] when trying to print from print preview with a frameset page

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P2)

Description

[Martijn Wargers (dead)]

Attachment: testcase

This is nr. 29 in the top crasher list:
http://crash-stats.mozilla.com/topcrasher/bybranch/1.9

I saw the url in one of the comments.
See testcase, to reproduce the crash:
- Go to Print Preview, then try to print the page from there.

Not sure when this regressed yet.

http://crash-stats.mozilla.com/report/index/d2a05d02-f37e-11dc-83bd-001a4bd43ed6
0  	nsPrintEngine::SetupToPrintContent()  	 mozilla/layout/printing/nsPrintEngine.cpp:1720
1 	nsPrintEngine::DocumentReadyForPrinting() 	mozilla/layout/printing/nsPrintEngine.cpp:1425
2 	nsPrintEngine::Observe(nsISupports*, char const*, unsigned short const*) 	mozilla/layout/printing/nsPrintEngine.cpp:3126
3 	nsPrintProgress::DoneIniting() 	mozilla/embedding/components/printingui/src/win/nsPrintProgress.cpp:221
4 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
5 	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2369

Comment 1

[Steve England [:stevee]]

Some Notes: I don't have a printer, so I added a new printer that prints to file. The printer I "installed" was an HP 2000C.

I found a range where the testcase goes from 'hanging' me to crashing me. (It doesn't hang me with 99% CPU time, it just gets stuck where the Printing dialog comes up and 'Progress:' doesn't change from 'Preparing...', and the Cancel button is greyed out. If I close the dialog with [x] then I can't close the print preview window or go back to the print dialog, and the printer appears to be stuck 'Printing' even tho the file I'm printing to doesn't change from 0 bytes)

Hang: 20080128_2216_firefox-3.0b3pre.en-US.win32
Crash: 20080128_2301_firefox-3.0b3pre.en-US.win32

Checkins to module PhoenixTinderbox between 2008-01-28 22:16 and 2008-01-28 23:01 : 
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-01-28+22%3A16&maxdate=2008-01-28+23%3A01&cvsroot=%2Fcvsroot

Bug 389358?

Comment 2

[Steve England [:stevee]]

FWIW the 'hanging' with the testcase is reproducible by me using Firefox 2.0 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0)

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

More printing fun for dholbert :(

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to comment #0)
> See testcase, to reproduce the crash:
> - Go to Print Preview, then try to print the page from there.

So, does this only happen when you print from print-preview?  i.e. it won't crash when you print directly?

I'm asking because on Linux / Mac, there's no way to print *from* print-preview.  (And it doesn't reproduce on Linux when I print directly, using a debug build from today.)

Comment 5

[Martijn Wargers (dead)]

Yes, it only happens when printing from print-preview. You mean there is no "Print..." button in the top left of the print preview page in linux?

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to comment #5)
> Yes, it only happens when printing from print-preview.

Ok, thanks.

> You mean there is no
> "Print..." button in the top left of the print preview page in linux?

Nope -- not in FF2 or FF3.  There's just page navigation buttons and a "close" button.

Anyway, I'll try to debug this on my Windows machine soon.

Comment 7

[Martijn Wargers (dead)]

Attachment: semi-automated testcase

This is a semi-automated testcase.
It uses enhanced privileges, so you need to download it to your computer.
It automatically goes into print preview after 1 second, and after 2 seconds, it shows the print dialog.
This should trigger the same crash.

Comment 8

[Martijn Wargers (dead)]

Attachment: semi-automated testcase2

This is probably the same crasher, the testcase is slightly different, but I think this is crashing in the same code.

Comment 9

[Daniel Holbert [:dholbert]]

semi-automated testcase2 is crashing at this line:

http://mxr.mozilla.org/seamonkey/source/layout/printing/nsPrintEngine.cpp#1839
  1839     mPrt->mPrintDC->GetDeviceSurfaceDimensions(pageWidth, pageHeight);

because mPrt->mPrintDC is null.

Comment 10

[Daniel Holbert [:dholbert]]

original testcase and semi-automated testcase1 are both crashing at this line:
http://mxr.mozilla.org/seamonkey/source/layout/printing/nsPrintEngine.cpp#1734
  1734     mPrt->mPrintObject->mPresShell->GetPageSequenceFrame(&seqFrame);
because mPrt->mPrintObject->mPresShell is null.

Note that this is within a 
  if (mIsDoingPrintPreview) {
clause -- so it looks like we're running print-preview code when we should actually be running printing code.

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to comment #10)
> Note that this is within a 
>   if (mIsDoingPrintPreview) {
> clause -- so it looks like we're running print-preview code when we should
> actually be running printing code.

Nevermind -- it looks like that's normal.  When we hit that clause, the print engine is currently engaged in both printing and displaying print-preview, so mIsPrinting and mIsDoingPrintPreview are *both* set.

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: fix v1 [checked in]

This should avoid the place where we're crashing in comment 10, by using mIsCreatingPrintPreview rather than mIsDoingPrintPreview, in the code added in bug 389358.

This makes sense because that code should only be called once, for *setting up* the print preview display, but not after that.   mIsCreatingPrintPreview seems to be a good way to check for this.

Comment 13

[Daniel Holbert [:dholbert]]

fix v1 is trivial -- it just corrects the patch added in bug 389358 so as to not crash, as described in comment 12.

Note that it restores our previous behavior of hanging, rather than crashing, on the first two testcases. (which was the FF2 behavior as well -- see comments 1 and 2.  Hanging isn't great, but it's due to a separate issue which can't be addressed until this crash is fixed.)


FWIW, I'm also pretty sure "semi-automated testcase2" is a different bug -- it crashes in a different area of code, dereferencing something different.  That crash also isn't in code added/called by the patch for bug 389358.  And I just got a crash while loading it on the nightly from 20080120, before bug 389358 landed.

Comment 14

[Daniel Holbert [:dholbert]]

Setting target for beta5, as this is a topcrash (see comment 0) and it's got a trivial/safe fix.

Comment 15

[Daniel Holbert [:dholbert]]

Thanks for the review!  

fix v1 checked in.
Checking in nsPrintEngine.cpp;
/cvsroot/mozilla/layout/printing/nsPrintEngine.cpp,v  <--  nsPrintEngine.cpp
new revision: 1.171; previous revision: 1.170
done

Comment 16

[Daniel Holbert [:dholbert]]

(In reply to comment #13)
> FWIW, I'm also pretty sure "semi-automated testcase2" is a different bug -- it
> crashes in a different area of code, dereferencing something different.  That
> crash also isn't in code added/called by the patch for bug 389358.  And I just
> got a crash while loading it on the nightly from 20080120, before bug 389358
> landed.

Martijn, could you file a follow-up bug on this testcase?

Comment 17

[Martijn Wargers (dead)]

Yeah, I was already afraid it was a different crash, I've now filed bug 424000 for it.

Comment 19

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9pre) Gecko/2008050606 Minefield/3.0pre ID:2008050606 - no crash on testcase

--> Verified fixed