Closed Bug 730470 Opened 8 years ago Closed 8 years ago

crash in nsFormFillController::RemoveForDocumentEnumerator @ nsINode::OwnerDoc

Categories

(Toolkit :: Form Manager, defect, critical)

13 Branch
All
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: davidb, Assigned: smaug)

References

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-fe2a5fe3-4553-49eb-8764-4aa832120224 .
=============================================================
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 714579
Reopening, please see bug 714579 comment 35.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Status: REOPENED → NEW
Component: Untriaged → Form Manager
Product: Firefox → Toolkit
QA Contact: untriaged → form.manager
Version: unspecified → Trunk
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
Blocks: 726334
Keywords: regression, topcrash
OS: Windows NT → Windows 7
Hardware: x86 → All
Summary: crash nsINode::OwnerDoc → crash in nsFormFillController::RemoveForDocumentEnumerator @ nsINode::OwnerDoc
Version: Trunk → 13 Branch
Assignee: nobody → bugs
Can anyone reproduce this? I have a patch, but would like someone to verify it.
Attached patch patch (obsolete) — Splinter Review
Attachment #600690 - Flags: review?
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.
Attachment #600690 - Flags: review?(gavin.sharp) → review+
Crash Signature: [@ nsINode::OwnerDoc()] → [@ nsINode::OwnerDoc()] [@ nsINode::RemoveMutationObserver(nsIMutationObserver*)]
Attached patch +commentsSplinter Review
Attachment #600690 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/499144e6fb86
Status: NEW → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.