825053 - We don't consistently support nodes as weak map keys any more

Status: RESOLVED FIXED

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

Description

[Andrew McCreight [:mccr8]]

Bug 821606 converted some nodes to the new DOM bindings, but we only support old DOM bindings as weak map keys. Bug 777385 tries to come up with an approach for all DOM bindings, but as Peter points out this requires the implication that all cycle collected new-bindings-ed classes are nsWrapperCache.

Maybe we can hew out some chunk of that patch that will just work for nodes, and not worry about the rest for now.

Comment 1

[Masatoshi Kimura [:emk]]

We need a reliable replacement to remove getUserData() and setUserData().

Comment 2

[Andrew McCreight [:mccr8]]

I assume all nodes are still nsISupports, so one thing we could do is to use the logic of bug 777385, but fail in the case of non-nsISupports cycle-collected objects. That will require reworking the logic a bit to handle failure, but it shouldn't be too bad.

Comment 3

[Andrew McCreight [:mccr8]]

Bug 777385 should fix this, and hopefully will be okay.

Comment 4

[Andrew McCreight [:mccr8]]

It would be good to either reverse the change made to this test when this is fixed ( https://hg.mozilla.org/integration/mozilla-inbound/rev/85b257295469 ) or just add a new test. We could have a separate test that just attempts to add a bunch of different elements as keys, as we will just throw when we fail to preserve the wrapper.

Comment 5

[Boris Zbarsky [:bzbarsky]]

One option is to take something like http://mxr.mozilla.org/mozilla-central/source/content/html/content/test/test_bug389797.html?force=1 and modify it to create one of each element and use them all as keys...

Comment 6

[Andrew McCreight [:mccr8]]

This should be fixed now, by 777385.

Comment 7

[Masatoshi Kimura [:emk]]

FIXED would be fine because we know what change fixed the problem.