558973 - "ASSERTION: Nodes that are not documents, document fragments or elements can't be parents!"

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Nodes that are not documents, document fragments or elements can't be parents!: 'aParent->IsNodeOfType(nsINode::eDOCUMENT) || aParent->IsNodeOfType(nsINode::eDOCUMENT_FRAGMENT) || aParent->IsNodeOfType(nsINode::eELEMENT)', file /Users/jruderman/mozilla-central/content/base/src/nsGenericElement.cpp, line 3574

This assertion was added in bug 540848 and is very easy to trigger.

There are already a bunch of mochitests (at least in part 2/5) that trigger this assertion, but assertions aren't fatal yet on Tinderbox :/

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

Apparently the DOM testsuite doesn't test this :-/. I don't like adding those (virtual) IsNodeOfType calls, but don't have a better idea.

Comment 2

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

Do we have spare bits in nsINode? One could be used for
"DO_NOT_ALLOW_ADDING_CHILD_NODES" or some such.
Though, I'm not sure if GetFlags is much faster than IsNodeOfType

Comment 3

[Boris Zbarsky [:bzbarsky]]

So wait.  Why do we need to call the generic methods from nsGenericDOMDataNode and add checks to them?  Because quickstubs calls those methods directly?

Comment 4

[Boris Zbarsky [:bzbarsky]]

And we do want to make flag-getting fast (just have them as bitfield members?) and replace some of our IsNodeOfType with flag bits, imo.  But that's a separate bug.

Comment 5

[Peter Van der Beken [:peterv]]

(In reply to comment #3)
> Why do we need to call the generic methods from nsGenericDOMDataNode

Technically we don't, but I personally like it better if all entry points end up running the same code.

> and add checks to them?

Quickstubs call the generic nsINode methods (through the inlined nsINode::InsertBefore/ReplaceChild/AppendChild). We could make the nsINode functions virtual and override them in nsGenericDOMDataNode if you like that better. That would remove one virtual function call overall for ReplaceOrInsertBefore (-2+1).

I think we need faster checks for nodetype, so that we can actually completely remove those virtual function calls (-2+0).

Comment 6

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/mozilla-central/rev/1e70c342a6cc

Comment 7

[Boris Zbarsky [:bzbarsky]]

> I think we need faster checks for nodetype,

I'm adding a faster IsElement() right now; we can add others similarly as needed.