Closed Bug 1402728 Opened 6 years ago Closed 6 years ago
Reduce occurrences of Track
Buffers Manager raw pointers
59 bytes, text/x-review-board-request
Bug 1247189 and bug 1402681 show that there may be some issues around TrackBuffersManager's lifetime management. There are a few places where TrackBuffersManagers are stored or accessed through raw pointers. To eliminate this potential (yet-unproven) risk, I would like to try and remove most of these raw pointers. Some of these changes may seem superfluous, as it could be reasoned that bad situations may not happen for them, however I think it's worth reducing raw pointer usage anyway: - Maybe we haven't thought of something sneaky involving them, - If we remove raw pointers from the code, at least we won't have to waste time thinking about them again, next time we get strange crash reports, - In most cases the cost of using RefPtrs should be null or negligible. (I'm opening a separate bug, and keeping the other two open, because I'm not confident that this will solve the actual problem.)
Oh, I've just noticed&remembered that `NewRunnableMethod<TBM*>` should automatically use RefPtr (because TBM is ref-counted). I'll keep the changes as it makes this extra clear, and even makes it a tiny bit more optimized. But I'll update the commit comment to avoid confusion.
Comment on attachment 8911620 [details] Bug 1402728 - Use RefPtr<TrackBuffersManager> instead of raw pointers - https://reviewboard.mozilla.org/r/183030/#review188234 ::: dom/media/mediasource/MediaSourceDemuxer.cpp:170 (Diff revision 2) > *crypto = mInfo.mCrypto; > return crypto; > } > > void > MediaSourceDemuxer::AttachSourceBuffer(TrackBuffersManager* aSourceBuffer) what about the argument type? if we want to be extra anal about it.
Attachment #8911620 - Flags: review?(jyavenard) → review+
Comment on attachment 8911620 [details] Bug 1402728 - Use RefPtr<TrackBuffersManager> instead of raw pointers - https://reviewboard.mozilla.org/r/183030/#review188234 > what about the argument type? > if we want to be extra anal about it. I considered it, but I didn't think it'd be useful here: The caller should already own the object during this call -- Whereas the other changes guarantee that we have extended the object lifetime after the dispatch, and until the end of the task. Though I suppose if I'm going for peace of mind, passing a RefPtr& would give us the assurance that the caller does own the object, at (I think) no cost; So ok I'll add it! Since you brought it up, I'll assume you approve.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/05b222ac8a54 Use RefPtr<TrackBuffersManager> instead of raw pointers - r=jya
You need to log in before you can comment on or make changes to this bug.