Closed Bug 680922 Opened 10 years ago Closed 10 years ago

Crash [@ nsIdentifierMapEntry::RemoveNameElement] on reload with form in binding and name, id, observes


(Core :: XUL, defect, P1)

Windows 7





(Reporter: martijn.martijn, Assigned: bzbarsky)



(Keywords: crash, testcase)

Crash Data


(2 files)

See zipped up testcase.
You need to set the dom.allowXULXBL_for_file or dom.allow_XUL_XBL_for_file pref to true in about:config to see the crash.
Open the file named 'crash1.xul' to see the crash.
0 	xul.dll 	nsVoidArray::IndexOf 	obj-firefox/xpcom/build/nsVoidArray.cpp:427
1 	xul.dll 	nsCOMArray_base::RemoveObject 	obj-firefox/xpcom/build/nsCOMArray.cpp:123
2 	xul.dll 	nsIdentifierMapEntry::RemoveNameElement 	content/base/src/nsDocument.cpp:446
3 	xul.dll 	nsDocument::RemoveFromNameTable 	content/base/src/nsDocument.cpp:2592
4 	xul.dll 	nsGenericHTMLElement::UnbindFromTree 	
5 	xul.dll 	nsHTMLFormElement::UnbindFromTree 	content/html/content/src/nsHTMLFormElement.cpp:548
6 	xul.dll 	nsXBLBinding::UninstallAnonymousContent 	content/xbl/src/nsXBLBinding.cpp:393
7 	xul.dll 	nsXBLBinding::ChangeDocument 	content/xbl/src/nsXBLBinding.cpp:1175
8 	xul.dll 	nsBindingManager::RemovedFromDocumentInternal 	
9 	xul.dll 	nsBindingManager::RemovedFromDocument 	obj-firefox/dist/include/nsBindingManager.h:98
10 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3654
11 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3665
12 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3665
13 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3665
14 	xul.dll 	nsDocument::Destroy 	content/base/src/nsDocument.cpp:7040
15 	xul.dll 	DocumentViewerImpl::Destroy 	layout/base/nsDocumentViewer.cpp:1652
16 	xul.dll 	DocumentViewerImpl::Show 	layout/base/nsDocumentViewer.cpp:1961
17 	xul.dll 	nsPresContext::EnsureVisible 	layout/base/nsPresContext.cpp:1753
18 	xul.dll 	PresShell::UnsuppressAndInvalidate 	layout/base/nsPresShell.cpp:4504
19 	xul.dll 	PresShell::UnsuppressPainting 	layout/base/nsPresShell.cpp:4553
20 	xul.dll 	DocumentViewerImpl::LoadComplete 	layout/base/nsDocumentViewer.cpp:1096
21 	xul.dll 	nsDocShell::EndPageLoad 	docshell/base/nsDocShell.cpp:6154
Attached file zipped up testcase
This regressed between 2011-04-22 and 2011-04-23:
Regression from bug 588990 is my guess.
Blocks: 588990
We're crashing because in nsIdentifierMapEntry::RemoveNameElement mNameContentList is null.

We get to RemoveNameElement via nsHTMLFormElement::UnbindFromTree calling nsGenericHTMLElement::RemoveFromNameTable.

And in particular, nsGenericHTMLElement::AddToNameTable checks IsInAnonymousSubtree, but nsGenericHTMLElement::RemoveFromNameTable does not.
Assignee: nobody → bzbarsky
Priority: -- → P1
Whiteboard: [need review]
Crash Signature: [@ nsVoidArray::IndexOf(void*) ]
Comment on attachment 555493 [details] [diff] [review]
Don't try to remove anonymous elements from the name map.

Review of attachment 555493 [details] [diff] [review]:

r=me either way

::: content/html/content/crashtests/680922-binding.xml
@@ +3,5 @@
> +<content>
> +<html:form id="g" observes="b"/>
> +</content>
> +</binding>
> +</bindings>
\ No newline at end of file

Add newline to the end of this file.

::: content/html/content/src/nsGenericHTMLElement.h
@@ +553,5 @@
>    }
>    void RemoveFromNameTable() {
>      if (HasName()) {
>        nsIDocument* doc = GetCurrentDoc();
> +      if (doc && !IsInAnonymousSubtree()) {

Hmm.. I *think* that my idea was that it should be ok to remove an element that wasn't in the name map. I guess I'm fine with this solution too, but I wonder if it wouldn't be faster and safer to add a nullcheck.
Attachment #555493 - Flags: review?(jonas) → review+
It _might_ be safer; in the common case of no anonymous content shenanigans it's probably a tad faster.  I can do the null-check instead; I assume your review applies to that? with the null-check and the added newline.
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla9
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.