285546 - weather.com crashes browser [@ nsXPConnect::SetDeferReleasesUntilAfterGarbageCollection]

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050309
Firefox/1.0+

Every build I've made so far this evening has crashed on the above URL.  The
official 3-09-2005 nightly doesn't crash on it, however.  So this appears to be
related to a checkin made after the release of the official 3-09-2005 nightly.

I don't have talkback, so I can't provide any more info, unfortunately.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Here's talkback information from someone else who had the crash:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?id=TB4240783K&search=2&type=iid

He also mentioned that the problem didn't occur when he had no plugins
installed, so it's likely plugin-related.

Comment 2

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050310
Firefox/1.0+

WFM

Comment 3

[OstGote!]

Incident ID: 4240783
Stack Signature	nsXPConnect::SetDeferReleasesUntilAfterGarbageCollection 692da9e4
Product ID	FirefoxTrunk
Build ID	2005030906
Trigger Time	2005-03-09 21:52:33.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (0000c899)
URL visited	
User Comments	bug 285546
Since Last Crash	410 sec
Total Uptime	410 sec
Trigger Reason	Access violation
Source File, Line No.
c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/nsXPConnect.cpp,
line 1035
Stack Trace 	
nsXPConnect::SetDeferReleasesUntilAfterGarbageCollection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/nsXPConnect.cpp,
line 1035]
nsDocShell::LoadErrorPage 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 3021]
nsCSSValue::SetIntValue 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/style/nsCSSValue.cpp,
line 258]
nsGenericHTMLFormElement::SetForm 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,
line 3171]
nsGenericElement::GetElementsByTagName 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 1495]
nsGenericHTMLElement::SetInnerHTML 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,
line 936]
nsGenericHTMLElement::SetInnerHTML 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,
line 936]
DocumentViewerImpl::PermitUnload 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsDocumentViewer.cpp,
line 1052]
nsDocShell::LoadErrorPage 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 3008]
nsCSSValue::SetIntValue 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/style/nsCSSValue.cpp,
line 258]
nsHTMLButtonControlFrame::Reflow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/forms/nsHTMLButtonControlFrame.cpp,
line 498]
nsMathMLmoFrame::ProcessOperatorData 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/mathml/base/src/nsMathMLmoFrame.cpp,
line 555]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsBoxFrame::GetInitialDirection 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 613]
nsSimplePageSequenceFrame::PrintNextPage 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/generic/nsSimplePageSequence.cpp,
line 836]
DocumentViewerImpl::PermitUnload 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsDocumentViewer.cpp,
line 1096]
nsDocShell::LoadErrorPage 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 3008]
nsXULWindow::GetInterface 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/xpfe/appshell/src/nsXULWindow.cpp,
line 178]
nsWebShellWindow::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp,
line 340]
nsWebShellWindow::~nsWebShellWindow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp,
line 163]
nsWindow::~nsWindow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 899]
nsWindow::~nsWindow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 935]
nsWindow::ProcessMessage 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 4319]
nsWindow::DealWithPopups 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1272]
USER32.dll + 0x8709 (0x77d48709)
USER32.dll + 0x87eb (0x77d487eb)
USER32.dll + 0xb368 (0x77d4b368)
USER32.dll + 0xb3b4 (0x77d4b3b4)
ntdll.dll + 0xeae3 (0x7c90eae3)
USER32.dll + 0xb2a1 (0x77d4b2a1)
uxtheme.dll + 0x3c20 (0x5ad73c20)
uxtheme.dll + 0x1e300 (0x5ad8e300)
uxtheme.dll + 0x1ac7 (0x5ad71ac7)
uxtheme.dll + 0x1b3d (0x5ad71b3d)
USER32.dll + 0xb9bd (0x77d4b9bd)
nsWindow::DealWithPopups 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1293]
USER32.dll + 0x8709 (0x77d48709)
USER32.dll + 0x87eb (0x77d487eb)
USER32.dll + 0xc00e (0x77d4c00e)
USER32.dll + 0xc034 (0x77d4c034)
nsWindow::DealWithPopups 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1290]
USER32.dll + 0x8709 (0x77d48709)
USER32.dll + 0x87eb (0x77d487eb)
USER32.dll + 0xb368 (0x77d4b368)
USER32.dll + 0xb3b4 (0x77d4b3b4)
ntdll.dll + 0xeae3 (0x7c90eae3)
USER32.dll + 0xb7ab (0x77d4b7ab)
uxtheme.dll + 0x2881f (0x5ad9881f)
uxtheme.dll + 0x1ac7 (0x5ad71ac7)
uxtheme.dll + 0x1b3d (0x5ad71b3d)
USER32.dll + 0xb9bd (0x77d4b9bd)
nsWindow::DealWithPopups 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1293]
USER32.dll + 0x8709 (0x77d48709)
USER32.dll + 0x87eb (0x77d487eb)
USER32.dll + 0xc00e (0x77d4c00e)
USER32.dll + 0xc034 (0x77d4c034)
nsWindow::DealWithPopups 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1290]
USER32.dll + 0x8709 (0x77d48709)

Comment 4

[timeless]

this stack doesn't make sense.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

I did a fresh CVS pull & build this morning.  The page still crashes, but
differently than it was last night.

To test, I opened up a new browser to (1) my default home page. I then went to
(2) the weather.com page in question. Finally, I went (3) back to my home page.

Here's what happens: (2) loads just fine when you first go there, all the way to
completion. However, the Reload button is disabled! (And given that I wrote the
patch which handles that, it shouldn't be :-)...).  Going to (3) re-enables the
reload button.

If when at (3), I hit the back button to go back to (2), I then get the Windows
application crash dialog once again like last night before the page is even
rendered.  I've also had one occasion where I hit Back quickly and got all the
way back to (1) without a crash. When I then hit Forward to go back to (2), the
browser crashed with no Windows dialog at all. The window just disappeared and
firefox.exe was no longer running in task manager.

I hope all this information helps.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

And for the record, using the steps outlined in Comment #5, I can make Firefox
crash every time without exception.  Also, I've removed all plugins from my
plugins directory and I can STILL make it crash following those steps.

Comment 7

[Peter van der Woude [:Peter6]]

Is this just another regression from bug 285404 ?

Comment 8

[Brian Ryner (not reading)]

Attachment: fix

The problem here is that we started passing an nsIDOMEventReceiver* to
[Compile|Register]ScriptEventListener().  This interface is implemented by a
tearoff object, not nsGenericElement.  Since objects are QI'd to nsISupports
when they are wrapped, the XPCWrappedNative was holding a reference to the
nsGenericElement.  However, the nsJSEventListener we created was just holding a
weak pointer to the tearoff, which goes away when
nsEventReceiverSH::RegisterCompileHandler returns.

I think it's best if we fix this at the level where we call WrapNative, by
ensuring that the nsJSEventListener is pointing to the same identity interface
that the WrappedNative is holding onto.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

The patch indeed does fix the crash, however the reload button still gets
disabled when it shouldn't be.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 177058 [details] [diff] [review]
fix

@@ -1140,6 +1140,8 @@ nsEventListenerManager::AddScriptEventLi

   nsCOMPtr<nsIDocument> doc;

+  nsISupports *objiSupp = aObject;
+

[...]
+    // Since JSEventListeners only have a raw nsISupports pointer, it's
+    // important that it point to the same object that the WrappedNative
wraps.
+    // (In the case of a tearoff, the tearoff will not persist).
+    nsCOMPtr<nsIXPConnectWrappedNative> wrapper = do_QueryInterface(holder);
+    NS_ASSERTION(wrapper, "wrapper must impl nsIXPConnectWrappedNative");
+
+    objiSupp = wrapper->Native();

You set objiSupp, but you don't ever use it. You want to pass it to
SetJSEventListener() at the end of this method, right?

r=jst with that.

Comment 11

[Brian Ryner (not reading)]

(In reply to comment #10)
> You set objiSupp, but you don't ever use it. You want to pass it to
> SetJSEventListener() at the end of this method, right?
> 

Right. My bad.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 177058 [details] [diff] [review]
fix

sr=bzbarsky

Comment 13

[David Bradley]

Is the reload button not enabled caused by something else, or is it a side
affect of bug 285404? Be interesting if you could to back out the patch from
285404 from your system and see what happens.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

(In reply to comment #13)
> Is the reload button not enabled caused by something else, or is it a side
> affect of bug 285404? Be interesting if you could to back out the patch from
> 285404 from your system and see what happens.

Not sure, but I've seen it on a few other pages too now (like a random slashdot
thread). I'm familiar with applying patches to the local tree, but not backing
them out. What command do I run for that?

Comment 15

[David Bradley]

patch -R <patchfile 

If I remember correctly.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Well, I can pull that patch, but then the compile fails miserably.

Comment 17

[David Bradley]

You'll also need to back out this patch if you still have it applied.

Did patch run without any errors?

Comment 18

[Ryan VanderMeulen [:RyanVM]]

There were a few occasional errors. I just did a fresh source pull, so your
patch shouldn't be applied now. I'll remove the patch again.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Even on a fresh CVS pull, 10 out of 42 hunks failed to be removed from
nsDOMClassInfo.cpp.  I stopped after that point.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

A few more examples of the reload button being disabled:
http://slashdot.org/article.pl?sid=05/03/11/0154230&tid=141&tid=95
http://yro.slashdot.org/article.pl?sid=05/03/11/011203&tid=141&tid=123

Yet some of the other articles on there work just fine.

Comment 21

[Brian Ryner (not reading)]

checked in.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

I just filed Bug 285738 for the reload button issues since this one is marked
fixed now.