[FIX]crash quitting print dialog with "cancel"

VERIFIED FIXED in mozilla1.0

Status

()

P1
critical
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: mozilla2007, Assigned: rods)

Tracking

({crash})

Trunk
mozilla1.0
x86
Linux
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2])

Attachments

(1 attachment, 1 obsolete attachment)

7.99 KB, patch
dcone
: review+
attinasi
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

17 years ago
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

17 years ago
tracy can you reproduce this one ?
i could not reproduce this, but the again I could not reproduce the steps. 
(machine and connection are too fast)

Comment 3

17 years ago
Boris, can you reproduce ?
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"?
(Reporter)

Comment 5

17 years ago
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. 
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash

Comment 7

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

Comment 8

17 years ago
Sorry, that's TB4078846Q.  (Forgot the 'Q'.)
Keywords: mozilla1.0, nsbeta1
(Assignee)

Comment 9

17 years ago
Created attachment 74982 [details] [diff] [review]
patch

Must check to make sure the diocument viewer isn't busy when about to do
printing or print preview.
nsbeta1+
Keywords: nsbeta1 → nsbeta1+
Priority: -- → P1
Target Milestone: --- → mozilla1.0
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Summary: crash quitting print dialog with "cancel" → [FIX]crash quitting print dialog with "cancel"

Comment 11

17 years ago
Comment on attachment 74982 [details] [diff] [review]
patch

r=dcone
Attachment #74982 - Flags: review+

Comment 12

17 years ago
Comment on attachment 74982 [details] [diff] [review]
patch

sr=attinasi
Attachment #74982 - Flags: superreview+
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.)
Attachment #74982 - Flags: approval+
(Assignee)

Comment 14

17 years ago
Yes, I didn't really like that text either, so I fixed it and gave each it's own
comment.
fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 15

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

let us know..thanks!
(Reporter)

Comment 16

17 years ago
WFM with build 2002032021 :) Thanx!

Comment 17

17 years ago
verified per comments.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 18

17 years ago
Work for the Browser but fails for mail because of the order of the listeners.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Summary: [FIX]crash quitting print dialog with "cancel" → crash quitting print dialog with "cancel"
(Assignee)

Comment 19

17 years ago
Comment on attachment 74982 [details] [diff] [review]
patch

This solution doesn't work for mail.
Attachment #74982 - Attachment is obsolete: true
(Assignee)

Updated

17 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 20

17 years ago
Created attachment 75515 [details] [diff] [review]
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.
(Assignee)

Updated

17 years ago
Keywords: review
Summary: crash quitting print dialog with "cancel" → [FIX]crash quitting print dialog with "cancel"

Comment 21

17 years ago
adt2 per adt triage
Whiteboard: [adt2]
(Reporter)

Comment 22

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

Comment 23

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

Comment 24

17 years ago
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
Attachment #75515 - Flags: superreview+
(Assignee)

Comment 25

17 years ago
Ok, I will before check in.

Comment 26

17 years ago
Comment on attachment 75515 [details] [diff] [review]
new patch

r=dcone
Attachment #75515 - Flags: review+
Comment on attachment 75515 [details] [diff] [review]
new patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75515 - Flags: approval+
(Assignee)

Comment 28

17 years ago
fixed
(Assignee)

Comment 29

17 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 30

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

Comment 31

17 years ago
verified in 3/25 build.

looks fixed now. I tried both Markus's and Boris's steps.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 32

17 years ago
WFM with build 2002040108
sorry for the delay
You need to log in before you can comment on or make changes to this bug.