101221 - regression: cancelling printing a page crashes - Trunk [@ nsDeviceContextWin::GetDeviceContextFor]

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[Olivier Cahagne]

Build ID: 2001092208 on Win2k.

Steps to reproduce:
 - load a page (whatever page), for me 'about:' and 'http://www.mozilla.org/'
trigger the bug,
 - Click on the 'Print' icon,
 - Cancel the print by clicking on 'Cancel' (Annuler in french, see attachment),
 - Mozilla crashes.

Expected behaviour: should not crash.
Talkback ID: TB35771679M.
Related bug: bug 93509 (which might be reopened ?).

Related details: I have a HP Laserjet but I don't think you even need a printer
as you could save it to file.
OS -> All as people on #mozillazine could reproduce the crash with recent build
on Linux.

Comment 1

[Olivier Cahagne]

Attachment: Screenshot of printing window, where I hit Cancel ('Annuler')

Comment 2

[Matthias Versen [:Matti]]

win2k stack trace (build 20010922)

nsDeviceContextWin::GetDeviceContextFor(nsDeviceContextWin * const 0x04cdea08, 
nsIDeviceContextSpec * 0x00000000, nsIDeviceContext * & 0x04d770e8) line 703 + 3 
bytes
DocumentViewerImpl::Print(DocumentViewerImpl * const 0x03c33f48, int 0, _iobuf * 
0x00000000, nsIPrintListener * 0x00000000) line 4395 + 69 bytes
GlobalWindowImpl::Print(GlobalWindowImpl * const 0x04cdee94) line 1872 + 29 
bytes
XPTC_InvokeByIndex(nsISupports * 0x04cdee94, unsigned int 69, unsigned int 0, 
nsXPTCVariant * 0x0012d5e0) line 139
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode 
CALL_METHOD) line 1952 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x04af89e0, JSObject * 0x04c55758, unsigned int 0, 
long * 0x04e4b0a8, long * 0x0012d818) line 1254 + 14 bytes
js_Invoke(JSContext * 0x04af89e0, unsigned int 0, unsigned int 0) line 807 + 23 
bytes
js_Interpret(JSContext * 0x04af89e0, long * 0x0012e5bc) line 2719 + 15 bytes
js_Invoke(JSContext * 0x04af89e0, unsigned int 1, unsigned int 2) line 824 + 13 
bytes
js_InternalInvoke(JSContext * 0x04af89e0, JSObject * 0x03c6fb78, long 63372160, 
unsigned int 0, unsigned int 1, long * 0x0012e79c, long * 0x0012e6e4) line 899 + 
20 bytes
JS_CallFunctionValue(JSContext * 0x04af89e0, JSObject * 0x03c6fb78, long 
63372160, unsigned int 1, long * 0x0012e79c, long * 0x0012e6e4) line 3362 + 31 
bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x04af84d8, void * 0x03c6fb78, 
void * 0x03c6fb80, unsigned int 1, void * 0x0012e79c, int * 0x0012e798, int 0) 
line 960 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x04b9dca8, nsIDOMEvent 
* 0x04d75304) line 139 + 74 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x04b3a440, 
nsIDOMEvent * 0x04d75304, nsIDOMEventTarget * 0x04b3a2a8, unsigned int 8, 
unsigned int 7) line 1197 + 20 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x04b9dc40, 
nsIPresContext * 0x04af3808, nsEvent * 0x0012f2c4, nsIDOMEvent * * 0x0012f170, 
nsIDOMEventTarget * 0x04b3a2a8, unsigned int 7, nsEventStatus * 0x0012f310) line 
2187 + 36 bytes
nsXULElement::HandleDOMEvent(nsXULElement * const 0x04b3a2a0, nsIPresContext * 
0x04af3808, nsEvent * 0x0012f2c4, nsIDOMEvent * * 0x0012f170, unsigned int 1, 
nsEventStatus * 0x0012f310) line 3711
PresShell::HandleDOMEventWithTarget(PresShell * const 0x04b085b8, nsIContent * 
0x04b3a2a0, nsEvent * 0x0012f2c4, nsEventStatus * 0x0012f310) line 5755 + 39 
bytes
nsMenuFrame::Execute() line 1537
nsMenuFrame::HandleEvent(nsMenuFrame * const 0x04d03488, nsIPresContext * 
0x04af3808, nsGUIEvent * 0x0012f768, nsEventStatus * 0x0012f65c) line 459
PresShell::HandleEventInternal(nsEvent * 0x0012f768, nsIView * 0x04d064a0, 
unsigned int 1, nsEventStatus * 0x0012f65c) line 5723 + 41 bytes
PresShell::HandleEvent(PresShell * const 0x04b085bc, nsIView * 0x04d064a0, 
nsGUIEvent * 0x0012f768, nsEventStatus * 0x0012f65c, int 0, int & 1) line 5633 + 
25 bytes
nsView::HandleEvent(nsView * const 0x04d064a0, nsGUIEvent * 0x0012f768, unsigned 
int 8, nsEventStatus * 0x0012f65c, int 0, int & 1) line 377
nsView::HandleEvent(nsView * const 0x04cf8d90, nsGUIEvent * 0x0012f768, unsigned 
int 8, nsEventStatus * 0x0012f65c, int 0, int & 1) line 350
nsView::HandleEvent(nsView * const 0x04d58910, nsGUIEvent * 0x0012f768, unsigned 
int 8, nsEventStatus * 0x0012f65c, int 0, int & 1) line 350
nsView::HandleEvent(nsView * const 0x04b08130, nsGUIEvent * 0x0012f768, unsigned 
int 28, nsEventStatus * 0x0012f65c, int 1, int & 1) line 350
nsViewManager::DispatchEvent(nsViewManager * const 0x04b08008, nsGUIEvent * 
0x0012f768, nsEventStatus * 0x0012f65c) line 2062
HandleEvent(nsGUIEvent * 0x0012f768) line 68
nsWindow::DispatchEvent(nsWindow * const 0x04cf8e4c, nsGUIEvent * 0x0012f768, 
nsEventStatus & nsEventStatus_eIgnore) line 728 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f768) line 749
nsWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 4262 + 
21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 
4514
nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 11665489, long * 
0x0012fb94) line 3221 + 24 bytes
nsWindow::WindowProc(HWND__ * 0x002c0434, unsigned int 514, unsigned int 0, long 
11665489) line 996 + 27 bytes
USER32! 77e02e98()
USER32! 77e030e0()
USER32! 77e05824()
nsAppShellService::Run(nsAppShellService * const 0x00f85280) line 442
main1(int 2, char * * 0x003577d0, nsISupports * 0x00000000) line 1278 + 32 bytes
main(int 2, char * * 0x003577d0) line 1606 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87d08()

Comment 3

[Roland Mainz]

Reporter, could you please post a stack trace from a
Linux/Solaris/AIX/HP-UX/etc. build, please ?

Comment 4

[Roland Mainz]

*** Bug 101299 has been marked as a duplicate of this bug. ***

Comment 5

[Luca Fasolo]

confirmed on Win NT, build 2001092308

Talkback incident ID TB35797417Z

Comment 6

[Roland Mainz]

This bug is a silly one:
nsDeviceContextSpecFactoryWin::CreateDeviceContextSpec() returns NS_OK even when
the print dialog was _canceled_ or something else went wrong there.
(This is partially my "fault" because I fixed error checking in bug 97907 and
assumed that every platform properly returns an error in these cases... but the
braindead code in gfx/src/windows/ doesn't do that... ouch...).

Solution:
If "BOOL result = ::PrintDlg(&prntdlg);" returns |FALSE|
nsDeviceContextSpecFactoryWin::CreateDeviceContextSpec() should return
NS_ERROR_ABORT instead of NS_OK (patch in bug 101289 will fix the anoying error
dialog for this case).

Unfortunately I do not have a Win32 build machine, I can't help much... ;-(

Comment 7

[Roland Mainz]

Attachment: (Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

Comment 8

[Roland Mainz]

OK... filed a patch which _should_ fix this... however I cannot test it due lack
of a matching build machine.

Comment 9

[Roland Mainz]

CC:'ing some printing staff, AFAIK dcone is on vacation ...

Can anyone please test attachment 50512 [details] [diff] [review] on a Win32 box, please ?

Comment 10

[Bernd]

I tried the patch under Win98  and ended in an infinite (more then 50 times)
loop of the following assertion
###!!! ASSERTION: nsFont name wasn't lowercase: 'name.Equals(aOther.name)', file
 C:\MOZ_SOUR\MOZILLA\mozilla\gfx\src\nsFont.cpp, line 56

Comment 11

[Roland Mainz]

Bernd Mielke wrote:
> I tried the patch under Win98  and ended in an infinite (more then 50 times)
> loop of the following assertion
> ###!!! ASSERTION: nsFont name wasn't lowercase: 'name.Equals(aOther.name)', 
> file
> C:\MOZ_SOUR\MOZILLA\mozilla\gfx\src\nsFont.cpp, line 56

No no no... this is caused by patch for bug 97299 ("Store font names in
lowercase to avoid conversions"). The patch was backed-out few hours ago.
IMHO totally unrelated to this one...

Comment 12

[Bernd]

The patch works, win98 I just updated my tree.

Comment 13

[Jay Patel [:jay]]

Adding topcrash keyword and Trunk [@ nsDeviceContextWin::GetDeviceContextFor] to
summary...this is a topcrasher on the MozillaTrunk:

nsDeviceContextWin::GetDeviceContextFor   13 
BBID range: 35720805 - 35789477
Min/Max Seconds since last crash: 9 - 10561
Min/Max Runtime: 23 - 93989
Crash data range: 2001-09-21 to 2001-09-23
Build ID range: 2001092114 to 2001092310

Stack Trace: 

	 nsDeviceContextWin::GetDeviceContextFor
[d:\builds\seamonkey\mozilla\gfx\src\windows\nsDeviceContextWin.cpp  line 705]
	 DocumentViewerImpl::Print
[d:\builds\seamonkey\mozilla\content\base\src\nsDocumentViewer.cpp  line 4397]
	 GlobalWindowImpl::Print
[d:\builds\seamonkey\mozilla\dom\src\base\nsGlobalWindow.cpp  line 1873]
	 XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp 
line 139]
	 XPCWrappedNative::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp  line 1954]
	 XPC_WN_CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp 
line 1255]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 809]
	 js_Interpret
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2720]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 825]
	 js_InternalInvoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 900]
	 JS_CallFunctionValue
[d:\builds\seamonkey\mozilla\js\src\jsapi.c  line 3364]
	 nsJSContext::CallEventHandler
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp  line 963]
	 nsJSEventListener::HandleEvent
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp  line 140]
	 nsEventListenerManager::HandleEventSubType
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1198]
	 nsEventListenerManager::HandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1368]
	 nsGenericElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericElement.cpp  line 1805]
	 nsGenericHTMLElement::HandleDOMEventForAnchors
[d:\builds\seamonkey\mozilla\content\html\content\src\nsGenericHTMLElement.cpp 
line 1254]
	 nsHTMLAnchorElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\html\content\src\nsHTMLAnchorElement.cpp 
line 400]
	 nsGenericDOMDataNode::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericDOMDataNode.cpp  line 706]
	 nsDOMDocumentType::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsDOMDocumentType.cpp  line 227]
	 PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5710]
	 PresShell::HandleEventWithTarget
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5680]
	 nsEventStateManager::CheckForAndDispatchClick
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line 2487]
	 nsEventStateManager::PostHandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line 1573]
	 PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5731]
	 PresShell::HandleEvent
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5635]
	 nsView::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 377]
	 nsView::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 350]
	 nsView::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 350]
	 nsViewManager::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp  line 2062]
	 HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 68]
	 nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 732]
	 nsWindow::DispatchWindowEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 749]
	 nsWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4264]
	 ChildWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4514]
	 nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 3251]
	 nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 997]
	 KERNEL32.DLL + 0x3613 (0xbff63613)
	 KERNEL32.DLL + 0x248f7 (0xbff848f7)
	 0x00688b5e
	 0x00058f64
	 0x04000000
 
 	Source File :
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/gfx/src/windows/nsDeviceContextWin.cpp
line : 705
     (35777245)	URL: http://www.mozilla.org/
(35777245)
Comments: bug 101221
     (35772044)	Comments: print cancel crash bug 101221
     (35771985)	URL: http://www.mozilla.org/
(35771985)
Comments: bug 101221
     (35771865)	Comments: menu -> file -> print  cancel dialog   crash
     (35771679)	URL: http://www.mozilla.org/
(35771679)
Comments: Cancelling printing
     (35771627)	Comments: GKGFXWIN.DLL print cancel
     (35771600)	URL: http://www.mozilla.org/
(35771600)
Comments: loading about:   trying to Print and cancelling
     (35771577)	URL: http://www.mozilla.org/
(35771530)
URL: http://www.mozilla.org/
(35771530)
Comments: Cancelling printing crashes...
     (35720805)	Comments: Press 'Cancel' button to close print dialog

Roland:  This particular stack signature is only happening on Win32...do we know
if this is also happening on Linux?

Comment 14

[Roland Mainz]

> Roland:  This particular stack signature is only happening on Win32...do we 
> know if this is also happening on Linux?

I doubt it crashes on Linux. I am running Solaris and it works here perfectly
(except bug 101289 - but that does not crash... it's only silly... :-)
And I am monitoring all new bug reports... no reports from any Unix/Linux
platforms about crashes yet.

This bug is caused by broken error checking in Windows-only code... that's
all...

Comment 15

[Roland Mainz]

Can anyone give an r=/sr= for that patch here, please ?

Comment 16

[kinmoz]

While the fix prevents the crash ... it has the annoying side affect of popping 
up an error dialog when there shouldn't be one.

Why is it considered an error if I decide to hit the cancel button?

You should probably keep the windows CreateDeviceContextSpec() method the way it 
is, return NS_OK, and roll back some of the changes you made 
to DocumentViewerImpl::Print() ... in particular put back the (nsnull != 
devspec) check:

    if (NS_SUCCEEDED(rv) && nsnull != devspec)

after the call to factory->CreateDeviceContextSpec() and restore the else clause 
at the bottom of the method that stops the print timer and ends printing to the 
way it was, I believe that has to execute regardless of the error returned ... 
you can still add a check for errors after that code executes to throw up a 
dialog if there really was an error.

Does mac crash too? I didn't see anywhere where they were returning 
NS_ERROR_ABORT.

Comment 17

[kinmoz]

By the way, I forgot to mention, the other reason why we shouldn't throw a 
dialog up when canceling is because it puts up a dialog you can't dismiss in the 
viewer test app which Layout guys use for testing layout and printing.

Comment 18

[Roland Mainz]

Comment on attachment 50512 [details] [diff] [review]
(Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

kin wrote:
> While the fix prevents the crash ... it has the annoying side affect of 
> popping up an error dialog when there shouldn't be one.
[etc. etc. etc.]

kin... I already have a fix for that issue. 
That's bug 101289 ('"Canceling" the print dialog fires-up a print error dialog') and it includes a working fix.
AFAIK no need to delay this one... :-)

Comment 19

[kinmoz]

What about the mac?

Comment 20

[Roland Mainz]

> Does mac crash too? 

Looking at the source... ... no... it looks that it correctly uses the error
codes.

> I didn't see anywhere where they were returning 
> NS_ERROR_ABORT.

I see... it looks that it returns NS_ERROR_FAILURE. We need a Mac expert to
plug-in a one-line fix for that in the right location (somewhere in
nsDeviceContextSpecMac::Init()).

Comment 21

[Roland Mainz]

Mac code simply needs a 
-- snip --
theResult = NS_ERROR_ABORT;
-- snip --
in the right place... that's all. Patch in bug 101289 will then suppress the
"you pressed the 'cancel' button"-alert ... :-)

Comment 22

[Roland Mainz]

> if (NS_SUCCEEDED(rv) && nsnull != devspec)
> after the call to factory->CreateDeviceContextSpec()

I don't like this code because it usually hides bugs. Think about (currently)
broken Win32 code which returns NS_OK in our case... which makes it later
impossible to get the cause of the error (and we don't call the cleanup code
which usually leads to other hard-to-hunt-down issues).
Thats why I removed that thing and cleaned-up error checking (unfortunately I
did not check what happens if someone selects the CANCEL button of the native
Win32 print dialog... ;-(( ).
I prefer to kill the real sources of a bug instead trying to hide it with
workarounds...

> and restore the else 
> clause at the bottom of the method that stops the print timer and ends 
> printing to the way it was, I believe that has to execute regardless of the 
> error returned ... 

No... that code must only be executed if an error occured. Otherwise havoc will
occur which ends in random coredumps... ;-(

Comment 23

[kinmoz]

Comment on attachment 50512 [details] [diff] [review]
(Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

Ok, so that's an sr=kin on the win32 patch,
your patch for bug 101289, covers gtk/xlib, so
just get someone to help you verify/fix mac so
all 3 platforms are in sync with their error
throwing/handling.

Comment 24

[Roland Mainz]

> Ok, so that's an sr=kin on the win32 patch,

thanks!
sujay/jepatel/rods/etc. ... can anyone give me a r= for that patch and check it
in, please (I don't have CVS write access) ?

> your patch for bug 101289, covers gtk/xlib, so
> just get someone to help you verify/fix mac so
> all 3 platforms are in sync with their error
> throwing/handling.

Any idea who can help ?

Comment 25

[rods (gone)]

*** Bug 101514 has been marked as a duplicate of this bug. ***

Comment 26

[karnaze (gone)]

Comment on attachment 50512 [details] [diff] [review]
(Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

r=karnaze

Comment 27

[karnaze (gone)]

Adding nsbranch+. Please don't close this bug until it is decided if it will 
make it into the m0.9.4 branch.

Comment 28

[Kevin McCluskey (gone)]

Reassigning to Rod.

Comment 29

[msanz]

adding PDT for review

Comment 30

[rods (gone)]

The 9.4 branch doesn't suffer from this unless the other changes get checked in.

Comment 31

[Tuukka Tolvanen (sp3000)]

*** Bug 101693 has been marked as a duplicate of this bug. ***

Comment 32

[rods (gone)]

Checked in on tip

it wouldn't hurt to check this in on the branch, its low risk

Comment 33

[kinmoz]

Note to PDT: This isn't a problem on the 0.9.4 branch because the print error
dialog changes that caused this regression/crash are only checked into the TRUNK
as rod said above.

Comment 34

[Kevin McCluskey (gone)]

Marking nsbranch- since it does not affect the branch.

Comment 35

[Kevin McCluskey (gone)]

Closing this bug as fixed.

Comment 36

[Olivier Cahagne]

*** Bug 101901 has been marked as a duplicate of this bug. ***

Comment 37

[sujay]

verified on 10/25 trunk.