Closed Bug 1286165 Opened 8 years ago Closed 8 years ago

Advance decoders' SourceBufferIterator directly in StreamingLexer

Categories

(Core :: Graphics: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(3 files)

As discussed in bug 1284031 comment 0, we need StreamingLexer to interact with SourceBufferIterator more directly so we can efficiently support yielding, which will allow for massive improvements to animated image playback.

In this bug we'll move the loop that advances a Decoder's SourceBufferIterator from Decoder::Decode() to StreamingLexer.
Depends on: 1285867
Depends on: 1286168
Blocks: 1286175
In part 2 we'll start passing an IResumable from Decoder::Decode() all the way
down to StreamingLexer. Since we don't always need an IResumable, we don't
really want to impose the requirement that we have one, so let's make
SourceBufferIterator and SourceBuffer allow a null IResumable. This will be
particularly handy in the case of nsICODecoder, which up to now has been
pointlessly allocating an IResumable that doesn't do anything.
Attachment #8770037 - Flags: review?(edwin)
Here's the meat of the patch. We want to move the loop that advances the
SourceBufferIterator from Decoder::Decode() into StreamingLexer. To keep the
individual steps in this patch series management, I've implemented this as a
second overload to Lex() which wraps the original one. Later on, when the time
comes to implement yield support, these things will become more tightly coupled.

All of the decoders need to be updated to call the new Lex() overload, and we
need the ability to pass an IResumable to them. This means we need to touch
DoDecode()'s interface yet again. Sorry, Edwin. =)
Attachment #8770042 - Flags: review?(n.nethercote)
Attachment #8770042 - Flags: review?(edwin)
Here's an update to the StreamingLexer tests to cover the new Lex() overload. I
needed some IResumable helper implementations for these tests; they're in
Common.h because we'll want to use them in the upcoming SourceBuffer test suite,
too.
Attachment #8770043 - Flags: review?(n.nethercote)
Comment on attachment 8770042 [details] [diff] [review]
(Part 2) - Advance decoders' SourceBufferIterator directly in StreamingLexer.

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

rs=me
Attachment #8770042 - Flags: review?(n.nethercote) → review+
Comment on attachment 8770043 [details] [diff] [review]
(Part 3) - Add StreamingLexer tests for the SourceBufferIterator version of Lex().

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

rs=me
Attachment #8770043 - Flags: review?(n.nethercote) → review+
Pushed by mfowler@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab4ae8b37941
(Part 1) - Allow advancing a SourceBufferIterator without an IResumable. r=edwin
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2069b87af4c
(Part 2) - Advance decoders' SourceBufferIterator directly in StreamingLexer. r=edwin,njn
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf3df70f738e
(Part 3) - Add StreamingLexer tests for the SourceBufferIterator version of Lex(). r=njn
You need to log in before you can comment on or make changes to this bug.