Steps to reproduce problem: 1. Insert a node into a document 2. Obtain its box object 3. Remove the node and all references to it 4. Attempt to get the node from the box object nsBoxObject::GetElement segfaults because mContent is stale.
Created attachment 213884 [details] [diff] [review] Band-aid I'm assuming here that bug 329181 will clean this up properly.
Hmm... we remove the box object from the box object table when a node is removed from the DOM, right? And never put it back? Is that true? If so, this is probably the right fix period.
Sigh; nsIDOMNSDocument::SetBoxObjectFor is scriptable, which would seem to suggest that content can associate fake box objects with its nodes to confuse extensions... there are only three callers a) GetBoxObjectFor, to store a new box object and b) nsGenericElement/nsXULElement::UnbindFromTree, to forget the box object. But I guess that ought to be handled in a separate bug...
Why do we let web content get a box object, isn't that an internal representation?
(In reply to comment #4) >Why do we let web content get a box object, isn't that an internal representation? No, it's what's supposed to exist to protect the internal representation. (This was the third rewrite, so I gave up and heavily summarised).
> Sigh; nsIDOMNSDocument::SetBoxObjectFor is scriptable Bug 326931. I just cced you. I outline a fix in that bug...
By "mContent is stale" I assume you mean it's a deleted object, and that we therefore want this fix on old branches too?
(In reply to comment #7) >By "mContent is stale" I assume you mean it's a deleted object Yes. >and that we therefore want this fix on old branches too? As far as I can tell, given my knowledge of security policy.
Comment on attachment 213884 [details] [diff] [review] Band-aid Fwiw, on trunk I've more or less rolled this into bug 329181. We should still fix on branch, though.
Fix checked in to the 1.8 branch. Targetting for 1.8 and marking fixed as per comment 9.
Comment on attachment 213884 [details] [diff] [review] Band-aid Trivial stale pointer fix.
Comment on attachment 213884 [details] [diff] [review] Band-aid approved for 1.8.0 branch, a=dveditz for drivers
Fix checked in to the 1.8.0 branch.
Adding [need testcase], can anyone provide a simplified testcase for this bug so we can verify the fix? If this one-liner is something that can't easily be tested, I think the comments in this bug are enough to marked this v.fixed. Let us know or just replace "fixed126.96.36.199" with "verified188.8.131.52". Thanks!
This bookmarklet alerts true on builds with the bug and false on fixed builds.
Reproducing reliably is hard -- it depends on JS GC timing...
I wasn't trying to reproduce the crash, just verify the fix.
verifying per reporters comment 15