Closed Bug 1372736 Opened 3 years ago Closed 3 years ago

Stop using AbstractThread outside of media code

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Once bug 1366072 lands, we won't need to use AbstractThread at all outside of media code. This patch removes it. I'm not really sure how to prevent new usage. We could move AbstractThread and the state mirroring stuff back to dom/media, but people might still use it. We could also rename it to AbstractMediaThread or something. That would be pretty disruptive to the media code, though.

Ben, if you don't have time to review this, please pass it on to someone else.
Attachment #8877325 - Flags: review?(bkelly)
Should we also replace our uses of TaskQueue[1] with something else too, like a ThrottledEventQueue subclass/variant that supports threadpools?  We used it in both those cases to have properly sequenced events dispatched at the StreamTransportService's threadpool.  Such a cleanup might also address QuotaManager's use of LazyIdleThread which at least has been problematic as-implemented for mozStorage's use of do_GetCurrentThread(), although I assume we're addressing uses of do_GetCurrentThread() in general too.

1: Currently used in dom/file/MutableBlobStorage, dom/indexedDB/ActorsChild
Hmm, I wasn't aware of those uses of TaskQueue. Since those uses don't enable tail dispatch, I guess they're not too much of a problem right now. But it would be good to split that stuff out somehow eventually.
Comment on attachment 8877325 [details] [diff] [review]
patch

Review of attachment 8877325 [details] [diff] [review]:
-----------------------------------------------------------------

Perhaps we should add a comment to AbstractThread.h suggesting devs consider nsISerialEventTarget instead.  Something to keep AbstractThread from creeping in more places, etc.
Attachment #8877325 - Flags: review?(bkelly) → review+
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/871e57ecd232
Avoid using AbstractThread for non-media MozPromises (r=bkelly)
https://hg.mozilla.org/mozilla-central/rev/871e57ecd232
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.