From 6d97dbaa4192c312999e051afefe91d14a5723fb Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Sat, 22 Jul 2017 07:50:31 -0400 Subject: [PATCH] Bug 1315554 - Part 5. Add method to clone a SourceBufferIterator when decoding. r=tnikkel --- image/SourceBuffer.cpp | 24 ++++++++++- image/SourceBuffer.h | 57 +++++++++++++++++++++------ image/StreamingLexer.h | 46 +++++++++++++++++++++ image/test/gtest/TestSourceBuffer.cpp | 52 ++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 15 deletions(-) diff --git a/image/SourceBuffer.cpp b/image/SourceBuffer.cpp index de0719d4511f..c667338a7765 100644 --- a/image/SourceBuffer.cpp +++ b/image/SourceBuffer.cpp @@ -42,6 +42,7 @@ SourceBufferIterator::operator=(SourceBufferIterator&& aOther) mData = aOther.mData; mChunkCount = aOther.mChunkCount; mByteCount = aOther.mByteCount; + mRemainderToRead = aOther.mRemainderToRead; return *this; } @@ -63,6 +64,25 @@ SourceBufferIterator::AdvanceOrScheduleResume(size_t aRequestedBytes, MOZ_ASSERT(mData.mIterating.mNextReadLength <= mData.mIterating.mAvailableLength); mData.mIterating.mOffset += mData.mIterating.mNextReadLength; mData.mIterating.mAvailableLength -= mData.mIterating.mNextReadLength; + + // An iterator can have a limit imposed on it to read only a subset of a + // source buffer. If it is present, we need to mimic the same behaviour as + // the owning SourceBuffer. + if (MOZ_UNLIKELY(mRemainderToRead != SIZE_MAX)) { + MOZ_ASSERT(mData.mIterating.mNextReadLength <= mRemainderToRead); + mRemainderToRead -= mData.mIterating.mNextReadLength; + + if (MOZ_UNLIKELY(mRemainderToRead == 0)) { + mData.mIterating.mNextReadLength = 0; + SetComplete(NS_OK); + return COMPLETE; + } + + if (MOZ_UNLIKELY(aRequestedBytes > mRemainderToRead)) { + aRequestedBytes = mRemainderToRead; + } + } + mData.mIterating.mNextReadLength = 0; if (MOZ_LIKELY(mState == READY)) { @@ -518,14 +538,14 @@ SourceBuffer::SizeOfIncludingThisWithComputedFallback(MallocSizeOf } SourceBufferIterator -SourceBuffer::Iterator() +SourceBuffer::Iterator(size_t aReadLength) { { MutexAutoLock lock(mMutex); mConsumerCount++; } - return SourceBufferIterator(this); + return SourceBufferIterator(this, aReadLength); } void diff --git a/image/SourceBuffer.h b/image/SourceBuffer.h index 7d14ead1bb98..7cbf1569f5ca 100644 --- a/image/SourceBuffer.h +++ b/image/SourceBuffer.h @@ -77,11 +77,12 @@ public: COMPLETE // The iterator is pointing to the end of the buffer. }; - explicit SourceBufferIterator(SourceBuffer* aOwner) + explicit SourceBufferIterator(SourceBuffer* aOwner, size_t aReadLimit) : mOwner(aOwner) , mState(START) , mChunkCount(0) , mByteCount(0) + , mRemainderToRead(aReadLimit) { MOZ_ASSERT(aOwner); mData.mIterating.mChunk = 0; @@ -97,6 +98,7 @@ public: , mData(aOther.mData) , mChunkCount(aOther.mChunkCount) , mByteCount(aOther.mByteCount) + , mRemainderToRead(aOther.mRemainderToRead) { } ~SourceBufferIterator(); @@ -179,6 +181,19 @@ public: /// @return a count of the bytes in all chunks we've advanced through. size_t ByteCount() const { return mByteCount; } + /// @return the source buffer which owns the iterator. + SourceBuffer* Owner() const + { + MOZ_ASSERT(mOwner); + return mOwner; + } + + /// @return the current offset from the beginning of the buffer. + size_t Position() const + { + return mByteCount - mData.mIterating.mAvailableLength; + } + private: friend class SourceBuffer; @@ -208,6 +223,11 @@ private: MOZ_ASSERT(mState != COMPLETE); mState = READY; + // Prevent the iterator from reporting more data than it is allowed to read. + if (aAvailableLength > mRemainderToRead) { + aAvailableLength = mRemainderToRead; + } + // Update state. mData.mIterating.mChunk = aChunk; mData.mIterating.mData = aData; @@ -246,19 +266,27 @@ private: */ union { struct { - uint32_t mChunk; - const char* mData; - size_t mOffset; - size_t mAvailableLength; - size_t mNextReadLength; - } mIterating; + uint32_t mChunk; // Index of the chunk in SourceBuffer. + const char* mData; // Pointer to the start of the chunk. + size_t mOffset; // Current read position of the iterator relative to + // mData. + size_t mAvailableLength; // How many bytes remain unread in the chunk, + // relative to mOffset. + size_t mNextReadLength; // How many bytes the last iterator advance + // requested to be read, so that we know much + // to increase mOffset and reduce mAvailableLength + // by when the next advance is requested. + } mIterating; // Cached info of the chunk currently iterating over. struct { - nsresult mStatus; - } mAtEnd; + nsresult mStatus; // Status code indicating if we read all the data. + } mAtEnd; // State info after iterator is complete. } mData; - uint32_t mChunkCount; // Count of chunks we've advanced through. - size_t mByteCount; // Count of bytes in all chunks we've advanced through. + uint32_t mChunkCount; // Count of chunks observed, including current chunk. + size_t mByteCount; // Count of readable bytes observed, including unread + // bytes from the current chunk. + size_t mRemainderToRead; // Count of bytes left to read if there is a maximum + // imposed by the caller. SIZE_MAX if unlimited. }; /** @@ -319,8 +347,11 @@ public: // Consumer methods. ////////////////////////////////////////////////////////////////////////////// - /// Returns an iterator to this SourceBuffer. - SourceBufferIterator Iterator(); + /** + * Returns an iterator to this SourceBuffer, which cannot read more than the + * given length. + */ + SourceBufferIterator Iterator(size_t aReadLength = SIZE_MAX); ////////////////////////////////////////////////////////////////////////////// diff --git a/image/StreamingLexer.h b/image/StreamingLexer.h index 46e1e71d9097..8b3d5e012e5e 100644 --- a/image/StreamingLexer.h +++ b/image/StreamingLexer.h @@ -393,6 +393,52 @@ public: SetTransition(aStartState); } + /** + * From the given SourceBufferIterator, aIterator, create a new iterator at + * the same position, with the given read limit, aReadLimit. The read limit + * applies after adjusting for the position. If the given iterator has been + * advanced, but required buffering inside StreamingLexer, the position + * of the cloned iterator will be at the beginning of buffered data; this + * should match the perspective of the caller. + */ + SourceBufferIterator Clone(SourceBufferIterator& aIterator, + size_t aReadLimit) const + { + // In order to advance to the current position of the iterator from the + // perspective of the caller, we need to take into account if we are + // buffering data. + size_t pos = aIterator.Position(); + if (!mBuffer.empty()) { + pos += aIterator.Length(); + MOZ_ASSERT(pos > mBuffer.length()); + pos -= mBuffer.length(); + } + + size_t readLimit = aReadLimit; + if (aReadLimit != SIZE_MAX) { + readLimit += pos; + } + + SourceBufferIterator other = aIterator.Owner()->Iterator(readLimit); + + // Since the current iterator has already advanced to this point, we + // know that the state can only be READY or COMPLETE. That does not mean + // everything is stored in a single chunk, and may require multiple Advance + // calls to get where we want to be. + DebugOnly state; + do { + state = other.Advance(pos); + MOZ_ASSERT(state != SourceBufferIterator::WAITING); + MOZ_ASSERT(pos >= other.Length()); + pos -= other.Length(); + } while (pos > 0); + + // Force the data pointer to be where we expect it to be. + state = other.Advance(0); + MOZ_ASSERT(state != SourceBufferIterator::WAITING); + return other; + } + template LexerResult Lex(SourceBufferIterator& aIterator, IResumable* aOnResume, diff --git a/image/test/gtest/TestSourceBuffer.cpp b/image/test/gtest/TestSourceBuffer.cpp index 05a88093f50c..ae70753ecf01 100644 --- a/image/test/gtest/TestSourceBuffer.cpp +++ b/image/test/gtest/TestSourceBuffer.cpp @@ -808,3 +808,55 @@ TEST_F(ImageSourceBuffer, ExpectLengthDoesNotTriggerResume) // ensure that the test fails. mSourceBuffer->ExpectLength(1000); } + +TEST_F(ImageSourceBuffer, CompleteSuccessWithSameReadLength) +{ + SourceBufferIterator iterator = mSourceBuffer->Iterator(1); + + // Write a single byte to the buffer and complete the buffer. (We have to + // write at least one byte because completing a zero length buffer always + // fails; see the ZeroLengthBufferAlwaysFails test.) + CheckedAppendToBuffer(mData, 1); + CheckedCompleteBuffer(iterator, 1); + + // We should be able to advance once (to read the single byte) and then should + // reach the COMPLETE state with a successful status. + CheckedAdvanceIterator(iterator, 1); + CheckIteratorIsComplete(iterator, 1); +} + +TEST_F(ImageSourceBuffer, CompleteSuccessWithSmallerReadLength) +{ + // Create an iterator limited to one byte. + SourceBufferIterator iterator = mSourceBuffer->Iterator(1); + + // Write two bytes to the buffer and complete the buffer. (We have to + // write at least one byte because completing a zero length buffer always + // fails; see the ZeroLengthBufferAlwaysFails test.) + CheckedAppendToBuffer(mData, 2); + CheckedCompleteBuffer(iterator, 2); + + // We should be able to advance once (to read the single byte) and then should + // reach the COMPLETE state with a successful status, because our iterator is + // limited to a single byte, rather than the full length. + CheckedAdvanceIterator(iterator, 1); + CheckIteratorIsComplete(iterator, 1); +} + +TEST_F(ImageSourceBuffer, CompleteSuccessWithGreaterReadLength) +{ + // Create an iterator limited to one byte. + SourceBufferIterator iterator = mSourceBuffer->Iterator(2); + + // Write a single byte to the buffer and complete the buffer. (We have to + // write at least one byte because completing a zero length buffer always + // fails; see the ZeroLengthBufferAlwaysFails test.) + CheckedAppendToBuffer(mData, 1); + CheckedCompleteBuffer(iterator, 1); + + // We should be able to advance once (to read the single byte) and then should + // reach the COMPLETE state with a successful status. Our iterator lets us + // read more but the underlying buffer has been completed. + CheckedAdvanceIterator(iterator, 1); + CheckIteratorIsComplete(iterator, 1); +}