This is the last piece of the puzzle before we can implement yielding in bug 1284031: we need the ability to tell a SourceBufferIterator "only advance by this many bytes", and we need it to work efficiently. In this case, we'll keep it efficient by having the SourceBufferIterator continue to grab as much data as it can from its owning SourceBuffer. It will then allow users to advance a few bytes at a time, if they so desire, through the data it has already grabbed. Only when it reaches the end of the data it has available does it need to talk to the owning SourceBuffer again to get more data. Since interacting with the SourceBuffer involves taking a lock, this greatly reduces the overhead required for advancing in many very small steps, which is what we want to support. We want tests for this, obviously. They're included in the patch in bug 1286161, which adds a GTest suite for SourceBuffer (there wasn't one up to now).
Created attachment 8770045 [details] [diff] [review] Allow SourceBufferIterator users to specify how many bytes to advance. Here's the patch. Basically the SourceBufferIterator now keeps track of the amount of data it actually got from the SourceBuffer most recently (mAvailableLength) as well as the amount that the caller wanted to read on the last call to Advance() (mNextReadLength). Calls to Advance() don't have to interact with the owning SourceBuffer and take the lock as long as |mAvailableLength| isn't exhausted. SourceBufferIterator's code could be a little cleaner; it's a bit weird how SourceBufferIterator calls into SourceBuffer which calls back into SourceBufferIterator to call methods like SetReady(). But this bug probably isn't the place to fix that, so I've stuck with that approach here.
Attachment #8770045 - Flags: review?(edwin)
Attachment #8770045 - Flags: review?(edwin) → review+
Created attachment 8770703 [details] [diff] [review] Allow SourceBufferIterator users to specify how many bytes to advance. Thanks for the review! This updated patch contains a small fix that makes sure that we don't read a new chunk from the SourceBuffer if a zero-byte read was requested, even if we're at the end of the previous chunk. I've updated the SourceBuffer GTest suite to include a test for this.
Attachment #8770045 - Attachment is obsolete: true
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/515b58073c46 Allow SourceBufferIterator users to specify how many bytes to advance. r=edwin
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.