131174 - [FIX]crash quitting print dialog with "cancel"

Status: VERIFIED FIXED

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

Description

[Markus Multrus]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.9) Gecko/20020310
BuildID:    2002031008

Reproducible: Always
Steps to Reproduce:
1.load a page
2.enter print dialog while loading another page, which is not displayed yet
(your connection mustn't be too fast ;-))
3.leave dialog with cancel

Talkback IDs: TB3935347Z, TB3935239Y

Comment 1

[sujay]

tracy can you reproduce this one ?

Comment 2

[Tracy Walker [:tracy]]

i could not reproduce this, but the again I could not reproduce the steps. 
(machine and connection are too fast)

Comment 3

[sujay]

Boris, can you reproduce ?

Comment 4

[Boris Zbarsky [:bzbarsky]]

No, I cannot.  Markus, what do you mean by "which is not displayed yet"?  Do you
mean "still connecting to server" or do you mean "laying out page as it's coming
in and partly done, but not all the data is in yet"?

Comment 5

[Markus Multrus]

In my cases: The server is contacted  or the data is transferred ("Transferring
data ..."), but the new page isn't displayed at all, you can still see the old
one. I can reproduce this (64kbit/s) by loading a page completely, trying to
load another page (link, bookmark) and right after that opening print-dialog by
pressing Ctrl-P. 

Comment 6

[Boris Zbarsky [:bzbarsky]]

Markus, thank you.  I misunderstood your original comment... I thought you were
talking about two separate windows.

Steps that crash for me:

1)  Load any web page (bugzilla pages are good, b/c the server is slow)
2)  Click on any link (the "first" and "last" links on a bugzilla bug page are
    good b/c they are slow)
3)  Immediately after the link click (while the old page is still up) hit ctrl-p
4)  Wait for the new page to load.
5)  Cancel the print dialog

GetPrimaryFrameFor() called while FrameManager is being destroyed!
GetPrimaryFrameFor() called while FrameManager is being destroyed!
GetPrimaryFrameFor() called while FrameManager is being destroyed!
GetPrimaryFrameFor() called while FrameManager is being destroyed!
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (runnable)]
nsCOMPtr<nsIPrintSettings>::operator-> (this=0x64)
    at ../../../dist/include/xpcom/nsCOMPtr.h:650
650               NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL
nsCOMPtr with operator->().");

#0  nsCOMPtr<nsIPrintSettings>::operator-> (this=0x64)
    at ../../../dist/include/xpcom/nsCOMPtr.h:650
#1  0x411f086f in DocumentViewerImpl::Print (this=0x8644040,
aPrintSettings=0x873c1a0, 
    aWebProgressListener=0x0) at nsDocumentViewer.cpp:6199
#2  0x4026bb05 in XPTC_InvokeByIndex (that=0x8644050, methodIndex=13, paramCount=2, 
    params=0xbfff7fe0) at xptcinvoke_unixish_x86.cpp:153
#3  0x40aef7e6 in XPCWrappedNative::CallMethod (ccx=@0xbfff8088, mode=CALL_METHOD)
    at xpcwrappednative.cpp:2025
#4  0x40af8227 in XPC_WN_CallMethod (cx=0x82bb798, obj=0x8685208, argc=2, 
    argv=0x8796b14, vp=0xbfff81ac) at xpcwrappednativejsops.cpp:1266
#5  0x400d86a2 in js_Invoke (cx=0x82bb798, argc=2, flags=0) at jsinterp.c:788
#6  0x400f0417 in js_Interpret (cx=0x82bb798, result=0xbfff8b5c) at jsinterp.c:2745
#7  0x400d871d in js_Invoke (cx=0x82bb798, argc=1, flags=2) at jsinterp.c:805

(gdb) frame 0
#0  nsCOMPtr<nsIPrintSettings>::operator-> (this=0x64)
    at ../../../dist/include/xpcom/nsCOMPtr.h:650
650               NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL
nsCOMPtr with operator->().");
(gdb) p mRawPtr
Cannot access memory at address 0x64
(gdb) p this
$1 = (nsCOMPtr<nsIPrintSettings> *) 0x64
(gdb) frame 1
#1  0x411f086f in DocumentViewerImpl::Print (this=0x8644040,
aPrintSettings=0x873c1a0, 
    aWebProgressListener=0x0) at nsDocumentViewer.cpp:6199
6199          mPrt->mPrintSettings->SetIsCancelled(PR_TRUE);
(gdb) p mPrt
$2 = (PrintData *) 0x0
(gdb) p mPrt->mPrintSettings
Cannot access memory at address 0x64

Sounds like we could use a null-check on mPrt

Comment 7

[guanxi]

See incident TB4078846 for another example of this bug
 - 2002031104 / win2k
 - note: i misidentified the bug# in the talkback incident

Comment 8

[guanxi]

Sorry, that's TB4078846Q.  (Forgot the 'Q'.)

Comment 9

[rods (gone)]

Attachment: patch

Must check to make sure the diocument viewer isn't busy when about to do
printing or print preview.

Comment 10

[Kevin McCluskey (gone)]

nsbeta1+

Comment 11

[dcone (gone)]

Comment on attachment 74982 [details] [diff] [review]
patch

r=dcone

Comment 12

[Marc Attinasi]

Comment on attachment 74982 [details] [diff] [review]
patch

sr=attinasi

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 74982 [details] [diff] [review]
patch

a=dbaron, although I think we'd be better off with error text other than "The
browser is busy..."  How about "The browser cannot print right now.  Please try
again when the page has finished loading." or something like that.

(I also wonder whether you're using the old document viewer or the new one, and
whether this is papering over a deeper problem.  However, it seems like
accepting a print command in the middle of a page load could also be ambiguous
since it isn't clear which page the user meant to print.)

Comment 14

[rods (gone)]

Yes, I didn't really like that text either, so I fixed it and gave each it's own
comment.
fixed.

Comment 15

[sujay]

Markus, can you verify this is working for you now? try in today's build..

let us know..thanks!

Comment 16

[Markus Multrus]

WFM with build 2002032021 :) Thanx!

Comment 17

[sujay]

verified per comments.

Comment 18

[rods (gone)]

Work for the Browser but fails for mail because of the order of the listeners.

Comment 19

[rods (gone)]

Comment on attachment 74982 [details] [diff] [review]
patch

This solution doesn't work for mail.

Comment 20

[rods (gone)]

Attachment: new patch

The PP part of the patch is exactly the same.

The original patch worked fine for regular documents but didn't work for mail
because of the ordering of nsIWebProgressListeners. Even tho the doc WAS
finished loading the notification goes to mail first abd then it print and then
check to see if it is busy and it comes back that it is. 

So instead I have to use two booleans to monitor if the document was asked to
be destroyed while it was preparing to print. This typically happens when the
user click on a click and it takes a while and then they decide to print. So
while the Print Dialog is up the document gets unloaded  and when it returned
back everything is gone.

So when the flow of control returns back from the Print Dialog it checks to see
if it "should" have been destroyed, and if so, it then aborts printing, cleans
and displays a message if the pressed "OK". If they pressed cancel then it
doesn't print the message.

Comment 21

[Syd Logan]

adt2 per adt triage

Comment 22

[Markus Multrus]

I think there's a relation to bug 132732.
http://bugzilla.mozilla.org/show_bug.cgi?id=132732

Comment 23

[sujay]

Olga, when this is fixed, we need to verify this in mail...

Comment 24

[Marc Attinasi]

Comment on attachment 75515 [details] [diff] [review]
new patch

I think we are going to need a documentation on what all these flags mean,
where they are set/checked/cleared and how they interact - there are a lot of
state variables around printing!

sr=attinasi for the patch, please consider documenting those bools

Comment 25

[rods (gone)]

Ok, I will before check in.

Comment 26

[dcone (gone)]

Comment on attachment 75515 [details] [diff] [review]
new patch

r=dcone

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 75515 [details] [diff] [review]
new patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 28

[rods (gone)]

fixed

Comment 29

[rods (gone)]

fixed

Comment 30

[sujay]

Markus, can you see if this is fixed for you now in today's build? thanks.
let us know..

Comment 31

[sujay]

verified in 3/25 build.

looks fixed now. I tried both Markus's and Boris's steps.

Comment 32

[Markus Multrus]

WFM with build 2002040108
sorry for the delay