393432 - Firefox crashes sometimes if you click the back button on www.userfriendly.org [@ DummyParserRequest::Cancel 334d84da]

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Simon Wagner]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6

While browsing www.userfriendly.org Firefox crashes sometimes if you click the Back button. This has happened to me now several times so i am sending now this bug report.

Unluckily, it is hard to reproduce the bug, it doesn't happen always

Reproducible: Sometimes

Steps to Reproduce:
1. Go to www.userfriendly.org
2. Most of the time it occurs when i am browsing some of the comments, so browse through the comments
3. click sometimes the back button. if you are lucky it will crash
Actual Results:  
firefox crashed

Expected Results:  
firfox should of course not crash but go back

Here are the talkback IDs:
TB35217388M, TB35217141X, TB35214891E, TB34944593H, TB34824450E

Comment 1

[Carsten Book [:Tomcat]]

Stack Signature	 DummyParserRequest::Cancel 334d84da
Product ID	Firefox2
Build ID	2007072518
Trigger Time	2007-08-23 09:32:09.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (0027ad11)
URL visited	http://ars.userfriendly.org/cartoons/?id=20040102
User Comments	
Since Last Crash	513 sec
Total Uptime	537843 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/content/html/document/src/nsHTMLContentSink.cpp, line 689
Stack Trace 	
DummyParserRequest::Cancel  [mozilla/content/html/document/src/nsHTMLContentSink.cpp, line 689]
nsDocLoader::Stop  [mozilla/uriloader/base/nsDocLoader.cpp, line 312]
nsDocLoader::Stop  [mozilla/uriloader/base/nsDocLoader.cpp, line 307]
nsDocLoader::Stop  [mozilla/uriloader/base/nsDocLoader.cpp, line 307]
nsDocLoader::Stop  [mozilla/uriloader/base/nsDocLoader.cpp, line 307]
nsDocShell::Stop  [mozilla/docshell/base/nsDocShell.cpp, line 3277]
nsDocShell::InternalLoad  [mozilla/docshell/base/nsDocShell.cpp, line 6709]
nsDocShell::LoadHistoryEntry  [mozilla/docshell/base/nsDocShell.cpp, line 7865]
nsDocShell::LoadURI  [mozilla/docshell/base/nsDocShell.cpp, line 770]
nsSHistory::InitiateLoad  [mozilla/xpfe/components/shistory/src/nsSHistory.cpp, line 1216]
nsSHistory::CompareFrames  [mozilla/xpfe/components/shistory/src/nsSHistory.cpp, line 1141]
nsSHistory::LoadEntry  [mozilla/xpfe/components/shistory/src/nsSHistory.cpp, line 1111]
nsSHistory::GoBack  [mozilla/xpfe/components/shistory/src/nsSHistory.cpp, line 703]
nsDocShell::GoBack  [mozilla/docshell/base/nsDocShell.cpp, line 2815]
XPTC_InvokeByIndex  [mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102]
XPCWrappedNative::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2169]
XPC_WN_CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1455]
js_Invoke  [mozilla/js/src/jsinterp.c, line 1375]
js_Interpret  [mozilla/js/src/jsinterp.c, line 3946]
js_Invoke  [mozilla/js/src/jsinterp.c, line 1394]
js_InternalInvoke  [mozilla/js/src/jsinterp.c, line 1469]
JS_CallFunctionValue  [mozilla/js/src/jsapi.c, line 4351]
nsJSContext::CallEventHandler  [mozilla/dom/src/base/nsJSEnvironment.cpp, line 1493]
nsJSEventListener::HandleEvent  [mozilla/dom/src/events/nsJSEventListener.cpp, line 195]
nsEventListenerManager::HandleEventSubType  [mozilla/content/events/src/nsEventListenerManager.cpp, line 1655]
nsEventListenerManager::HandleEvent  [mozilla/content/events/src/nsEventListenerManager.cpp, line 1762]
nsXULElement::HandleDOMEvent  [mozilla/content/xul/content/src/nsXULElement.cpp, line 2232]
nsXULElement::HandleDOMEvent  [mozilla/content/xul/content/src/nsXULElement.cpp, line 2255]
PresShell::HandleDOMEventWithTarget  [mozilla/layout/base/nsPresShell.cpp, line 6531]
nsButtonBoxFrame::DoMouseClick  [mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp, line 181]
nsButtonBoxFrame::MouseClicked  [mozilla/layout/xul/base/src/nsButtonBoxFrame.h, line 61]
PresShell::HandleEventInternal  [mozilla/layout/base/nsPresShell.cpp, line 6473]
PresShell::HandleEventWithTarget  [mozilla/layout/base/nsPresShell.cpp, line 6330]
nsEventStateManager::CheckForAndDispatchClick  [mozilla/content/events/src/nsEventStateManager.cpp, line 3207]
nsEventStateManager::PostHandleEvent  [mozilla/content/events/src/nsEventStateManager.cpp, line 2170]
PresShell::HandleEventInternal  [mozilla/layout/base/nsPresShell.cpp, line 6504]
PresShell::HandleEvent  [mozilla/layout/base/nsPresShell.cpp, line 6268]
nsViewManager::HandleEvent  [mozilla/view/src/nsViewManager.cpp, line 2566]
nsViewManager::DispatchEvent  [mozilla/view/src/nsViewManager.cpp, line 2253]
HandleEvent  [mozilla/view/src/nsView.cpp, line 174]
nsWindow::DispatchEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 1319]
nsWindow::DispatchMouseEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 6329]
ChildWindow::DispatchMouseEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 6576]
nsWindow::WindowProc  [mozilla/widget/src/windows/nsWindow.cpp, line 1507]
USER32.dll + 0x8734 (0x7e368734)
USER32.dll + 0x8816 (0x7e368816)
USER32.dll + 0x89cd (0x7e3689cd)
USER32.dll + 0x8a10 (0x7e368a10)
nsAppShell::Run  [mozilla/widget/src/windows/nsAppShell.cpp, line 159]
nsAppStartup::Run  [mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 152]
main  [mozilla/browser/app/nsBrowserApp.cpp, line 61]
kernel32.dll + 0x16fd7 (0x7c816fd7)

Comment 2

[Jo Hermans]

Hmmm ... the talkback reports are consistent, but such a crash in
DummyParserRequest was never reported before. Reporter, it might be related to
an extension that you are running. Are you ?

Comment 3

[Simon Wagner]

Hm, might be.

Installed extensions:
Html Validator
Live HTTP Headers
Tab Mix Plus
Talkback
Web Developer
wmlbrowser

I will now deactivate them and see whether the crash still appears

Comment 4

[Boris Zbarsky [:bzbarsky]]

Does the crash still appear without the extensions?

Comment 5

[Colin Blake]

We are seeing what I think is the same crash in our product which is based on 1.8 branch. For us the
problem happens when content js has a timer which fires which initiates a page reload using:

  document.location=document.location

Here's our stack trace....

00 gklayout!DummyParserRequest::Cancel+0x1d
01 necko!nsLoadGroup::Cancel+0xb0
02 docshell!nsDocLoader::Stop+0x65
03 docshell!nsDocLoader::Stop+0x4d
04 docshell!nsDocLoader::Stop+0x4d
05 docshell!nsDocShell::Stop+0x93
06 docshell!nsDocShell::InternalLoad+0x7e0
07 docshell!nsDocShell::LoadURI+0x37d
08 gklayout!nsLocation::SetURI+0x8d
09 gklayout!nsLocation::SetHrefWithBase+0x1b7
0a gklayout!nsLocation::SetHrefWithContext+0x32
0b gklayout!nsLocation::SetHref+0x7a
0c gklayout!nsDocumentSH::SetProperty+0x169
0d xpc3250!XPC_WN_Helper_SetProperty+0x54
0e js3250!js_Interpret+0x84ba
0f js3250!js_Execute+0x1a3
10 js3250!JS_EvaluateUCScriptForPrincipals+0x56
11 gklayout!nsJSContext::EvaluateString+0x1b2
12 gklayout!nsGlobalWindow::RunTimeout+0x1c1
13 gklayout!nsGlobalWindow::TimerCallback+0x10

Comment 6

[François Gagné]

Could you provide a testcase?

Also what is the exact version of mozilla code are you using?

Comment 7

[Colin Blake]

Sorry, don't have a test case, but that page on which it happens has a timer
which fires after a second or two and then does the:

  document.location = document.location

Our product is based on MOZILLA_1_8_BRANCH as of 13:10 on 10/26/2007. This was Firefox 2.0.0.8+

Comment 8

[François Gagné]

I'm not a dev,

But things tend to get fixed faster if a testcase or 100% reprodutable steps are given. This way a dev can reproduce the problem locally.

Comment 9

[timeless]

french frog: that's nice. but colin is a dev. he knows this, and telling him this isn't helpful. if he knew of a better simpler testcase, he'd have posted it.

Comment 10

[François Gagné]

Fine, I just stating a fact: bugs with not reprodutable steps tend to stay unfixed.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Looking at the stack... the problem is that there is this "weak reference" thing going on.  That looks wrong, because there is nothing ensuring that the sink won't go away.

Could we just use an nsWeakReference here on branch?  Colin, do you have the cycles to give that a stab?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Oh, and dereferencing dead objects is bad.  We should fix if possible.

Comment 13

[Colin Blake]

Attachment: Store the sink in a nsWeakPtr and verify it is still valid before using.

Sorry the patch is for our local CVS, I am having trouble getting CVS to
cvs.mozilla.org working. But the patch should be the same.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Colin, unless you have a CVS account on cvs.m.o, you probably want to be using "cvs -d:pserver:[e-mail]:/cvsroot ..." (see http://developer.mozilla.org/en/docs/Mozilla_Source_Code_Via_CVS).

Comment 15

[Colin Blake]

I do have (or did have) a CVS account, but it isn't working for me at the
moment (see bug 421170).

And I did do a checkout using anonymous@cvs-mirror and that worked but then
any "cvs status" or "cvs diff -u" on an individual file would just hang.

Not sure what's going on, but I wanted to get the patch posted for reviews.

Comment 16

[Johnny Stenback (:jst)]

Comment on attachment 307569 [details] [diff] [review]
Store the sink in a nsWeakPtr and verify it is still valid before using.

Yeah, that should be safer.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 307569 [details] [diff] [review]
Store the sink in a nsWeakPtr and verify it is still valid before using.

sr=bzbarsky.

Let's try this..

Comment 18

[Daniel Veditz [:dveditz]]

assigning to colin to make sure this gets in.

(In reply to comment #5)
> We are seeing what I think is the same crash in our product which is based on
> 1.8 branch.

Does that mean you have a reproducible testcase that can be used to verify the fix, even if you can't share it?

Comment 19

[Colin Blake]

I could not never reproduce it myself, but we have a customer who is seeing it
all the time. I am in the process of getting him the fix to try.

Comment 20

[Samuel Sidler (old account; do not CC)]

Comment on attachment 307569 [details] [diff] [review]
Store the sink in a nsWeakPtr and verify it is still valid before using.

Approved for 1.8.1.13; a=ss for release-drivers.

Please land ASAP. Our code freeze is tonight at 11:59pm PST.

Comment 21

[Colin Blake]

Can someone check this in for me. I still don't have my CVS account working.

Comment 22

[Reed Loden [:reed]]

Checking in content/html/document/src/nsHTMLContentSink.cpp;
/cvsroot/mozilla/content/html/document/src/nsHTMLContentSink.cpp,v  <--  nsHTMLContentSink.cpp
new revision: 3.719.4.11; previous revision: 3.719.4.10
done

Comment 23

[Al Billings [:abillings - ex-MoCo]]

(In reply to comment #19)
> I could not never reproduce it myself, but we have a customer who is seeing it
> all the time. I am in the process of getting him the fix to try.
> 

Can you have him try the 2.0.0.13rc1 at ftp://ftp.mozilla.org/pub/firefox/nightly/2.0.0.13-candidates/rc1?

Comment 24

[timeless]

al: he can't, they ship custom builds, but you can be sure they'll ship new builds to the customer. please read comment 19 more carefully :(.

Comment 25

[Colin Blake]

I've already given the fix to the customer, but am still waiting to hear back
if it resolved the problem.

Comment 26

[Al Billings [:abillings - ex-MoCo]]

Timeless, I think you mean comment 5 and comment 7, not comment 19, which doesn't really address that. If you're going to chastise me, you might want to point me at the right comments. :-P

Comment 27

[Al Billings [:abillings - ex-MoCo]]

In any case, we'll wait for reports from Colin and his client about whether this seems to be fixed or not. I apologize if I wasn't paying enough attention earlier when going through the bug. 

Comment 28

[Colin Blake]

Customer confirms that crash has gone away with this fix.

Comment 29

[Al Billings [:abillings - ex-MoCo]]

Thank you, Colin. Marking verified.