Bug 1117607 - Make decoders responsible for their own frame allocations. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 10 Jul 2015 19:26:15 -0700
changeset 284125 aea2836ce9fecb317d04c3c49ce5122049239d96
parent 284124 4bd1786761cdb5f79eacd0678ac281ddafc08a83
child 284126 836d79194ff1bd3789b8d5a05af8ab858db20d58
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1117607
milestone42.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 1117607 - Make decoders responsible for their own frame allocations. r=tn
image/Decoder.cpp
image/Decoder.h
image/RasterImage.cpp
image/RasterImage.h
image/decoders/nsBMPDecoder.cpp
image/decoders/nsGIFDecoder2.cpp
image/decoders/nsGIFDecoder2.h
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
image/decoders/nsIconDecoder.cpp
image/decoders/nsJPEGDecoder.cpp
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
image/imgFrame.cpp
image/imgFrame.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -36,18 +36,16 @@ Decoder::Decoder(RasterImage* aImage)
   , mDecodeDone(false)
   , mDataError(false)
   , mDecodeAborted(false)
   , mShouldReportError(false)
   , mImageIsTransient(false)
   , mImageIsLocked(false)
   , mFrameCount(0)
   , mFailCode(NS_OK)
-  , mNeedsNewFrame(false)
-  , mNeedsToFlushData(false)
   , mInitialized(false)
   , mSizeDecode(false)
   , mInFrame(false)
   , mIsAnimated(false)
 { }
 
 Decoder::~Decoder()
 {
@@ -84,43 +82,16 @@ Decoder::Init()
   MOZ_ASSERT(!mInitialized, "Can't re-initialize a decoder!");
 
   // Implementation-specific initialization
   InitInternal();
 
   mInitialized = true;
 }
 
-// Initializes a decoder whose image and observer is already being used by a
-// parent decoder
-void
-Decoder::InitSharedDecoder(uint8_t* aImageData, uint32_t aImageDataLength,
-                           uint32_t* aColormap, uint32_t aColormapSize,
-                           RawAccessFrameRef&& aFrameRef)
-{
-  // No re-initializing
-  MOZ_ASSERT(!mInitialized, "Can't re-initialize a decoder!");
-
-  mImageData = aImageData;
-  mImageDataLength = aImageDataLength;
-  mColormap = aColormap;
-  mColormapSize = aColormapSize;
-  mCurrentFrame = Move(aFrameRef);
-
-  // We have all the frame data, so we've started the frame.
-  if (!IsSizeDecode()) {
-    mFrameCount++;
-    PostFrameStart();
-  }
-
-  // Implementation-specific initialization
-  InitInternal();
-  mInitialized = true;
-}
-
 nsresult
 Decoder::Decode()
 {
   MOZ_ASSERT(mInitialized, "Should be initialized here");
   MOZ_ASSERT(mIterator, "Should have a SourceBufferIterator");
 
   // We keep decoding chunks until the decode completes or there are no more
   // chunks available.
@@ -174,75 +145,55 @@ Decoder::ShouldSyncDecode(size_t aByteLi
 }
 
 void
 Decoder::Write(const char* aBuffer, uint32_t aCount)
 {
   PROFILER_LABEL("ImageDecoder", "Write",
     js::ProfileEntry::Category::GRAPHICS);
 
+  MOZ_ASSERT(aBuffer);
+  MOZ_ASSERT(aCount > 0);
+
   // We're strict about decoder errors
   MOZ_ASSERT(!HasDecoderError(),
              "Not allowed to make more decoder calls after error!");
 
   // Begin recording telemetry data.
   TimeStamp start = TimeStamp::Now();
   mChunkCount++;
 
   // Keep track of the total number of bytes written.
   mBytesDecoded += aCount;
 
-  // If we're flushing data, clear the flag.
-  if (aBuffer == nullptr && aCount == 0) {
-    MOZ_ASSERT(mNeedsToFlushData, "Flushing when we don't need to");
-    mNeedsToFlushData = false;
-  }
-
   // If a data error occured, just ignore future data.
   if (HasDataError()) {
     return;
   }
 
   if (IsSizeDecode() && HasSize()) {
     // More data came in since we found the size. We have nothing to do here.
     return;
   }
 
-  MOZ_ASSERT(!NeedsNewFrame() || HasDataError(),
-             "Should not need a new frame before writing anything");
-  MOZ_ASSERT(!NeedsToFlushData() || HasDataError(),
-             "Should not need to flush data before writing anything");
-
   // Pass the data along to the implementation.
   WriteInternal(aBuffer, aCount);
 
-  // If we need a new frame to proceed, let's create one and call it again.
-  while (NeedsNewFrame() && !HasDataError()) {
-    MOZ_ASSERT(!IsSizeDecode(), "Shouldn't need new frame for size decode");
-
-    nsresult rv = AllocateFrame();
-
-    if (NS_SUCCEEDED(rv)) {
-      // Use the data we saved when we asked for a new frame.
-      WriteInternal(nullptr, 0);
-    }
-
-    mNeedsToFlushData = false;
-  }
-
   // Finish telemetry.
   mDecodeTime += (TimeStamp::Now() - start);
 }
 
 void
 Decoder::CompleteDecode()
 {
   // Implementation-specific finalization
   if (!HasError()) {
     FinishInternal();
+  } else {
+    FinishWithErrorInternal();
   }
 
   // If the implementation left us mid-frame, finish that up.
   if (mInFrame && !HasError()) {
     PostFrameStop();
   }
 
   // If PostDecodeDone() has not been called, and this decoder wasn't aborted
@@ -323,145 +274,61 @@ Decoder::Finish()
 
     // If this image wasn't animated and isn't a transient image, mark its frame
     // as optimizable. We don't support optimizing animated images and
     // optimizing transient images isn't worth it.
     if (!mIsAnimated && !mImageIsTransient && mCurrentFrame) {
       mCurrentFrame->SetOptimizable();
     }
 
-    mImage->OnDecodingComplete();
-  }
-}
-
-void
-Decoder::FinishSharedDecoder()
-{
-  if (!HasError()) {
-    FinishInternal();
+    mImage->OnDecodingComplete(mIsAnimated);
   }
 }
 
 nsresult
-Decoder::AllocateFrame(const nsIntSize& aTargetSize /* = nsIntSize() */)
+Decoder::AllocateFrame(uint32_t aFrameNum,
+                       const nsIntSize& aTargetSize,
+                       const nsIntRect& aFrameRect,
+                       gfx::SurfaceFormat aFormat,
+                       uint8_t aPaletteDepth)
 {
-  MOZ_ASSERT(mNeedsNewFrame);
-
-  nsIntSize targetSize = aTargetSize;
-  if (targetSize == nsIntSize()) {
-    MOZ_ASSERT(HasSize());
-    targetSize = mImageMetadata.GetSize();
-  }
-
-  mCurrentFrame = EnsureFrame(mNewFrameData.mFrameNum,
-                              targetSize,
-                              mNewFrameData.mFrameRect,
-                              GetDecodeFlags(),
-                              mNewFrameData.mFormat,
-                              mNewFrameData.mPaletteDepth,
-                              mCurrentFrame.get());
+  mCurrentFrame = AllocateFrameInternal(aFrameNum, aTargetSize, aFrameRect,
+                                        GetDecodeFlags(), aFormat,
+                                        aPaletteDepth, mCurrentFrame.get());
 
   if (mCurrentFrame) {
     // Gather the raw pointers the decoders will use.
     mCurrentFrame->GetImageData(&mImageData, &mImageDataLength);
     mCurrentFrame->GetPaletteData(&mColormap, &mColormapSize);
 
-    if (mNewFrameData.mFrameNum + 1 == mFrameCount) {
+    if (aFrameNum + 1 == mFrameCount) {
       PostFrameStart();
     }
   } else {
     PostDataError();
   }
 
-  // Mark ourselves as not needing another frame before talking to anyone else
-  // so they can tell us if they need yet another.
-  mNeedsNewFrame = false;
-
-  // If we've received any data at all, we may have pending data that needs to
-  // be flushed now that we have a frame to decode into.
-  if (mBytesDecoded > 0) {
-    mNeedsToFlushData = true;
-  }
-
   return mCurrentFrame ? NS_OK : NS_ERROR_FAILURE;
 }
 
 RawAccessFrameRef
-Decoder::EnsureFrame(uint32_t aFrameNum,
-                     const nsIntSize& aTargetSize,
-                     const nsIntRect& aFrameRect,
-                     uint32_t aDecodeFlags,
-                     SurfaceFormat aFormat,
-                     uint8_t aPaletteDepth,
-                     imgFrame* aPreviousFrame)
+Decoder::AllocateFrameInternal(uint32_t aFrameNum,
+                               const nsIntSize& aTargetSize,
+                               const nsIntRect& aFrameRect,
+                               uint32_t aDecodeFlags,
+                               SurfaceFormat aFormat,
+                               uint8_t aPaletteDepth,
+                               imgFrame* aPreviousFrame)
 {
   if (mDataError || NS_FAILED(mFailCode)) {
     return RawAccessFrameRef();
   }
 
-  MOZ_ASSERT(aFrameNum <= mFrameCount, "Invalid frame index!");
-  if (aFrameNum > mFrameCount) {
-    return RawAccessFrameRef();
-  }
-
-  // Adding a frame that doesn't already exist. This is the normal case.
-  if (aFrameNum == mFrameCount) {
-    return InternalAddFrame(aFrameNum, aTargetSize, aFrameRect, aDecodeFlags,
-                            aFormat, aPaletteDepth, aPreviousFrame);
-  }
-
-  // We're replacing a frame. It must be the first frame; there's no reason to
-  // ever replace any other frame, since the first frame is the only one we
-  // speculatively allocate without knowing what the decoder really needs.
-  // XXX(seth): I'm not convinced there's any reason to support this at all. We
-  // should figure out how to avoid triggering this and rip it out.
-  MOZ_ASSERT(aFrameNum == 0, "Replacing a frame other than the first?");
-  MOZ_ASSERT(mFrameCount == 1, "Should have only one frame");
-  MOZ_ASSERT(aPreviousFrame, "Need the previous frame to replace");
-  if (aFrameNum != 0 || !aPreviousFrame || mFrameCount != 1) {
-    return RawAccessFrameRef();
-  }
-
-  MOZ_ASSERT(!aPreviousFrame->GetRect().IsEqualEdges(aFrameRect) ||
-             aPreviousFrame->GetFormat() != aFormat ||
-             aPreviousFrame->GetPaletteDepth() != aPaletteDepth,
-             "Replacing first frame with the same kind of frame?");
-
-  // Reset our state.
-  mInFrame = false;
-  RawAccessFrameRef ref = Move(mCurrentFrame);
-
-  MOZ_ASSERT(ref, "No ref to current frame?");
-
-  // Reinitialize the old frame.
-  nsIntSize oldSize = aPreviousFrame->GetImageSize();
-  bool nonPremult =
-    aDecodeFlags & imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA;
-  if (NS_FAILED(aPreviousFrame->ReinitForDecoder(oldSize, aFrameRect, aFormat,
-                                                 aPaletteDepth, nonPremult))) {
-    NS_WARNING("imgFrame::ReinitForDecoder should succeed");
-    mFrameCount = 0;
-    aPreviousFrame->Abort();
-    return RawAccessFrameRef();
-  }
-
-  return ref;
-}
-
-RawAccessFrameRef
-Decoder::InternalAddFrame(uint32_t aFrameNum,
-                          const nsIntSize& aTargetSize,
-                          const nsIntRect& aFrameRect,
-                          uint32_t aDecodeFlags,
-                          SurfaceFormat aFormat,
-                          uint8_t aPaletteDepth,
-                          imgFrame* aPreviousFrame)
-{
-  MOZ_ASSERT(aFrameNum <= mFrameCount, "Invalid frame index!");
-  if (aFrameNum > mFrameCount) {
+  if (aFrameNum != mFrameCount) {
+    MOZ_ASSERT_UNREACHABLE("Allocating frames out of order");
     return RawAccessFrameRef();
   }
 
   if (aTargetSize.width <= 0 || aTargetSize.height <= 0 ||
       aFrameRect.width <= 0 || aFrameRect.height <= 0) {
     NS_WARNING("Trying to add frame with zero or negative size");
     return RawAccessFrameRef();
   }
@@ -554,16 +421,17 @@ Decoder::SetSizeOnImage()
 
 /*
  * Hook stubs. Override these as necessary in decoder implementations.
  */
 
 void Decoder::InitInternal() { }
 void Decoder::WriteInternal(const char* aBuffer, uint32_t aCount) { }
 void Decoder::FinishInternal() { }
+void Decoder::FinishWithErrorInternal() { }
 
 /*
  * Progress Notifications
  */
 
 void
 Decoder::PostSize(int32_t aWidth,
                   int32_t aHeight,
@@ -680,34 +548,16 @@ Decoder::PostDecoderError(nsresult aFail
   // needs to know its URI first
   NS_WARNING("Image decoding error - This is probably a bug!");
 
   if (mInFrame && mCurrentFrame) {
     mCurrentFrame->Abort();
   }
 }
 
-void
-Decoder::NeedNewFrame(uint32_t framenum, uint32_t x_offset, uint32_t y_offset,
-                      uint32_t width, uint32_t height,
-                      gfx::SurfaceFormat format,
-                      uint8_t palette_depth /* = 0 */)
-{
-  // Decoders should never call NeedNewFrame without yielding back to Write().
-  MOZ_ASSERT(!mNeedsNewFrame);
-
-  // We don't want images going back in time or skipping frames.
-  MOZ_ASSERT(framenum == mFrameCount || framenum == (mFrameCount - 1));
-
-  mNewFrameData = NewFrameData(framenum,
-                               nsIntRect(x_offset, y_offset, width, height),
-                               format, palette_depth);
-  mNeedsNewFrame = true;
-}
-
 Telemetry::ID
 Decoder::SpeedHistogram()
 {
   // Use HistogramCount as an invalid Histogram ID.
   return Telemetry::HistogramCount;
 }
 
 } // namespace image
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -29,32 +29,17 @@ public:
   explicit Decoder(RasterImage* aImage);
 
   /**
    * Initialize an image decoder. Decoders may not be re-initialized.
    */
   void Init();
 
   /**
-   * Initializes a decoder whose image and observer is already being used by a
-   * parent decoder. Decoders may not be re-initialized.
-   *
-   * Notifications Sent: TODO
-   */
-  void InitSharedDecoder(uint8_t* aImageData, uint32_t aImageDataLength,
-                         uint32_t* aColormap, uint32_t aColormapSize,
-                         RawAccessFrameRef&& aFrameRef);
-
-  /**
    * Decodes, reading all data currently available in the SourceBuffer. If more
-   * If aBuffer is null and aCount is 0, Write() flushes any buffered data to
-   * the decoder. Data is buffered if the decoder wasn't able to completely
-   * decode it because it needed a new frame.  If it's necessary to flush data,
-   * NeedsToFlushData() will return true.
-   *
    * data is needed, Decode() automatically ensures that it will be called again
    * on a DecodePool thread when the data becomes available.
    *
    * Any errors are reported by setting the appropriate state on the decoder.
    */
   nsresult Decode();
 
   /**
@@ -65,24 +50,16 @@ public:
 
   /**
    * Given a maximum number of bytes we're willing to decode, @aByteLimit,
    * returns true if we should attempt to run this decoder synchronously.
    */
   bool ShouldSyncDecode(size_t aByteLimit);
 
   /**
-   * Informs the shared decoder that all the data has been written.
-   * Should only be used if InitSharedDecoder was useed
-   *
-   * Notifications Sent: TODO
-   */
-  void FinishSharedDecoder();
-
-  /**
    * Gets the invalidation region accumulated by the decoder so far, and clears
    * the decoder's invalidation region. This means that each call to
    * TakeInvalidRect() returns only the invalidation region accumulated since
    * the last call to TakeInvalidRect().
    */
   nsIntRect TakeInvalidRect()
   {
     nsIntRect invalidRect = mInvalidRect;
@@ -254,60 +231,34 @@ public:
 
   void SetFlags(uint32_t aFlags) { mFlags = aFlags; }
   uint32_t GetFlags() const { return mFlags; }
   uint32_t GetDecodeFlags() const { return DecodeFlags(mFlags); }
 
   bool HasSize() const { return mImageMetadata.HasSize(); }
   void SetSizeOnImage();
 
-  void SetSize(const nsIntSize& aSize, const Orientation& aOrientation)
-  {
-    PostSize(aSize.width, aSize.height, aOrientation);
-  }
-
   nsIntSize GetSize() const
   {
     MOZ_ASSERT(HasSize());
     return mImageMetadata.GetSize();
   }
 
   virtual Telemetry::ID SpeedHistogram();
 
   ImageMetadata& GetImageMetadata() { return mImageMetadata; }
 
   /**
    * Returns a weak pointer to the image associated with this decoder.
    */
   RasterImage* GetImage() const { MOZ_ASSERT(mImage); return mImage.get(); }
 
-  // Tell the decoder infrastructure to allocate a frame. By default, frame 0
-  // is created as an ARGB frame with no offset and with size width * height.
-  // If decoders need something different, they must ask for it.
-  // This is called by decoders when they need a new frame. These decoders
-  // must then save the data they have been sent but not yet processed and
-  // return from WriteInternal. When the new frame is created, WriteInternal
-  // will be called again with nullptr and 0 as arguments.
-  void NeedNewFrame(uint32_t frameNum, uint32_t x_offset, uint32_t y_offset,
-                    uint32_t width, uint32_t height,
-                    gfx::SurfaceFormat format,
-                    uint8_t palette_depth = 0);
-
-  virtual bool NeedsNewFrame() const { return mNeedsNewFrame; }
-
-  // Try to allocate a frame as described in mNewFrameData and return the
-  // status code from that attempt. Clears mNewFrameData.
-  virtual nsresult AllocateFrame(const nsIntSize& aTargetSize = nsIntSize());
-
-  already_AddRefed<imgFrame> GetCurrentFrame()
-  {
-    nsRefPtr<imgFrame> frame = mCurrentFrame.get();
-    return frame.forget();
-  }
-
+  // XXX(seth): This should be removed once we can optimize imgFrame objects
+  // off-main-thread. It only exists to support the code in Finish() for
+  // nsICODecoder.
   RawAccessFrameRef GetCurrentFrameRef()
   {
     return mCurrentFrame ? mCurrentFrame->RawAccessRef()
                          : RawAccessFrameRef();
   }
 
   /**
    * Writes data to the decoder. Only public for the benefit of nsICODecoder;
@@ -317,25 +268,28 @@ public:
    * @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;
+
   virtual ~Decoder();
 
   /*
    * Internal hooks. Decoder implementations may override these and
    * only these methods.
    */
   virtual void InitInternal();
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount);
   virtual void FinishInternal();
+  virtual void FinishWithErrorInternal();
 
   /*
    * Progress notifications.
    */
 
   // Called by decoders when they determine the size of the image. Informs
   // the image of its size and sends notifications.
   void PostSize(int32_t aWidth,
@@ -391,55 +345,51 @@ protected:
   // For animated images, specify the loop count. -1 means loop forever, 0
   // 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);
 
-  // Returns true if we may have stored data that we need to flush now that we
-  // have a new frame to decode into. Callers can use Write() to actually
-  // flush the data; see the documentation for that method.
-  bool NeedsToFlushData() const { return mNeedsToFlushData; }
-
   /**
    * 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();
 
   /**
-   * Ensures that a given frame number exists with the given parameters, and
-   * returns a RawAccessFrameRef for that frame.
-   * It is not possible to create sparse frame arrays; you can only append
-   * frames to the current frame array, or if there is only one frame in the
-   * array, replace that frame.
-   * @aTargetSize specifies the target size we're decoding to. If we're not
-   * downscaling during decode, this will always be the same as the image's
-   * intrinsic size.
+   * Allocates a new frame, making it our current frame if successful.
+   *
+   * The @aFrameNum parameter only exists as a sanity check; it's illegal to
+   * create a new frame anywhere but immediately after the existing frames.
    *
    * If a non-paletted frame is desired, pass 0 for aPaletteDepth.
    */
-  RawAccessFrameRef EnsureFrame(uint32_t aFrameNum,
-                                const nsIntSize& aTargetSize,
-                                const nsIntRect& aFrameRect,
-                                uint32_t aDecodeFlags,
-                                gfx::SurfaceFormat aFormat,
-                                uint8_t aPaletteDepth,
-                                imgFrame* aPreviousFrame);
+  nsresult AllocateFrame(uint32_t aFrameNum,
+                         const nsIntSize& aTargetSize,
+                         const nsIntRect& aFrameRect,
+                         gfx::SurfaceFormat aFormat,
+                         uint8_t aPaletteDepth = 0);
 
-  RawAccessFrameRef InternalAddFrame(uint32_t aFrameNum,
-                                     const nsIntSize& aTargetSize,
-                                     const nsIntRect& aFrameRect,
-                                     uint32_t aDecodeFlags,
-                                     gfx::SurfaceFormat aFormat,
-                                     uint8_t aPaletteDepth,
-                                     imgFrame* aPreviousFrame);
+  /// Helper method for decoders which only have 'basic' frame allocation needs.
+  nsresult AllocateBasicFrame() {
+    nsIntSize size = GetSize();
+    return AllocateFrame(0, size, nsIntRect(nsIntPoint(), size),
+                         gfx::SurfaceFormat::B8G8R8A8);
+  }
+
+  RawAccessFrameRef AllocateFrameInternal(uint32_t aFrameNum,
+                                          const nsIntSize& aTargetSize,
+                                          const nsIntRect& aFrameRect,
+                                          uint32_t aDecodeFlags,
+                                          gfx::SurfaceFormat aFormat,
+                                          uint8_t aPaletteDepth,
+                                          imgFrame* aPreviousFrame);
 
   /*
    * Member variables.
    *
    */
   nsRefPtr<RasterImage> mImage;
   Maybe<SourceBufferIterator> mIterator;
   RawAccessFrameRef mCurrentFrame;
@@ -467,37 +417,16 @@ protected:
   bool mImageIsTransient;
   bool mImageIsLocked;
 
 private:
   uint32_t mFrameCount; // Number of frames, including anything in-progress
 
   nsresult mFailCode;
 
-  struct NewFrameData
-  {
-    NewFrameData() { }
-
-    NewFrameData(uint32_t aFrameNum, const nsIntRect& aFrameRect,
-                 gfx::SurfaceFormat aFormat, uint8_t aPaletteDepth)
-      : mFrameNum(aFrameNum)
-      , mFrameRect(aFrameRect)
-      , mFormat(aFormat)
-      , mPaletteDepth(aPaletteDepth)
-    { }
-
-    uint32_t mFrameNum;
-    nsIntRect mFrameRect;
-    gfx::SurfaceFormat mFormat;
-    uint8_t mPaletteDepth;
-  };
-
-  NewFrameData mNewFrameData;
-  bool mNeedsNewFrame;
-  bool mNeedsToFlushData;
   bool mInitialized;
   bool mSizeDecode;
   bool mInFrame;
   bool mIsAnimated;
 };
 
 } // namespace image
 } // namespace mozilla
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -958,19 +958,17 @@ RasterImage::OnAddedFrame(uint32_t aNewF
 {
   if (!NS_IsMainThread()) {
     nsCOMPtr<nsIRunnable> runnable =
       new OnAddedFrameRunnable(this, aNewFrameCount, aNewRefreshArea);
     NS_DispatchToMainThread(runnable);
     return;
   }
 
-  MOZ_ASSERT((mFrameCount == 1 && aNewFrameCount == 1) ||
-             mFrameCount < aNewFrameCount,
-             "Frame count running backwards");
+  MOZ_ASSERT(aNewFrameCount <= mFrameCount + 1, "Skipped a frame?");
 
   if (mError) {
     return;  // We're in an error state, possibly due to OOM. Bail.
   }
 
   if (aNewFrameCount > mFrameCount) {
     mFrameCount = aNewFrameCount;
 
@@ -1029,33 +1027,51 @@ RasterImage::SetSize(int32_t aWidth, int
   mSize.SizeTo(aWidth, aHeight);
   mOrientation = aOrientation;
   mHasSize = true;
 
   return NS_OK;
 }
 
 void
-RasterImage::OnDecodingComplete()
+RasterImage::OnDecodingComplete(bool aIsAnimated)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mError) {
     return;
   }
 
   // Flag that we've been decoded before.
   mHasBeenDecoded = true;
 
-  // Let our FrameAnimator know not to expect any more frames.
-  if (mAnim) {
-    mAnim->SetDoneDecoding(true);
+  if (aIsAnimated) {
+    if (mAnim) {
+      mAnim->SetDoneDecoding(true);
+    } else {
+      // The OnAddedFrame event that will create mAnim is still in the event
+      // queue. Wait for it.
+      nsCOMPtr<nsIRunnable> runnable =
+        NS_NewRunnableMethod(this, &RasterImage::MarkAnimationDecoded);
+      NS_DispatchToMainThread(runnable);
+    }
   }
 }
 
+void
+RasterImage::MarkAnimationDecoded()
+{
+  MOZ_ASSERT(mAnim, "Should have an animation now");
+  if (!mAnim) {
+    return;
+  }
+
+  mAnim->SetDoneDecoding(true);
+}
+
 NS_IMETHODIMP
 RasterImage::SetAnimationMode(uint16_t aAnimationMode)
 {
   if (mAnim) {
     mAnim->SetAnimationMode(aAnimationMode);
   }
   return SetAnimationModeInternal(aAnimationMode);
 }
@@ -1447,28 +1463,16 @@ RasterImage::CreateDecoder(const Maybe<I
   if (!mHasBeenDecoded && aSize) {
     // Lock the image while we're decoding, so that it doesn't get evicted from
     // the SurfaceCache before we have a chance to realize that it's animated.
     // The corresponding unlock happens in FinalizeDecoder.
     LockImage();
     decoder->SetImageIsLocked();
   }
 
-  if (aSize) {
-    // We already have the size; tell the decoder so it can preallocate a
-    // frame.  By default, we create an ARGB frame with no offset. If decoders
-    // need a different type, they need to ask for it themselves.
-    // XXX(seth): Note that we call SetSize() and NeedNewFrame() with the
-    // image's intrinsic size, but AllocateFrame with our target size.
-    decoder->SetSize(mSize, mOrientation);
-    decoder->NeedNewFrame(0, 0, 0, aSize->width, aSize->height,
-                          SurfaceFormat::B8G8R8A8);
-    decoder->AllocateFrame(*aSize);
-  }
-
   if (aSize && decoder->HasError()) {
     if (gfxPrefs::ImageDecodeRetryOnAllocFailure() &&
         mRetryCount < 10) {
       // We couldn't allocate the first frame for this image. We're probably in
       // a temporary low-memory situation, so fire off a runnable and hope that
       // things have improved when it runs. (Unless we've already retried 10
       // times in a row, in which case just give up.)
       mRetryCount++;
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -201,17 +201,20 @@ public:
 
   /**
    * Number of times to loop the image.
    * @note -1 means forever.
    */
   void     SetLoopCount(int32_t aLoopCount);
 
   /// Notification that the entire image has been decoded.
-  void OnDecodingComplete();
+  void OnDecodingComplete(bool aIsAnimated);
+
+  /// Helper method for OnDecodingComplete.
+  void MarkAnimationDecoded();
 
   /**
    * Sends the provided progress notifications to ProgressTracker.
    *
    * Main-thread only.
    *
    * @param aProgress    The progress notifications to send.
    * @param aInvalidRect An invalidation rect to send.
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -432,21 +432,25 @@ nsBMPDecoder::WriteInternal(const char* 
         // + 4 because the line is padded to a 4 bit boundary, but
         // I don't want to make exact calculations here, that's unnecessary.
         // Also, it compensates rounding error.
         if (!mRow) {
           PostDataError();
           return;
         }
       }
-      if (!mImageData) {
-        PostDecoderError(NS_ERROR_FAILURE);
-        return;
+
+      MOZ_ASSERT(!mImageData, "Already have a buffer allocated?");
+      nsresult rv = AllocateBasicFrame();
+      if (NS_FAILED(rv)) {
+          return;
       }
 
+      MOZ_ASSERT(mImageData, "Should have a buffer now");
+
       // Prepare for transparency
       if ((mBIH.compression == BI_RLE8) || (mBIH.compression == BI_RLE4)) {
         // Clear the image, as the RLE may jump over areas
         memset(mImageData, 0, mImageDataLength);
       }
   }
 
   if (mColors && mPos >= mLOH) {
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -157,58 +157,54 @@ nsGIFDecoder2::BeginGIF()
   }
 
   mGIFOpen = true;
 
   PostSize(mGIFStruct.screen_width, mGIFStruct.screen_height);
 }
 
 //******************************************************************************
-void
+nsresult
 nsGIFDecoder2::BeginImageFrame(uint16_t aDepth)
 {
   MOZ_ASSERT(HasSize());
 
   gfx::SurfaceFormat format;
   if (mGIFStruct.is_transparent) {
     format = gfx::SurfaceFormat::B8G8R8A8;
     PostHasTransparency();
   } else {
     format = gfx::SurfaceFormat::B8G8R8X8;
   }
 
+  nsIntRect frameRect(mGIFStruct.x_offset, mGIFStruct.y_offset,
+                      mGIFStruct.width, mGIFStruct.height);
+
   // Use correct format, RGB for first frame, PAL for following frames
   // and include transparency to allow for optimization of opaque images
+  nsresult rv = NS_OK;
   if (mGIFStruct.images_decoded) {
-    // Image data is stored with original depth and palette
-    NeedNewFrame(mGIFStruct.images_decoded, mGIFStruct.x_offset,
-                 mGIFStruct.y_offset, mGIFStruct.width, mGIFStruct.height,
-                 format, aDepth);
+    // Image data is stored with original depth and palette.
+    rv = AllocateFrame(mGIFStruct.images_decoded, GetSize(),
+                       frameRect, format, aDepth);
   } else {
-    nsRefPtr<imgFrame> currentFrame = GetCurrentFrame();
-
-    // Our first full frame is automatically created by the image decoding
-    // infrastructure. Just use it as long as it matches up.
-    if (!currentFrame->GetRect().IsEqualEdges(nsIntRect(mGIFStruct.x_offset,
-                                                        mGIFStruct.y_offset,
-                                                        mGIFStruct.width,
-                                                        mGIFStruct.height))) {
-
+    if (!nsIntRect(nsIntPoint(), GetSize()).IsEqualEdges(frameRect)) {
       // We need padding on the first frame, which means that we don't draw into
       // part of the image at all. Report that as transparency.
       PostHasTransparency();
+    }
 
-      // Regardless of depth of input, image is decoded into 24bit RGB
-      NeedNewFrame(mGIFStruct.images_decoded, mGIFStruct.x_offset,
-                   mGIFStruct.y_offset, mGIFStruct.width, mGIFStruct.height,
-                   format);
-    }
+    // Regardless of depth of input, the first frame is decoded into 24bit RGB.
+    rv = AllocateFrame(mGIFStruct.images_decoded, GetSize(),
+                       frameRect, format);
   }
 
   mCurrentFrameIndex = mGIFStruct.images_decoded;
+
+  return rv;
 }
 
 
 //******************************************************************************
 void
 nsGIFDecoder2::EndImageFrame()
 {
   Opacity opacity = Opacity::SOME_TRANSPARENCY;
@@ -962,37 +958,23 @@ nsGIFDecoder2::WriteInternal(const char*
         depth = (q[8]&0x07) + 1;
       }
       uint32_t realDepth = depth;
       while (mGIFStruct.tpixel >= (1 << realDepth) && (realDepth < 8)) {
         realDepth++;
       }
       // Mask to limit the color values within the colormap
       mColorMask = 0xFF >> (8 - realDepth);
-      BeginImageFrame(realDepth);
 
-      if (NeedsNewFrame()) {
-        // We now need a new frame from the decoder framework. We leave all our
-        // data in the buffer as if it wasn't consumed, copy to our hold and
-        // return to the decoder framework.
-        uint32_t size =
-          len + mGIFStruct.bytes_to_consume + mGIFStruct.bytes_in_hold;
-        if (size) {
-          if (SetHold(q,
-                      mGIFStruct.bytes_to_consume + mGIFStruct.bytes_in_hold,
-                      buf, len)) {
-            // Back into the decoder infrastructure so we can get called again.
-            GETN(9, gif_image_header_continue);
-            return;
-          }
-        }
-        break;
-      } else {
-        // FALL THROUGH
+      if (NS_FAILED(BeginImageFrame(realDepth))) {
+        mGIFStruct.state = gif_error;
+        return;
       }
+
+      // FALL THROUGH
     }
 
     case gif_image_header_continue: {
       // While decoders can reuse frames, we unconditionally increment
       // mGIFStruct.images_decoded when we're done with a frame, so we both can
       // and need to zero out the colormap and image data after every new frame.
       memset(mImageData, 0, mImageDataLength);
       if (mColormap) {
--- a/image/decoders/nsGIFDecoder2.h
+++ b/image/decoders/nsGIFDecoder2.h
@@ -30,17 +30,17 @@ public:
   virtual void FinishInternal() override;
   virtual Telemetry::ID SpeedHistogram() override;
 
 private:
   // These functions will be called when the decoder has a decoded row,
   // frame size information, etc.
 
   void      BeginGIF();
-  void      BeginImageFrame(uint16_t aDepth);
+  nsresult  BeginImageFrame(uint16_t aDepth);
   void      EndImageFrame();
   void      FlushImageData();
   void      FlushImageData(uint32_t fromRow, uint32_t rows);
 
   nsresult  GifWrite(const uint8_t* buf, uint32_t numbytes);
   uint32_t  OutputRow();
   bool      DoLzw(const uint8_t* q);
   bool      SetHold(const uint8_t* buf, uint32_t count,
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -1,9 +1,9 @@
-/* vim:set tw=80 expandtab softtabstop=4 ts=4 sw=4: */
+/* vim:set tw=80 expandtab softtabstop=2 ts=2 sw=2: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* This is a Cross-Platform ICO Decoder, which should work everywhere, including
  * Big-Endian machines like the PowerPC. */
 
 #include <stdlib.h>
@@ -75,23 +75,45 @@ nsICODecoder::~nsICODecoder()
 }
 
 void
 nsICODecoder::FinishInternal()
 {
   // We shouldn't be called in error cases
   MOZ_ASSERT(!HasError(), "Shouldn't call FinishInternal after error!");
 
-  // Finish the internally used decoder as well
-  if (mContainedDecoder) {
-    mContainedDecoder->FinishSharedDecoder();
-    mDecodeDone = mContainedDecoder->GetDecodeDone();
-    mProgress |= mContainedDecoder->TakeProgress();
-    mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect());
+  // Finish the internally used decoder as well.
+  if (mContainedDecoder && !mContainedDecoder->HasError()) {
+    mContainedDecoder->FinishInternal();
   }
+
+  GetFinalStateFromContainedDecoder();
+}
+
+void
+nsICODecoder::FinishWithErrorInternal()
+{
+  GetFinalStateFromContainedDecoder();
+}
+
+void
+nsICODecoder::GetFinalStateFromContainedDecoder()
+{
+  if (!mContainedDecoder) {
+    return;
+  }
+
+  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();
 }
 
 // Returns a buffer filled with the bitmap file header in little endian:
 // Signature 2 bytes 'BM'
 // FileSize      4 bytes File size in bytes
 // reserved      4 bytes unused (=0)
 // DataOffset    4 bytes File offset to Raster Data
 // Returns true if successful
@@ -212,23 +234,18 @@ nsICODecoder::SetHotSpotIfCursor()
 
   mImageMetadata.SetHotspot(mDirEntry.mXHotspot, mDirEntry.mYHotspot);
 }
 
 void
 nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call WriteInternal after error!");
-
-  if (!aCount) {
-    if (mContainedDecoder) {
-      WriteToContainedDecoder(aBuffer, aCount);
-    }
-    return;
-  }
+  MOZ_ASSERT(aBuffer);
+  MOZ_ASSERT(aCount > 0);
 
   while (aCount && (mPos < ICONCOUNTOFFSET)) { // Skip to the # of icons.
     if (mPos == 2) { // if the third byte is 1: This is an icon, 2: a cursor
       if ((*aBuffer != 1) && (*aBuffer != 2)) {
         PostDataError();
         return;
       }
       mIsCursor = (*aBuffer == 2);
@@ -338,19 +355,17 @@ nsICODecoder::WriteInternal(const char* 
     aBuffer += toCopy;
 
     mIsPNG = !memcmp(mSignature, nsPNGDecoder::pngSignatureBytes,
                      PNGSIGNATURESIZE);
     if (mIsPNG) {
       mContainedDecoder = new nsPNGDecoder(mImage);
       mContainedDecoder->SetSizeDecode(IsSizeDecode());
       mContainedDecoder->SetSendPartialInvalidations(mSendPartialInvalidations);
-      mContainedDecoder->InitSharedDecoder(mImageData, mImageDataLength,
-                                           mColormap, mColormapSize,
-                                           Move(mRefForContainedDecoder));
+      mContainedDecoder->Init();
       if (!WriteToContainedDecoder(mSignature, PNGSIGNATURESIZE)) {
         return;
       }
     }
   }
 
   // If we have a PNG, let the PNG decoder do all of the rest of the work
   if (mIsPNG && mContainedDecoder && mPos >= mImageOffset + PNGSIGNATURESIZE) {
@@ -417,19 +432,17 @@ nsICODecoder::WriteInternal(const char* 
     // Init the bitmap decoder which will do most of the work for us
     // It will do everything except the AND mask which isn't present in bitmaps
     // bmpDecoder is for local scope ease, it will be freed by mContainedDecoder
     nsBMPDecoder* bmpDecoder = new nsBMPDecoder(mImage);
     mContainedDecoder = bmpDecoder;
     bmpDecoder->SetUseAlphaData(true);
     mContainedDecoder->SetSizeDecode(IsSizeDecode());
     mContainedDecoder->SetSendPartialInvalidations(mSendPartialInvalidations);
-    mContainedDecoder->InitSharedDecoder(mImageData, mImageDataLength,
-                                         mColormap, mColormapSize,
-                                         Move(mRefForContainedDecoder));
+    mContainedDecoder->Init();
 
     // The ICO format when containing a BMP does not include the 14 byte
     // bitmap file header. To use the code of the BMP decoder we need to
     // generate this header ourselves and feed it to the BMP decoder.
     int8_t bfhBuffer[BMPFILEHEADERSIZE];
     if (!FillBitmapFileHeaderBuffer(bfhBuffer)) {
       PostDataError();
       return;
@@ -622,40 +635,10 @@ nsICODecoder::ProcessDirEntry(IconDirEnt
   aTarget.mBitCount = LittleEndian::readUint16(&aTarget.mBitCount);
   memcpy(&aTarget.mBytesInRes, mDirEntryArray + 8, sizeof(aTarget.mBytesInRes));
   aTarget.mBytesInRes = LittleEndian::readUint32(&aTarget.mBytesInRes);
   memcpy(&aTarget.mImageOffset, mDirEntryArray + 12,
          sizeof(aTarget.mImageOffset));
   aTarget.mImageOffset = LittleEndian::readUint32(&aTarget.mImageOffset);
 }
 
-bool
-nsICODecoder::NeedsNewFrame() const
-{
-  if (mContainedDecoder) {
-    return mContainedDecoder->NeedsNewFrame();
-  }
-
-  return Decoder::NeedsNewFrame();
-}
-
-nsresult
-nsICODecoder::AllocateFrame(const nsIntSize& aTargetSize /* = nsIntSize() */)
-{
-  nsresult rv;
-
-  if (mContainedDecoder) {
-    rv = mContainedDecoder->AllocateFrame(aTargetSize);
-    mCurrentFrame = mContainedDecoder->GetCurrentFrameRef();
-    mProgress |= mContainedDecoder->TakeProgress();
-    mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect());
-    return rv;
-  }
-
-  // Grab a strong ref that we'll later hand over to the contained decoder. This
-  // lets us avoid creating a RawAccessFrameRef off-main-thread.
-  rv = Decoder::AllocateFrame(aTargetSize);
-  mRefForContainedDecoder = GetCurrentFrameRef();
-  return rv;
-}
-
 } // namespace image
 } // namespace mozilla
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -35,27 +35,26 @@ public:
   // Obtains the height of the icon directory entry
   uint32_t GetRealHeight() const
   {
     return mDirEntry.mHeight == 0 ? 256 : mDirEntry.mHeight;
   }
 
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual void FinishInternal() override;
-  virtual nsresult AllocateFrame(const nsIntSize& aTargetSize
-                                   /* = nsIntSize() */) override;
-
-protected:
-  virtual bool NeedsNewFrame() const override;
+  virtual void FinishWithErrorInternal() override;
 
 private:
   // 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);
 
+  // Gets decoder state from the contained decoder so it's visible externally.
+  void GetFinalStateFromContainedDecoder();
+
   // Processes a single dir entry of the icon resource
   void ProcessDirEntry(IconDirEntry& aTarget);
   // Sets the hotspot property of if we have a cursor
   void SetHotSpotIfCursor();
   // Creates a bitmap file header buffer, returns true if successful
   bool FillBitmapFileHeaderBuffer(int8_t* bfh);
   // Fixes the ICO height to match that of the BIH.
   // and also fixes the BIH height to be /2 of what it was.
@@ -79,17 +78,16 @@ private:
   uint16_t mNumIcons; // Stores the number of icons in the ICO file
   uint16_t mCurrIcon; // Stores the current dir entry index we are processing
   uint32_t mImageOffset; // Stores the offset of the image data we want
   uint8_t* mRow;      // Holds one raw line of the image
   int32_t mCurLine;   // Line index of the image that's currently being decoded
   uint32_t mRowBytes; // How many bytes of the row were already received
   int32_t mOldLine;   // Previous index of the line
   nsRefPtr<Decoder> mContainedDecoder; // Contains either a BMP or PNG resource
-  RawAccessFrameRef mRefForContainedDecoder; // Avoid locking off-main-thread
 
   char mDirEntryArray[ICODIRENTRYSIZE]; // Holds the current dir entry buffer
   IconDirEntry mDirEntry; // Holds a decoded dir entry
   // Holds the potential bytes that can be a PNG signature
   char mSignature[PNGSIGNATURESIZE];
   // Holds the potential bytes for a bitmap information header
   char mBIHraw[40];
   // Stores whether or not the icon file we are processing has type 1 (icon)
--- a/image/decoders/nsIconDecoder.cpp
+++ b/image/decoders/nsIconDecoder.cpp
@@ -68,21 +68,27 @@ nsIconDecoder::WriteInternal(const char*
         }
 
         // If We're doing a size decode, we're done
         if (IsSizeDecode()) {
           mState = iconStateFinished;
           break;
         }
 
-        if (!mImageData) {
-          PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
-          return;
+        {
+          MOZ_ASSERT(!mImageData, "Already have a buffer allocated?");
+          nsresult rv = AllocateBasicFrame();
+          if (NS_FAILED(rv)) {
+            mState = iconStateFinished;
+            return;
+          }
         }
 
+        MOZ_ASSERT(mImageData, "Should have a buffer now");
+
         // Book Keeping
         aBuffer++;
         aCount--;
         mState = iconStateReadPixels;
         break;
 
       case iconStateReadPixels: {
 
--- a/image/decoders/nsJPEGDecoder.cpp
+++ b/image/decoders/nsJPEGDecoder.cpp
@@ -391,24 +391,30 @@ nsJPEGDecoder::WriteInternal(const char*
       }
     }
 
     // Don't allocate a giant and superfluous memory buffer
     // when not doing a progressive decode.
     mInfo.buffered_image = mDecodeStyle == PROGRESSIVE &&
                            jpeg_has_multiple_scans(&mInfo);
 
-    if (!mImageData) {
+    MOZ_ASSERT(!mImageData, "Already have a buffer allocated?");
+    nsIntSize targetSize = mDownscaler ? mDownscaler->TargetSize() : GetSize();
+    nsresult rv = AllocateFrame(0, targetSize,
+                                nsIntRect(nsIntPoint(), targetSize),
+                                gfx::SurfaceFormat::B8G8R8A8);
+    if (NS_FAILED(rv)) {
       mState = JPEG_ERROR;
-      PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
       MOZ_LOG(GetJPEGDecoderAccountingLog(), LogLevel::Debug,
              ("} (could not initialize image frame)"));
       return;
     }
 
+    MOZ_ASSERT(mImageData, "Should have a buffer now");
+
     if (mDownscaler) {
       nsresult rv = mDownscaler->BeginFrame(GetSize(),
                                             mImageData,
                                             /* aHasAlpha = */ false);
       if (NS_FAILED(rv)) {
         mState = JPEG_ERROR;
         return;
       }
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -136,61 +136,68 @@ nsPNGDecoder::~nsPNGDecoder()
     // mTransform belongs to us only if mInProfile is non-null
     if (mTransform) {
       qcms_transform_release(mTransform);
     }
   }
 }
 
 // CreateFrame() is used for both simple and animated images
-void nsPNGDecoder::CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset,
-                               int32_t width, int32_t height,
-                               gfx::SurfaceFormat format)
+nsresult
+nsPNGDecoder::CreateFrame(png_uint_32 aXOffset, png_uint_32 aYOffset,
+                          int32_t aWidth, int32_t aHeight,
+                          gfx::SurfaceFormat aFormat)
 {
   MOZ_ASSERT(HasSize());
 
-  if (format == gfx::SurfaceFormat::B8G8R8A8) {
+  if (aFormat == gfx::SurfaceFormat::B8G8R8A8) {
     PostHasTransparency();
   }
 
-  // Our first full frame is automatically created by the image decoding
-  // infrastructure. Just use it as long as it matches up.
-  nsIntRect neededRect(x_offset, y_offset, width, height);
-  nsRefPtr<imgFrame> currentFrame = GetCurrentFrame();
-  if (!currentFrame->GetRect().IsEqualEdges(neededRect)) {
-    if (mNumFrames == 0) {
-      // We need padding on the first frame, which means that we don't draw into
-      // part of the image at all. Report that as transparency.
-      PostHasTransparency();
-    }
-
-    NeedNewFrame(mNumFrames, x_offset, y_offset, width, height, format);
-  } else if (mNumFrames != 0) {
-    NeedNewFrame(mNumFrames, x_offset, y_offset, width, height, format);
+  nsIntRect frameRect(aXOffset, aYOffset, aWidth, aHeight);
+  if (mNumFrames == 0 &&
+      !nsIntRect(nsIntPoint(), GetSize()).IsEqualEdges(frameRect)) {
+    // We need padding on the first frame, which means that we don't draw into
+    // part of the image at all. Report that as transparency.
+    PostHasTransparency();
   }
 
-  mFrameRect = neededRect;
+  // XXX(seth): Some tests depend on the first frame of PNGs being B8G8R8A8.
+  // This is something we should fix.
+  gfx::SurfaceFormat format = aFormat;
+  if (mNumFrames == 0) {
+    format = gfx::SurfaceFormat::B8G8R8A8;
+  }
+
+  nsresult rv = AllocateFrame(mNumFrames, GetSize(), frameRect, format);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  mFrameRect = frameRect;
 
   MOZ_LOG(GetPNGDecoderAccountingLog(), LogLevel::Debug,
          ("PNGDecoderAccounting: nsPNGDecoder::CreateFrame -- created "
           "image frame with %dx%d pixels in container %p",
-          width, height,
+          aWidth, aHeight,
           &mImage));
 
 #ifdef PNG_APNG_SUPPORTED
   if (png_get_valid(mPNG, mInfo, PNG_INFO_acTL)) {
     mAnimInfo = AnimFrameInfo(mPNG, mInfo);
 
     if (mAnimInfo.mDispose == DisposalMethod::CLEAR) {
       // We may have to display the background under this image during
       // animation playback, so we regard it as transparent.
       PostHasTransparency();
     }
   }
 #endif
+
+  return NS_OK;
 }
 
 // set timeout and frame disposal method for the current frame
 void
 nsPNGDecoder::EndImageFrame()
 {
   if (mFrameIsHidden) {
     return;
@@ -646,17 +653,21 @@ nsPNGDecoder::info_callback(png_structp 
     png_set_progressive_frame_fn(png_ptr, nsPNGDecoder::frame_info_callback,
                                  nullptr);
   }
 
   if (png_get_first_frame_is_hidden(png_ptr, info_ptr)) {
     decoder->mFrameIsHidden = true;
   } else {
 #endif
-    decoder->CreateFrame(0, 0, width, height, decoder->format);
+    nsresult rv = decoder->CreateFrame(0, 0, width, height, decoder->format);
+    if (NS_FAILED(rv)) {
+      png_longjmp(decoder->mPNG, 5); // NS_ERROR_OUT_OF_MEMORY
+    }
+    MOZ_ASSERT(decoder->mImageData, "Should have a buffer now");
 #ifdef PNG_APNG_SUPPORTED
   }
 #endif
 
   if (decoder->mTransform &&
       (channels <= 2 || interlace_type == PNG_INTERLACE_ADAM7)) {
     uint32_t bpp[] = { 0, 3, 4, 3, 4 };
     decoder->mCMSLine =
@@ -669,22 +680,16 @@ nsPNGDecoder::info_callback(png_structp 
   if (interlace_type == PNG_INTERLACE_ADAM7) {
     if (height < INT32_MAX / (width * channels)) {
       decoder->interlacebuf = (uint8_t*)malloc(channels * width * height);
     }
     if (!decoder->interlacebuf) {
       png_longjmp(decoder->mPNG, 5); // NS_ERROR_OUT_OF_MEMORY
     }
   }
-
-  if (decoder->NeedsNewFrame()) {
-    // We know that we need a new frame, so pause input so the decoder
-    // infrastructure can give it to us.
-    png_process_data_pause(png_ptr, /* save = */ 1);
-  }
 }
 
 void
 nsPNGDecoder::row_callback(png_structp png_ptr, png_bytep new_row,
                            png_uint_32 row_num, int pass)
 {
   /* libpng comments:
    *
@@ -826,23 +831,22 @@ nsPNGDecoder::frame_info_callback(png_st
   // Only the first frame can be hidden, so unhide unconditionally here.
   decoder->mFrameIsHidden = false;
 
   x_offset = png_get_next_frame_x_offset(png_ptr, decoder->mInfo);
   y_offset = png_get_next_frame_y_offset(png_ptr, decoder->mInfo);
   width = png_get_next_frame_width(png_ptr, decoder->mInfo);
   height = png_get_next_frame_height(png_ptr, decoder->mInfo);
 
-  decoder->CreateFrame(x_offset, y_offset, width, height, decoder->format);
-
-  if (decoder->NeedsNewFrame()) {
-    // We know that we need a new frame, so pause input so the decoder
-    // infrastructure can give it to us.
-    png_process_data_pause(png_ptr, /* save = */ 1);
+  nsresult rv =
+    decoder->CreateFrame(x_offset, y_offset, width, height, decoder->format);
+  if (NS_FAILED(rv)) {
+    png_longjmp(decoder->mPNG, 5); // NS_ERROR_OUT_OF_MEMORY
   }
+  MOZ_ASSERT(decoder->mImageData, "Should have a buffer now");
 }
 #endif
 
 void
 nsPNGDecoder::end_callback(png_structp png_ptr, png_infop info_ptr)
 {
   /* libpng comments:
    *
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -26,19 +26,19 @@ class nsPNGDecoder : public Decoder
 public:
   explicit nsPNGDecoder(RasterImage* aImage);
   virtual ~nsPNGDecoder();
 
   virtual void InitInternal() override;
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual Telemetry::ID SpeedHistogram() override;
 
-  void CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset,
-                   int32_t width, int32_t height,
-                   gfx::SurfaceFormat format);
+  nsresult CreateFrame(png_uint_32 aXOffset, png_uint_32 aYOffset,
+                       int32_t aWidth, int32_t aHeight,
+                       gfx::SurfaceFormat aFormat);
   void EndImageFrame();
 
   // Check if PNG is valid ICO (32bpp RGBA)
   // http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
   bool IsValidICO() const
   {
     // If there are errors in the call to png_get_IHDR, the error_callback in
     // nsPNGDecoder.cpp is called.  In this error callback we do a longjmp, so
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -163,90 +163,16 @@ imgFrame::~imgFrame()
   MOZ_ASSERT(mAborted || IsImageCompleteInternal());
 #endif
 
   free(mPalettedImageData);
   mPalettedImageData = nullptr;
 }
 
 nsresult
-imgFrame::ReinitForDecoder(const nsIntSize& aImageSize,
-                           const nsIntRect& aRect,
-                           SurfaceFormat aFormat,
-                           uint8_t aPaletteDepth /* = 0 */,
-                           bool aNonPremult /* = false */)
-{
-  MonitorAutoLock lock(mMonitor);
-
-  if (mDecoded.x != 0 || mDecoded.y != 0 ||
-      mDecoded.width != 0 || mDecoded.height != 0) {
-    MOZ_ASSERT_UNREACHABLE("Shouldn't reinit after write");
-    return NS_ERROR_FAILURE;
-  }
-  if (mAborted) {
-    MOZ_ASSERT_UNREACHABLE("Shouldn't reinit if aborted");
-    return NS_ERROR_FAILURE;
-  }
-  if (mLockCount < 1) {
-    MOZ_ASSERT_UNREACHABLE("Shouldn't reinit unless locked");
-    return NS_ERROR_FAILURE;
-  }
-
-  // Restore everything (except mLockCount, which we need to keep) to how it was
-  // when we were first created.
-  // XXX(seth): This is probably a little excessive, but I want to be *really*
-  // sure that nothing got missed.
-  mDecoded = nsIntRect(0, 0, 0, 0);
-  mTimeout = 100;
-  mDisposalMethod = DisposalMethod::NOT_SPECIFIED;
-  mBlendMethod = BlendMethod::OVER;
-  mHasNoAlpha = false;
-  mAborted = false;
-  mPaletteDepth = 0;
-  mNonPremult = false;
-  mSinglePixel = false;
-  mCompositingFailed = false;
-  mOptimizable = false;
-  mImageSize = IntSize();
-  mSize = IntSize();
-  mOffset = nsIntPoint();
-  mSinglePixelColor = Color();
-
-  // Release all surfaces.
-  mImageSurface = nullptr;
-  mOptSurface = nullptr;
-  mVBuf = nullptr;
-  mVBufPtr = nullptr;
-  free(mPalettedImageData);
-  mPalettedImageData = nullptr;
-
-  // Reinitialize.
-  nsresult rv = InitForDecoder(aImageSize, aRect, aFormat,
-                               aPaletteDepth, aNonPremult);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  // We were locked before; perform the same actions we would've performed when
-  // we originally got locked.
-  if (mImageSurface) {
-    mVBufPtr = mVBuf;
-    return NS_OK;
-  }
-
-  if (!mPalettedImageData) {
-    MOZ_ASSERT_UNREACHABLE("We got optimized somehow during reinit");
-    return NS_ERROR_FAILURE;
-  }
-
-  // Paletted images don't have surfaces, so there's nothing to do.
-  return NS_OK;
-}
-
-nsresult
 imgFrame::InitForDecoder(const nsIntSize& aImageSize,
                          const nsIntRect& aRect,
                          SurfaceFormat aFormat,
                          uint8_t aPaletteDepth /* = 0 */,
                          bool aNonPremult /* = false */)
 {
   // Assert for properties that should be verified by decoders,
   // warn for properties related to bad content.
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -152,29 +152,16 @@ public:
    * more expensive than in the InitForDecoder() case.
    */
   nsresult InitWithDrawable(gfxDrawable* aDrawable,
                             const nsIntSize& aSize,
                             const SurfaceFormat aFormat,
                             GraphicsFilter aFilter,
                             uint32_t aImageFlags);
 
-  /**
-   * Reinitializes an existing imgFrame with new parameters. You must be holding
-   * a RawAccessFrameRef to the imgFrame, and it must never have been written
-   * to, marked finished, or aborted.
-   *
-   * XXX(seth): We will remove this in bug 1117607.
-   */
-  nsresult ReinitForDecoder(const nsIntSize& aImageSize,
-                            const nsIntRect& aRect,
-                            SurfaceFormat aFormat,
-                            uint8_t aPaletteDepth = 0,
-                            bool aNonPremult = false);
-
   DrawableFrameRef DrawableRef();
   RawAccessFrameRef RawAccessRef();
 
   /**
    * Make this imgFrame permanently available for raw access.
    *
    * This is irrevocable, and should be avoided whenever possible, since it
    * prevents this imgFrame from being optimized and makes it impossible for its