Closed Bug 1437640 Opened 5 years ago Closed 5 years ago
can we make Mutable
Blob Stream Listener usefully retargetable?
Over in bug 1437188 Eric noticed we are getting a lot of failed channel retargeting when running a blob-heavy WPT test. This is because MutableBlobStreamListener and its backing storage class are not thread safe. For now we are avoiding the retarget when we know we explicitly add a non-thread-safe listener like this, but could we make it thread-safe? Could MutableBlobStreamListener be retargeted to another thread usefully? I say "usefully", because if the implementation is just bouncing the runnables to the main thread, then that its probably just best not to retarget. It would only really be helpful if it could write the buffers OMT. Andrea, what do you think?
Assignee: nobody → amarchesini
Attachment #8950555 - Flags: review?(bugs)
Comment on attachment 8950555 [details] [diff] [review] retarget.patch Those *Locked() methods are rather error prone. Please don't rename, but add a aProofOfLock parameter, similar to https://searchfox.org/mozilla-central/rev/9d47b5fb14152865ad1375eb3ee2e571d81ecdb9/xpcom/threads/LabeledEventQueue.h#47 -// This class is main-thread only. +// This class is main-thread only, but ::Append() can be called on any thread. class MutableBlobStorage final Odd comment. "main-thread only" ... but can be used on any thread after all. And Append calls many other methods. So, the class isn't main-thread only.
Attachment #8950555 - Flags: review?(bugs) → review-
Fixed comments and aProofOfLock param.
Comment on attachment 8950568 [details] [diff] [review] retarget.patch better to push to try (debug builds)
Attachment #8950568 - Flags: review?(bugs) → review+
Andrea, when you land this can you remove the `if (rr && rl)` bit of code that was added in bug 1437188? We only added that to work around MutableBlobStreamListener being non-retargetable. The WPT mimesniff/mime-type/parsers.any.html test should trigger this code a lot if you want to verify that the warnings are indeed still gone.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/100460d7a9af Implement nsIThreadRetargetableStreamListener in MutableBlobStreamListener, r=smaug
I applied the suggested comments before landing.
You need to log in before you can comment on or make changes to this bug.