919585 - ASSERTION: Want to fire DOMNodeRemoved event, but it's not safe: 'aChild->IsNodeOfType(nsINode::eCONTENT) && static_cast<nsIContent*>(aChild)-> IsInNativeAnonymousSubtree() || IsSafeToRunScript() || sDOMNodeRemovedSuppressCount'

Status: RESOLVED DUPLICATE of bug 736695

(Core :: XBL, defect)

Description

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

+++ This bug was initially created as a clone of Bug #712521 +++

The stack trace in bug 712521 is actually rather worrisome. We're running JS while doing UnbindFromTree.
That should not be allowed. I thought we execute XBL ctor and dtor using a script runner, but maybe not.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I think we fire ctor from a script runner, but maybe not dtor.

We should try to fix this...

Comment 2

[Andrew McCreight [:mccr8]]

Blake, can you look at this?  Thanks.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

I'll probably get to this after the summit.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is this the old "we're doing teardown of bindings the wrong way" bug? IIRC we need to separate "remove things from the DOM" from "run XBL ctor" and do the former synchronously and the latter from a scriptrunner.

Or some such, it's been too long :(

The really tricky part is that doing this while keeping backwards compatibility with regards to what DOM the ctor sees is going to be RealHard

Comment 5

[Andrew McCreight [:mccr8]]

(In reply to Blake Kaplan (:mrbkap) from comment #3)
> I'll probably get to this after the summit.

Have you had a chance to look at this, Blake?  Maybe William could investigate this?  This bug has sat dead for almost two months.

Comment 6

[Andrew McCreight [:mccr8]]

William, could you look at this?  Thanks!

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

I think wchen will be able to get to this before me. William, tell me if I'm wrong!

Comment 8

[William Chen [:wchen]]

I haven't been able to reproduce the stack that removes a child node in UnbindFromTree. I'm getting the same result as Blake in Bug 712521 c4, I've tried in both nightly and a build I have from half a year ago.

Comment 9

[Andrew McCreight [:mccr8]]

Can you still reproduce this, mats?

Comment 10

[Mats Palmgren (inactive)]

I haven't been involved in this bug at all, so I'm not sure why you ask me...
But yeah, running this still reproduces the assertion:
./mach mochitest-browser browser/base/content/test/general/browser_aboutHome.js

Comment 11

[Andrew McCreight [:mccr8]]

Well, you filed bug 712521 back in Dec. 2011, which is the original source.  Thanks for checking.

Comment 12

[William Chen [:wchen]]

(In reply to Mats Palmgren (:mats) from comment #10)
> I haven't been involved in this bug at all, so I'm not sure why you ask me...
> But yeah, running this still reproduces the assertion:
> ./mach mochitest-browser
> browser/base/content/test/general/browser_aboutHome.js

That test hits the same assertion but with a different stack. This bug is concerning removing a child while in UnbindFromTree, the assertion from browser_aboutHome.js is due to the parser.

Comment 13

[Andrew McCreight [:mccr8]]

It seems we don't have a test case for this, but comment 0 and comment 1 sound like there's still something actionable here: make sure that XBL ctor and dtor are executed using a script runner (probably the ctor is but maybe not the dtor?).  Can you look into that, William?  Thanks.

Comment 14

[William Chen [:wchen]]

Upon further investigation it looks like this was fixed in Bug 736695.

The stack in bug 712521 shows that the destructor was being executed by a call to nsBindingManager::RemovedFromDocument in nsGenericElement::UnbindFromTree. The call was moved into a script runner here: https://hg.mozilla.org/mozilla-central/rev/ca716959ca2b

Comment 15

[Andrew McCreight [:mccr8]]

Great!  Thanks for the analysis.