Closed Bug 780151 Opened 8 years ago Closed 8 years ago

Simplify this-translating code

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Details

Attachments

(2 files)

Unsurprisingly, the API is overly generic and over-engineered.
Attachment #648699 - Flags: review?(bobbyholley+bmo)
Nobody is interested in *getting* the translators, only in setting them.
Attachment #648701 - Flags: review?(bobbyholley+bmo)
Comment on attachment 648699 [details] [diff] [review]
Part a: Remove unused arguments to TranslateThis

I've never seen this code. Bouncing to peter.
Attachment #648699 - Flags: review?(bobbyholley+bmo) → review?(peterv)
Attachment #648701 - Flags: review?(bobbyholley+bmo) → review?(peterv)
Comment on attachment 648699 [details] [diff] [review]
Part a: Remove unused arguments to TranslateThis

r=me
Attachment #648699 - Flags: review?(peterv) → review+
Comment on attachment 648701 [details] [diff] [review]
Part b: Don't hand out ThisTranslators

Do you not need the lock because xpconnect is mainthread-only now?  If so, can you get rid of that lock altogether?

r=me with a followup for that.
Attachment #648701 - Flags: review?(peterv) → review+
(In reply to Boris Zbarsky (:bz) from comment #4)
> Comment on attachment 648701 [details] [diff] [review]
> Part b: Don't hand out ThisTranslators
> 
> Do you not need the lock because xpconnect is mainthread-only now?  If so,
> can you get rid of that lock altogether?
> 
> r=me with a followup for that.

Can I claim bug 770535?
Locks can't quite go away until I finish getting necko and friends to stop AddRef/Release-ing XPCWrappedJS objects off-main-thread (bug 773610). Until then, since XPCWrappedJS::Release needs to call into XPConnect CC machinery, I think we should leave the locks in place.
https://hg.mozilla.org/mozilla-central/rev/2f03d52c7190
https://hg.mozilla.org/mozilla-central/rev/ee22f4c023e2
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.