782792 - Make XPCJSRuntime::mJSHolders infallible

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: convert it

Less gross code, less error paths to worry about.

Comment 2

[Andrew McCreight [:mccr8]]

Comment on attachment 651897 [details] [diff] [review]
convert it

Does my definition of JSHolder in xpcprivate.h look reasonable, Justin?  I'm making a hash table from void* to nsScriptObjectTracer*.  I'm reasonably confident about the rest, but these key types weird me out a bit.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Not an nsDataHashtable?

Comment 4

[Justin Lebar (not reading bugmail)]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> Comment on attachment 651897 [details] [diff] [review]
> convert it
> 
> Does my definition of JSHolder in xpcprivate.h look reasonable, Justin?

I think Ms2ger is right, nsDataHashtable would make more sense.  Although this is not bad at all.

Comment 5

[Andrew McCreight [:mccr8]]

Ah, nice, I didn't notice that!  Thanks, I'll switch it over.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: JSDHash to nsDataHashtable

Comment 7

[Bobby Holley (:bholley)]

Comment on attachment 652898 [details] [diff] [review]
JSDHash to nsDataHashtable

The documentation seems to imply that we'd want an nsClassHashtable here, since nsScriptObjectTracer isn't a primitive type. Is that what we want here? I have no idea.

This looks reasonable, but I'm no expert in our hash tables, nor have I seen this cycle collection code before. But r=bholley if you're comfortable with that.

Comment 8

[Andrew McCreight [:mccr8]]

nsClassHashtable just looks like a type that does nsRefPtr on its values, which isn't what we want here. I'll check that this works in a try run.

Comment 9

[Andrew McCreight [:mccr8]]

Pushed with a slightly inspecific commit message.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0d1f69cc901

try run: https://tbpl.mozilla.org/?tree=Try&rev=6763c07cc2be
(the permaorange there is something that has since been disabled, see bug 681138)

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/c0d1f69cc901