730470 - crash in nsFormFillController::RemoveForDocumentEnumerator @ nsINode::OwnerDoc

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

This bug was filed from the Socorro interface and is 
report bp-fe2a5fe3-4553-49eb-8764-4aa832120224 .
=============================================================

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

Reopening, please see bug 714579 comment 35.

Comment 3

[Scoobidiver (away)]

It's #10 top crasher in 13.0a1 over the last day.

With that stack, it first appeared in 13.0a1/20120223. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=373c710112e6&tochange=5e756e59a794
It's likely a regression from bug 726334.

Signature 	nsINode::OwnerDoc() More Reports Search
UUID	fe2a5fe3-4553-49eb-8764-4aa832120224
Date Processed	2012-02-24 22:01:34
Uptime	121
Last Crash	13.7 hours before submission
Install Age	2.0 minutes since version was first installed.
Install Time	2012-02-24 21:59:31
Product	Firefox
Version	13.0a1
Build ID	20120224031039
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 37 stepping 2
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0xffffffffdadadade
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x0046, AdapterSubsysID: 00000000, AdapterDriverVersion: 8.15.10.2509
D2D? D2D+
DWrite? DWrite+
D3D10 Layers? D3D10 Layers+
EMCheckCompatibility	True	
Total Virtual Memory	4294836224
Available Virtual Memory	3705896960
System Memory Use Percentage	45
Available Page File	6172868608
Available Physical Memory	2220212224

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsINode::OwnerDoc 	obj-firefox/dist/include/nsINode.h:429
1 	xul.dll 	nsFormFillController::RemoveForDocumentEnumerator 	toolkit/components/satchel/nsFormFillController.cpp:798
2 	xul.dll 	nsBaseHashtable<nsISupportsHashKey,nsCOMPtr<nsXMLEventsListener>,nsXMLEventsListener*>::s_EnumStub 	obj-firefox/dist/include/nsBaseHashtable.h:414
3 	xul.dll 	PL_DHashTableEnumerate 	obj-firefox/xpcom/build/pldhash.cpp:754
4 	xul.dll 	nsFormFillController::HandleEvent 	toolkit/components/satchel/nsFormFillController.cpp:785
5 	xul.dll 	mozilla::net::SpdyStream::ChangeState 	netwerk/protocol/http/SpdyStream.cpp:638
6 	xul.dll 	nsNSSASN1Tree::GetCellText 	security/manager/pki/src/nsASN1Tree.cpp:328
7 	xul.dll 	xul.dll@0x15bd8f 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsINode%3A%3AOwnerDoc%28%29

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Can anyone reproduce this? I have a patch, but would like someone to verify it.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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

https://tbpl.mozilla.org/?tree=Try&rev=ba02419a217f

Comment 7

[:Gavin Sharp [email: [e-mail]]]

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

Can you add a comment in the destructor about why it's unnecessary to use MaybeRemoveMutationObserver() (enumerator will have already cleared all of mPwmgrInputs). Alternatively, use MaybeRemoveMutationObserver there too even though it's unnecessary, just to be consistent.

It would be nice to have a comment in MaybeRemoveMutationObserver similar to:
"Nodes being tracked in mPwmgrInputs will have their observers removed when they stop being tracked." Similarly, a comment like "mFocusedInputNodes observer is tracked separately, don't remove it here" in RemoveForDocumentEnumerator.

I think it's impossible for the list node to be added to mPwmgrInputs (MarkAsLoginManagerField takes a nsIDOMHTMLInputElement*), so you don't really need to use MaybeRemoveMutationObserver for mListNode, and you don't need the mListNode check in RemoveForDocumentEnumerator.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +comments

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/499144e6fb86