Closed Bug 397849 Opened 13 years ago Closed 12 years ago

"ASSERTION: Shallow unbind won't clear document and binding parent on kids!"

Categories

(Core :: XBL, defect, P4)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jruderman, Assigned: sicking)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(1 file)

1.02 KB, application/xhtml+xml
Details
Attached file testcase
###!!! ASSERTION: Shallow unbind won't clear document and binding parent on kids!: 'aDeep || (!GetCurrentDoc() && !GetBindingParent())', file /Users/jruderman/trunk/mozilla/content/base/src/nsGenericElement.cpp, line 2086

Split from bug 360078.  The testcase is the same as the testcase from that bug; I haven't attempted to reduce it further.
This is kinda fun.  We have a node with NODE_FORCE_XBL_BINDINGS set on it.  It never gets inserted into a DOM.  Then its ownerDocument dies, which kills the binding manager, which releases the <content> node from the binding.... which still has all the anon content as kids (though their parent is the bound node).  So we assert because those kids have a bindingParent that's not null, and we're doing a shallow unbind on them.

I think the right thing to do when a binding goes away would be to unbind all that content... or something.  Jonas?
"that content" being the anonymous content in the binding? If so yes. Can this cause badness if we don't fix for 1.9? If so we should consider blocking.

Ideally those nodes shouldn't even be children of the <content>, but instead kept alive using a simple nsCOMArray<nsIContent> somewhere.
"that content" is the anonymous content of the binding, yes.

I think we might use the fact that they're kids of the <content> in XBL code, so switching that would not be all that simple.  But I may be wrong... I haven't looked at that code recently.

Not sure about badness.  It does violate some expectations we seem to have (e.g. that a binding won't outlive the binding manager).
The testcase in bug 394800 also triggers this assertion.
I can't reproduce this assertion with this testcase or the one in bug 394800.
I still hit it with this testcase, but not as often as when I initially reported the bug.  I hit the assertion within a few minutes just now.
A clean Mac trunk Sunbird debug build outputs the following on startup:

###!!! ASSERTION: Shallow unbind won't clear document and binding parent on kids!: 'aDeep || (!GetCurrentDoc() && !GetBindingParent())', file /Users/skywalker/Desktop/Mozilla/cvs/mozilla/content/base/src/nsGenericElement.cpp, line 2177
Blocks: sb-noise
Assignee: nobody → jonas
Flags: wanted1.9.0.x+
Flags: wanted1.9.0.x-
Flags: wanted1.9.0.x+
Flags: wanted-next+
Priority: -- → P4
I do think we want this since potentially this can cause inconsistencies leading to crashes. Keeping low priority though until we've actually seen that.
Flags: wanted1.9.0.x- → wanted1.9.0.x+
Same happens on Windows when restarting Firefox after a crash and the Session Restore error page is loading.

###!!! ASSERTION: Shallow unbind won't clear document and binding parent on kids
!: 'aDeep || (!GetCurrentDoc() && !GetBindingParent())', file c:/mozilla/src/con
tent/base/src/nsGenericElement.cpp, line 2654
OS: Mac OS X → All
Hardware: PC → All
> Same happens on Windows when restarting Firefox after a crash and the Session
> Restore error page is loading.

Are you sure it's "same"?  This assertion will fire any time someone screws up binding/unbinding using nsIContent APIs.  This bug describes one way it can happen, but there are plenty of others.  I strongly suggest that unless you have some data indicating that the other screwup is the exact XBL screwup described here (which, honestly, I doubt) filing a bug to investigate the issue is a lot more productive than assuming that it's the same as this one.
Sorry, filed that as bug 460686.
I can't reproduce this (on OSX) anymore.
Me too. Smaug, is it also fixed by the patch on bug 461027?
This testcase WFM, but I still see the assertion frequently.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
in-testsuite-: very slow testcase
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.