Bug 1185799 (Part 3) - Make nsICODecoder use only the public Decoder interface for writing to its contained decoder. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Sat, 02 Jul 2016 23:21:00 -0600
changeset 304070 038d7b021492e648483aba1f1291ad7e902d4611
parent 304069 090ab64054fd86f10f7f30c378c84bb57d112f62
child 304071 2b7c2618bc0320de21cf93cf116f12150cdd9465
push id79247
push usermfowler@mozilla.com
push dateThu, 07 Jul 2016 22:51:00 +0000
treeherdermozilla-inbound@038d7b021492 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1185799
milestone50.0a1
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
Bug 1185799 (Part 3) - Make nsICODecoder use only the public Decoder interface for writing to its contained decoder. r=edwin
image/Decoder.cpp
image/Decoder.h
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -65,16 +65,19 @@ Decoder::~Decoder()
  */
 
 void
 Decoder::Init()
 {
   // No re-initializing
   MOZ_ASSERT(!mInitialized, "Can't re-initialize a decoder!");
 
+  // All decoders must have a SourceBufferIterator.
+  MOZ_ASSERT(mIterator);
+
   // It doesn't make sense to decode anything but the first frame if we can't
   // store anything in the SurfaceCache, since only the last frame we decode
   // will be retrievable.
   MOZ_ASSERT(ShouldUseSurfaceCache() || IsFirstFrameDecode());
 
   // Implementation-specific initialization
   InitInternal();
 
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -128,22 +128,20 @@ public:
    * -moz-sample-size media fragment.
    *
    *  XXX(seth): Support for -moz-sample-size will be removed in bug 1120056.
    */
   virtual void SetSampleSize(int aSampleSize) { }
 
   /**
    * Set an iterator to the SourceBuffer which will feed data to this decoder.
+   * This must always be called before calling Init(). (And only before Init().)
    *
-   * This should be called for almost all decoders; the exceptions are the
-   * contained decoders of an nsICODecoder, which will be fed manually via Write
-   * instead.
-   *
-   * This must be called before Init() is called.
+   * XXX(seth): We should eliminate this method and pass a SourceBufferIterator
+   * to the various decoder constructors instead.
    */
   void SetIterator(SourceBufferIterator&& aIterator)
   {
     MOZ_ASSERT(!mInitialized, "Shouldn't be initialized yet");
     mIterator.emplace(Move(aIterator));
   }
 
   /**
@@ -267,27 +265,16 @@ public:
   RasterImage* GetImageMaybeNull() const { return mImage.get(); }
 
   RawAccessFrameRef GetCurrentFrameRef()
   {
     return mCurrentFrame ? mCurrentFrame->RawAccessRef()
                          : RawAccessFrameRef();
   }
 
-  /**
-   * Writes data to the decoder. Only public for the benefit of nsICODecoder;
-   * other callers should use Decode().
-   *
-   * @param aBuffer buffer containing the data to be written
-   * @param aCount the number of bytes to write
-   *
-   * Any errors are reported by setting the appropriate state on the decoder.
-   */
-  void Write(const char* aBuffer, uint32_t aCount);
-
 
 protected:
   friend class nsICODecoder;
   friend class PalettedSurfaceSink;
   friend class SurfaceSink;
 
   virtual ~Decoder();
 
@@ -368,16 +355,26 @@ protected:
   // means a single iteration, stopping on the last frame.
   void PostDecodeDone(int32_t aLoopCount = 0);
 
   // Data errors are the fault of the source data, decoder errors are our fault
   void PostDataError();
   void PostDecoderError(nsresult aFailCode);
 
   /**
+   * Called by Decode() to write data to the decoder.
+   *
+   * @param aBuffer A buffer containing the data to be written.
+   * @param aCount The number of bytes to write.
+   *
+   * Any errors are reported by setting the appropriate state on the decoder.
+   */
+  void Write(const char* aBuffer, uint32_t aCount);
+
+  /**
    * CompleteDecode() finishes up the decoding process after Decode() determines
    * that we're finished. It records final progress and does all the cleanup
    * that's possible off-main-thread.
    */
   void CompleteDecode();
 
   /**
    * Allocates a new frame, making it our current frame if successful.
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -49,16 +49,17 @@ nsICODecoder::GetNumColors()
     }
   }
   return numColors;
 }
 
 nsICODecoder::nsICODecoder(RasterImage* aImage)
   : Decoder(aImage)
   , mLexer(Transition::To(ICOState::HEADER, ICOHEADERSIZE))
+  , mDoNotResume(WrapNotNull(new DoNotResume))
   , mBiggestResourceColorDepth(0)
   , mBestResourceDelta(INT_MIN)
   , mBestResourceColorDepth(0)
   , mNumIcons(0)
   , mCurrIcon(0)
   , mBPP(0)
   , mMaskRowSize(0)
   , mCurrMaskLine(0)
@@ -83,19 +84,28 @@ nsICODecoder::FinishWithErrorInternal()
 
 void
 nsICODecoder::GetFinalStateFromContainedDecoder()
 {
   if (!mContainedDecoder) {
     return;
   }
 
-  // Finish the internally used decoder.
-  mContainedDecoder->CompleteDecode();
+  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);
+    if (NS_FAILED(mContainedDecoder->Decode(mDoNotResume))) {
+      PostDataError();
+    }
+  }
+
+  // Make our state the same as the state of the contained decoder.
   mDecodeDone = mContainedDecoder->GetDecodeDone();
   mDataError = mDataError || mContainedDecoder->HasDataError();
   mFailCode = NS_SUCCEEDED(mFailCode) ? mContainedDecoder->GetDecoderError()
                                       : mFailCode;
   mDecodeAborted = mContainedDecoder->WasAborted();
   mProgress |= mContainedDecoder->TakeProgress();
   mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect());
   mCurrentFrame = mContainedDecoder->GetCurrentFrameRef();
@@ -287,16 +297,17 @@ nsICODecoder::SniffResource(const char* 
 {
   // 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) {
     // 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),
                                                   WrapNotNull(this));
 
     if (!WriteToContainedDecoder(aData, PNGSIGNATURESIZE)) {
       return Transition::TerminateFailure();
     }
@@ -364,16 +375,17 @@ nsICODecoder::ReadBIH(const char* aData)
       return Transition::TerminateFailure();
     }
     dataOffset += 4 * numColors;
   }
 
   // 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),
                                                 WrapNotNull(this),
                                                 Some(dataOffset));
   RefPtr<nsBMPDecoder> bmpDecoder =
     static_cast<nsBMPDecoder*>(mContainedDecoder.get());
 
@@ -630,22 +642,38 @@ nsICODecoder::WriteInternal(const char* 
   if (terminalState == Some(TerminalState::FAILURE)) {
     PostDataError();
   }
 }
 
 bool
 nsICODecoder::WriteToContainedDecoder(const char* aBuffer, uint32_t aCount)
 {
-  mContainedDecoder->Write(aBuffer, aCount);
+  MOZ_ASSERT(mContainedDecoder);
+  MOZ_ASSERT(mContainedSourceBuffer);
+
+  // Append the provided data to the SourceBuffer that the contained decoder is
+  // reading from.
+  mContainedSourceBuffer->Append(aBuffer, aCount);
+
+  // 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 (NS_FAILED(mContainedDecoder->Decode(mDoNotResume))) {
+    PostDataError();
+  }
+
+  // Make our state the same as the state of the contained decoder.
   mProgress |= mContainedDecoder->TakeProgress();
   mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect());
   if (mContainedDecoder->HasDataError()) {
     PostDataError();
   }
   if (mContainedDecoder->HasDecoderError()) {
     PostDecoderError(mContainedDecoder->GetDecoderError());
   }
+
   return !HasError();
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -5,20 +5,21 @@
 
 
 #ifndef mozilla_image_decoders_nsICODecoder_h
 #define mozilla_image_decoders_nsICODecoder_h
 
 #include "StreamingLexer.h"
 #include "Decoder.h"
 #include "imgFrame.h"
+#include "mozilla/gfx/2D.h"
+#include "mozilla/NotNull.h"
 #include "nsBMPDecoder.h"
 #include "nsPNGDecoder.h"
 #include "ICOFileHeaders.h"
-#include "mozilla/gfx/2D.h"
 
 namespace mozilla {
 namespace image {
 
 class RasterImage;
 
 enum class ICOState
 {
@@ -105,19 +106,32 @@ private:
   LexerTransition<ICOState> ReadPNG(const char* aData, uint32_t aLen);
   LexerTransition<ICOState> ReadBIH(const char* aData);
   LexerTransition<ICOState> ReadBMP(const char* aData, uint32_t aLen);
   LexerTransition<ICOState> PrepareForMask();
   LexerTransition<ICOState> ReadMaskRow(const char* aData);
   LexerTransition<ICOState> FinishMask();
   LexerTransition<ICOState> FinishResource();
 
+  // A helper implementation of IResumable which just does nothing; see
+  // WriteToContainedDecoder() for more details.
+  class DoNotResume final : public IResumable
+  {
+  public:
+    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DoNotResume, override)
+    void Resume() override { }
+
+  private:
+    virtual ~DoNotResume() { }
+  };
+
   StreamingLexer<ICOState, 32> mLexer; // The lexer.
   RefPtr<Decoder> mContainedDecoder; // Either a BMP or PNG decoder.
   RefPtr<SourceBuffer> mContainedSourceBuffer;  // SourceBuffer for mContainedDecoder.
+  NotNull<RefPtr<IResumable>> mDoNotResume;  // IResumable helper for SourceBuffer.
   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.
   gfx::IntSize mBiggestResourceSize;   // Used to select the intrinsic size.
   gfx::IntSize mBiggestResourceHotSpot; // Used to select the intrinsic size.
   uint16_t mBiggestResourceColorDepth; // Used to select the intrinsic size.
   int32_t mBestResourceDelta;          // Used to select the best resource.
   uint16_t mBestResourceColorDepth;    // Used to select the best resource.