Backed out changeset 6fa3ad97ce9c (bug 1315554)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 22 Jul 2017 11:04:07 +0200
changeset 419101 2df50fe08bc681d471b8f351b8de8337d41adb4e
parent 419100 aee78154a945c91d826a856d7ad1dfd3553d9eeb
child 419102 a652fc078608bc1086574d6eeb89e45e9e7ce5a4
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1315554
milestone56.0a1
backs out6fa3ad97ce9c28ac9e66adf75c37142b0a9e21e1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 6fa3ad97ce9c (bug 1315554)
image/SourceBuffer.cpp
image/SourceBuffer.h
image/StreamingLexer.h
image/test/gtest/TestSourceBuffer.cpp
--- a/image/SourceBuffer.cpp
+++ b/image/SourceBuffer.cpp
@@ -37,17 +37,16 @@ SourceBufferIterator::operator=(SourceBu
     mOwner->OnIteratorRelease();
   }
 
   mOwner = Move(aOther.mOwner);
   mState = aOther.mState;
   mData = aOther.mData;
   mChunkCount = aOther.mChunkCount;
   mByteCount = aOther.mByteCount;
-  mRemainderToRead = aOther.mRemainderToRead;
 
   return *this;
 }
 
 SourceBufferIterator::State
 SourceBufferIterator::AdvanceOrScheduleResume(size_t aRequestedBytes,
                                               IResumable* aConsumer)
 {
@@ -59,35 +58,16 @@ SourceBufferIterator::AdvanceOrScheduleR
   }
 
   // The range of data [mOffset, mOffset + mNextReadLength) has just been read
   // by the caller (or at least they don't have any interest in it), so consume
   // that data.
   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)) {
     // If the caller wants zero bytes of data, that's easy enough; we just
     // configured ourselves for a zero-byte read above!  In theory we could do
     // this even in the START state, but it's not important for performance and
     // breaking the ability of callers to assert that the pointer returned by
     // Data() is non-null doesn't seem worth it.
@@ -533,24 +513,24 @@ SourceBuffer::SizeOfIncludingThisWithCom
 
     n += chunkSize;
   }
 
   return n;
 }
 
 SourceBufferIterator
-SourceBuffer::Iterator(size_t aReadLength)
+SourceBuffer::Iterator()
 {
   {
     MutexAutoLock lock(mMutex);
     mConsumerCount++;
   }
 
-  return SourceBufferIterator(this, aReadLength);
+  return SourceBufferIterator(this);
 }
 
 void
 SourceBuffer::OnIteratorRelease()
 {
   MutexAutoLock lock(mMutex);
 
   MOZ_ASSERT(mConsumerCount > 0, "Consumer count doesn't add up");
--- a/image/SourceBuffer.h
+++ b/image/SourceBuffer.h
@@ -72,38 +72,36 @@ class SourceBufferIterator final
 public:
   enum State {
     START,    // The iterator is at the beginning of the buffer.
     READY,    // The iterator is pointing to new data.
     WAITING,  // The iterator is blocked and the caller must yield.
     COMPLETE  // The iterator is pointing to the end of the buffer.
   };
 
-  explicit SourceBufferIterator(SourceBuffer* aOwner, size_t aReadLimit)
+  explicit SourceBufferIterator(SourceBuffer* aOwner)
     : mOwner(aOwner)
     , mState(START)
     , mChunkCount(0)
     , mByteCount(0)
-    , mRemainderToRead(aReadLimit)
   {
     MOZ_ASSERT(aOwner);
     mData.mIterating.mChunk = 0;
     mData.mIterating.mData = nullptr;
     mData.mIterating.mOffset = 0;
     mData.mIterating.mAvailableLength = 0;
     mData.mIterating.mNextReadLength = 0;
   }
 
   SourceBufferIterator(SourceBufferIterator&& aOther)
     : mOwner(Move(aOther.mOwner))
     , mState(aOther.mState)
     , mData(aOther.mData)
     , mChunkCount(aOther.mChunkCount)
     , mByteCount(aOther.mByteCount)
-    , mRemainderToRead(aOther.mRemainderToRead)
   { }
 
   ~SourceBufferIterator();
 
   SourceBufferIterator& operator=(SourceBufferIterator&& aOther);
 
   /**
    * Returns true if there are no more than @aBytes remaining in the
@@ -176,29 +174,16 @@ public:
   }
 
   /// @return a count of the chunks we've advanced through.
   uint32_t ChunkCount() const { return mChunkCount; }
 
   /// @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;
 
   SourceBufferIterator(const SourceBufferIterator&) = delete;
   SourceBufferIterator& operator=(const SourceBufferIterator&) = delete;
 
   bool HasMore() const { return mState != COMPLETE; }
 
@@ -218,21 +203,16 @@ private:
 
   State SetReady(uint32_t aChunk, const char* aData,
                  size_t aOffset, size_t aAvailableLength,
                  size_t aRequestedBytes)
   {
     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;
     mData.mIterating.mOffset = aOffset;
     mData.mIterating.mAvailableLength = aAvailableLength;
 
     // Update metrics.
     mChunkCount++;
@@ -261,37 +241,29 @@ private:
 
   /**
    * This union contains our iteration state if we're still iterating (for
    * states START, READY, and WAITING) and the status the SourceBuffer was
    * completed with if we're in state COMPLETE.
    */
   union {
     struct {
-      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.
+      uint32_t mChunk;
+      const char* mData;
+      size_t mOffset;
+      size_t mAvailableLength;
+      size_t mNextReadLength;
+    } mIterating;
     struct {
-      nsresult mStatus;  // Status code indicating if we read all the data.
-    } mAtEnd;            // State info after iterator is complete.
+      nsresult mStatus;
+    } mAtEnd;
   } mData;
 
-  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.
+  uint32_t mChunkCount;  // Count of chunks we've advanced through.
+  size_t mByteCount;     // Count of bytes in all chunks we've advanced through.
 };
 
 /**
  * SourceBuffer is a parallel data structure used for storing image source
  * (compressed) data.
  *
  * SourceBuffer is a single producer, multiple consumer data structure. The
  * single producer calls Append() to append data to the buffer. In parallel,
@@ -342,21 +314,18 @@ public:
   /// Memory reporting.
   size_t SizeOfIncludingThisWithComputedFallback(MallocSizeOf) const;
 
 
   //////////////////////////////////////////////////////////////////////////////
   // Consumer methods.
   //////////////////////////////////////////////////////////////////////////////
 
-  /**
-   * Returns an iterator to this SourceBuffer, which cannot read more than the
-   * given length.
-   */
-  SourceBufferIterator Iterator(size_t aReadLength = SIZE_MAX);
+  /// Returns an iterator to this SourceBuffer.
+  SourceBufferIterator Iterator();
 
 
   //////////////////////////////////////////////////////////////////////////////
   // Consumer methods.
   //////////////////////////////////////////////////////////////////////////////
 
   /**
    * The minimum chunk capacity we'll allocate, if we don't know the correct
--- a/image/StreamingLexer.h
+++ b/image/StreamingLexer.h
@@ -388,62 +388,16 @@ public:
       // zero bytes.
       MOZ_ASSERT_UNREACHABLE("Truncated state makes no sense");
       return;
     }
 
     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<SourceBufferIterator::State> 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 <typename Func>
   LexerResult Lex(SourceBufferIterator& aIterator,
                   IResumable* aOnResume,
                   Func aFunc)
   {
     if (mTransition.NextStateIsTerminal()) {
       // We've already reached a terminal state. We never deliver any more data
       // in this case; just return the terminal state again immediately.
--- a/image/test/gtest/TestSourceBuffer.cpp
+++ b/image/test/gtest/TestSourceBuffer.cpp
@@ -803,60 +803,8 @@ TEST_F(ImageSourceBuffer, ExpectLengthDo
 
   // Check that we can't advance.
   CheckIteratorMustWait(iterator, mExpectNoResume);
 
   // Call ExpectLength(). If this triggers a resume, |mExpectNoResume| will
   // 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);
-}