Bug 1315554 - Part 6. Reuse the same SourceBuffer when decoding a resource within an ICO. r=tnikkel

Author: aosmond

image/DecoderFactory.cpp

@@ -234,7 +234,7 @@ DecoderFactory::CreateMetadataDecoder(DecoderType aType,
 
 /* static */ already_AddRefed<Decoder>
 DecoderFactory::CreateDecoderForICOResource(DecoderType aType,
-                                            NotNull<SourceBuffer*> aSourceBuffer,
+                                            SourceBufferIterator&& aIterator,
                                             NotNull<nsICODecoder*> aICODecoder,
                                             const Maybe<IntSize>& aExpectedSize,
                                             const Maybe<uint32_t>& aDataOffset
@@ -263,7 +263,7 @@ DecoderFactory::CreateDecoderForICOResource(DecoderType aType,
   // Initialize the decoder, copying settings from @aICODecoder.
   MOZ_ASSERT(!aICODecoder->IsMetadataDecode());
   decoder->SetMetadataDecode(aICODecoder->IsMetadataDecode());
-  decoder->SetIterator(aSourceBuffer->Iterator());
+  decoder->SetIterator(Forward<SourceBufferIterator>(aIterator));
   decoder->SetOutputSize(aICODecoder->OutputSize());
   if (aExpectedSize) {
     decoder->SetExpectedSize(*aExpectedSize);

image/DecoderFactory.h

@@ -23,6 +23,7 @@ class IDecodingTask;
 class nsICODecoder;
 class RasterImage;
 class SourceBuffer;
+class SourceBufferIterator;
 
 /**
  * The type of decoder; this is usually determined from a MIME type using
@@ -119,8 +120,8 @@ class DecoderFactory
    *
    * @param aType Which type of decoder to create. This must be either BMP or
    *              PNG.
-   * @param aSourceBuffer The SourceBuffer which the decoder will read its data
-   *                      from.
+   * @param aIterator The SourceBufferIterator which the decoder will read its
+   *                  data from.
    * @param aICODecoder The ICO decoder which is controlling this resource
    *                    decoder. @aICODecoder's settings will be copied to the
    *                    resource decoder, so the two decoders will have the
@@ -133,7 +134,7 @@ class DecoderFactory
    */
   static already_AddRefed<Decoder>
   CreateDecoderForICOResource(DecoderType aType,
-                              NotNull<SourceBuffer*> aSourceBuffer,
+                              SourceBufferIterator&& aIterator,
                               NotNull<nsICODecoder*> aICODecoder,
                               const Maybe<gfx::IntSize>& aExpectedSize,
                               const Maybe<uint32_t>& aDataOffset = Nothing());

image/decoders/nsICODecoder.cpp

@@ -91,14 +91,8 @@ nsICODecoder::GetFinalStateFromContainedDecoder()
     return NS_OK;
   }
 
-  MOZ_ASSERT(mContainedSourceBuffer,
-             "Should have a SourceBuffer if we have a decoder");
-
   // Let the contained decoder finish up if necessary.
-  if (!mContainedSourceBuffer->IsComplete()) {
-    mContainedSourceBuffer->Complete(NS_OK);
-    mContainedDecoder->Decode();
-  }
+  FlushContainedDecoder();
 
   // Make our state the same as the state of the contained decoder.
   mDecodeDone = mContainedDecoder->GetDecodeDone();
@@ -255,28 +249,26 @@ nsICODecoder::ReadDirEntry(const char* aData)
 LexerTransition<ICOState>
 nsICODecoder::SniffResource(const char* aData)
 {
+  // Prepare a new iterator for the contained decoder to advance as it wills.
+  // Cloning at the point ensures it will begin at the resource offset.
+  mContainedIterator.emplace(mLexer.Clone(*mIterator, mDirEntry.mBytesInRes));
+
   // We use the first PNGSIGNATURESIZE bytes to determine whether this resource
   // is a PNG or a BMP.
   bool isPNG = !memcmp(aData, nsPNGDecoder::pngSignatureBytes,
                        PNGSIGNATURESIZE);
   if (isPNG) {
+    if (mDirEntry.mBytesInRes <= PNGSIGNATURESIZE) {
+      return Transition::TerminateFailure();
+    }
+
     // Create a PNG decoder which will do the rest of the work for us.
-    mContainedSourceBuffer = new SourceBuffer();
-    mContainedSourceBuffer->ExpectLength(mDirEntry.mBytesInRes);
     mContainedDecoder =
       DecoderFactory::CreateDecoderForICOResource(DecoderType::PNG,
-                                                  WrapNotNull(mContainedSourceBuffer),
+                                                  Move(*mContainedIterator),
                                                   WrapNotNull(this),
                                                   Some(GetRealSize()));
 
-    if (!WriteToContainedDecoder(aData, PNGSIGNATURESIZE)) {
-      return Transition::TerminateFailure();
-    }
-
-    if (mDirEntry.mBytesInRes <= PNGSIGNATURESIZE) {
-      return Transition::TerminateFailure();
-    }
-
     // Read in the rest of the PNG unbuffered.
     size_t toRead = mDirEntry.mBytesInRes - PNGSIGNATURESIZE;
     return Transition::ToUnbuffered(ICOState::FINISHED_RESOURCE,
@@ -299,9 +291,9 @@ nsICODecoder::SniffResource(const char* aData)
 }
 
 LexerTransition<ICOState>
-nsICODecoder::ReadResource(const char* aData, uint32_t aLen)
+nsICODecoder::ReadResource()
 {
-  if (!WriteToContainedDecoder(aData, aLen)) {
+  if (!FlushContainedDecoder()) {
     return Transition::TerminateFailure();
   }
 
@@ -334,19 +326,18 @@ nsICODecoder::ReadBIH(const char* aData)
 
   // Create a BMP decoder which will do most of the work for us; the exception
   // is the AND mask, which isn't present in standalone BMPs.
-  mContainedSourceBuffer = new SourceBuffer();
-  mContainedSourceBuffer->ExpectLength(mDirEntry.mBytesInRes);
   mContainedDecoder =
     DecoderFactory::CreateDecoderForICOResource(DecoderType::BMP,
-                                                WrapNotNull(mContainedSourceBuffer),
+                                                Move(*mContainedIterator),
                                                 WrapNotNull(this),
                                                 Some(GetRealSize()),
                                                 Some(dataOffset));
+
   RefPtr<nsBMPDecoder> bmpDecoder =
     static_cast<nsBMPDecoder*>(mContainedDecoder.get());
 
-  // Write out the BMP's bitmap info header.
-  if (!WriteToContainedDecoder(mBIHraw, sizeof(mBIHraw))) {
+  // Ensure the decoder has parsed at least the BMP's bitmap info header.
+  if (!FlushContainedDecoder()) {
     return Transition::TerminateFailure();
   }
 
@@ -372,6 +363,14 @@ nsICODecoder::ReadBIH(const char* aData)
 LexerTransition<ICOState>
 nsICODecoder::PrepareForMask()
 {
+  // We have received all of the data required by the BMP decoder so flushing
+  // here guarantees the decode has finished.
+  if (!FlushContainedDecoder()) {
+    return Transition::TerminateFailure();
+  }
+
+  MOZ_ASSERT(mContainedDecoder->GetDecodeDone());
+
   RefPtr<nsBMPDecoder> bmpDecoder =
     static_cast<nsBMPDecoder*>(mContainedDecoder.get());
 
@@ -517,6 +516,14 @@ nsICODecoder::FinishMask()
 LexerTransition<ICOState>
 nsICODecoder::FinishResource()
 {
+  // We have received all of the data required by the PNG/BMP decoder so
+  // flushing here guarantees the decode has finished.
+  if (!FlushContainedDecoder()) {
+    return Transition::TerminateFailure();
+  }
+
+  MOZ_ASSERT(mContainedDecoder->GetDecodeDone());
+
   // Make sure the actual size of the resource matches the size in the directory
   // entry. If not, we consider the image corrupt.
   if (mContainedDecoder->HasSize() &&
@@ -559,7 +566,7 @@ nsICODecoder::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
       case ICOState::SNIFF_RESOURCE:
         return SniffResource(aData);
       case ICOState::READ_RESOURCE:
-        return ReadResource(aData, aLength);
+        return ReadResource();
       case ICOState::READ_BIH:
         return ReadBIH(aData);
       case ICOState::PREPARE_FOR_MASK:
@@ -579,21 +586,16 @@ nsICODecoder::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
 }
 
 bool
-nsICODecoder::WriteToContainedDecoder(const char* aBuffer, uint32_t aCount)
+nsICODecoder::FlushContainedDecoder()
 {
   MOZ_ASSERT(mContainedDecoder);
-  MOZ_ASSERT(mContainedSourceBuffer);
-
-  // Append the provided data to the SourceBuffer that the contained decoder is
-  // reading from.
-  mContainedSourceBuffer->Append(aBuffer, aCount);
 
   bool succeeded = true;
 
-  // Write to the contained decoder. If we run out of data, the ICO decoder will
-  // get resumed when there's more data available, as usual, so we don't need
-  // the contained decoder to get resumed too. To avoid that, we provide an
-  // IResumable which just does nothing.
+  // If we run out of data, the ICO decoder will get resumed when there's more
+  // data available, as usual, so we don't need the contained decoder to get
+  // resumed too. To avoid that, we provide an IResumable which just does
+  // nothing. All the caller needs to do is flush when there is new data.
   LexerResult result = mContainedDecoder->Decode();
   if (result == LexerResult(TerminalState::FAILURE)) {
     succeeded = false;

image/decoders/nsICODecoder.h

@@ -79,9 +79,9 @@ class nsICODecoder : public Decoder
   // Decoders should only be instantiated via DecoderFactory.
   explicit nsICODecoder(RasterImage* aImage);
 
-  // Writes to the contained decoder and sets the appropriate errors
-  // Returns true if there are no errors.
-  bool WriteToContainedDecoder(const char* aBuffer, uint32_t aCount);
+  // Flushes the contained decoder to read all available data and sets the
+  // appropriate errors. Returns true if there are no errors.
+  bool FlushContainedDecoder();
 
   // Gets decoder state from the contained decoder so it's visible externally.
   nsresult GetFinalStateFromContainedDecoder();
@@ -92,7 +92,7 @@ class nsICODecoder : public Decoder
   LexerTransition<ICOState> ReadHeader(const char* aData);
   LexerTransition<ICOState> ReadDirEntry(const char* aData);
   LexerTransition<ICOState> SniffResource(const char* aData);
-  LexerTransition<ICOState> ReadResource(const char* aData, uint32_t aLen);
+  LexerTransition<ICOState> ReadResource();
   LexerTransition<ICOState> ReadBIH(const char* aData);
   LexerTransition<ICOState> PrepareForMask();
   LexerTransition<ICOState> ReadMaskRow(const char* aData);
@@ -101,7 +101,7 @@ class nsICODecoder : public Decoder
 
   StreamingLexer<ICOState, 32> mLexer; // The lexer.
   RefPtr<Decoder> mContainedDecoder; // Either a BMP or PNG decoder.
-  RefPtr<SourceBuffer> mContainedSourceBuffer;  // SourceBuffer for mContainedDecoder.
+  Maybe<SourceBufferIterator> mContainedIterator; // Iterator for the subdecoder.
   UniquePtr<uint8_t[]> mMaskBuffer;    // A temporary buffer for the alpha mask.
   char mBIHraw[bmp::InfoHeaderLength::WIN_ICO]; // The bitmap information header.
   IconDirEntry mDirEntry;              // The dir entry for the selected resource.