Closed Bug 782792 Opened 7 years ago Closed 7 years ago

Make XPCJSRuntime::mJSHolders infallible

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch convert it (obsolete) — Splinter Review
Less gross code, less error paths to worry about.
Summary: Make mJSHolders infallible → Make XPCJSRuntime::mJSHolders infallible
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.
Attachment #651897 - Flags: feedback?(justin.lebar+bug)
Not an nsDataHashtable?
(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.
Attachment #651897 - Flags: feedback?(justin.lebar+bug) → feedback+
Ah, nice, I didn't notice that!  Thanks, I'll switch it over.
Attachment #651897 - Attachment is obsolete: true
Attachment #652898 - Flags: review?(bobbyholley+bmo)
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.
Attachment #652898 - Flags: review?(bobbyholley+bmo) → review+
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.
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)
https://hg.mozilla.org/mozilla-central/rev/c0d1f69cc901
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.