465767 - document.adoptNode() stops working if node was previously adopted in another document

Status: VERIFIED FIXED

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

Description

[Wladimir Palant]

Attachment: Testcase

I got this issue reported for the Element Hiding Helper extension. Apparently, in the current trunk nightlies document.adoptNode() at some point starts throwing NS_ERROR_UNEXPECTED. I managed to create a minimized testcase. Open it, click "Adopt node" (will be successful). Wait some time without moving your mouse (30 seconds), click "Adopt node" again - this time there will be an error. The testcase unloads the document the node was adopted in, it seems that something gets garbage collected as a result and makes new adoptNode() calls fail. During my testing, this was very consistently reproducible in Minefield 20081119 build on Ubuntu 8.10, on Windows this wasn't quite as reliable (maybe I just didn't wait long enough).

Comment 1

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

Attachment: no need to be so strict

Strong parentNode/ownerDocument would fix this too, but since we don't have
that yet we can do this.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 349007 [details] [diff] [review]
no need to be so strict

>+++ mozilla/content/base/test
>+function CC() {
>+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
>+  window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
>+        .getInterface(Components.interfaces.nsIDOMWindowUtils)
>+        .garbageCollect();
>+}

Having to copy/paste this all over is ridiculous.  How about putting it in SimpleTest.js as SimpleTest.gc instead?  I've had that in my tree for a while now, but the patch it's part of doesn't seem to be getting review love... ;)

Other than that, looks fine to me.

Comment 3

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

Yeah, I was thinking about that .CC thing too. I'll file a new bug about that.

Comment 4

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

filed Bug 465809

Comment 5

[Wladimir Palant]

Good to know that you can force garbage collection, this might make some of my testcases simpler in future :)

Comment 6

[mdew]

could this land for b3?

Comment 7

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

Yes, I'm hoping so. The patch must get approval first.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 349007 [details] [diff] [review]
no need to be so strict

a191=beltzner

Comment 9

[Johnny Stenback (:jst)]

Not blocking, but this has approval and looks ready to land...

Comment 10

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

Fixed on trunk and 1.9.1

Comment 11

[Aakash Desai [:aakashd]]

verified FIXED using attached testcase on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090721
Minefield/3.6a1pre ID:20090721044139

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090720
Shiretoko/3.5.1pre ID:20090720042942