Bug 1117607 - Make decoders responsible for their own frame allocations. r=tn
☠☠ backed out by d4be320ebecb ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Fri, 09 Jan 2015 15:02:48 -0800
changeset 248912 17fc30214d848a64c87e87b043566d3499dd1c03
parent 248911 eb4ffa05e89c100b85459f149264b3c2cd9b0461
child 248913 bceadf54fee87aebcd295cd7bf1320d5976e0b27
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1117607
milestone37.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/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/src/Decoder.cpp
image/src/Decoder.h
image/src/ImageMetadata.h
image/src/RasterImage.cpp
testing/web-platform/meta/2dcontext/drawing-images-to-the-canvas/2d.drawImage.broken.html.ini
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -434,21 +434,26 @@ 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);
+
+      MOZ_ASSERT(!mImageData, "Already have a buffer allocated?");
+      nsresult rv = AllocateFrame(0, nsIntRect(nsIntPoint(), GetSize()),
+                                  gfx::SurfaceFormat::B8G8R8A8);
+      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
@@ -156,58 +156,52 @@ 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, 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, frameRect, format);
   }
 
   mCurrentFrameIndex = mGIFStruct.images_decoded;
+
+  return rv;
 }
 
 
 //******************************************************************************
 void
 nsGIFDecoder2::EndImageFrame()
 {
   Opacity opacity = Opacity::SOME_TRANSPARENCY;
@@ -952,37 +946,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() MOZ_OVERRIDE;
   virtual Telemetry::ID SpeedHistogram() MOZ_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
@@ -77,20 +77,23 @@ nsICODecoder::~nsICODecoder()
 void
 nsICODecoder::FinishInternal()
 {
   // We shouldn't be called in error cases
   NS_ABORT_IF_FALSE(!HasError(), "Shouldn't call FinishInternal after error!");
 
   // Finish the internally used decoder as well
   if (mContainedDecoder) {
-    mContainedDecoder->FinishSharedDecoder();
+    if (!mContainedDecoder->HasError()) {
+      mContainedDecoder->FinishInternal();
+    }
     mDecodeDone = mContainedDecoder->GetDecodeDone();
     mProgress |= mContainedDecoder->TakeProgress();
     mInvalidRect.Union(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
@@ -337,19 +340,17 @@ nsICODecoder::WriteInternal(const char* 
     aCount -= toCopy;
     aBuffer += toCopy;
 
     mIsPNG = !memcmp(mSignature, nsPNGDecoder::pngSignatureBytes,
                      PNGSIGNATURESIZE);
     if (mIsPNG) {
       mContainedDecoder = new nsPNGDecoder(mImage);
       mContainedDecoder->SetSizeDecode(IsSizeDecode());
-      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) {
@@ -415,19 +416,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->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;
@@ -620,40 +619,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()
-{
-  nsresult rv;
-
-  if (mContainedDecoder) {
-    rv = mContainedDecoder->AllocateFrame();
-    mCurrentFrame = mContainedDecoder->GetCurrentFrameRef();
-    mProgress |= mContainedDecoder->TakeProgress();
-    mInvalidRect.Union(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();
-  mRefForContainedDecoder = GetCurrentFrameRef();
-  return rv;
-}
-
 } // namespace image
 } // namespace mozilla
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -35,20 +35,16 @@ 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) MOZ_OVERRIDE;
   virtual void FinishInternal() MOZ_OVERRIDE;
-  virtual nsresult AllocateFrame() MOZ_OVERRIDE;
-
-protected:
-  virtual bool NeedsNewFrame() const MOZ_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);
 
   // Processes a single dir entry of the icon resource
   void ProcessDirEntry(IconDirEntry& aTarget);
@@ -78,17 +74,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,28 @@ 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 = AllocateFrame(0, nsIntRect(nsIntPoint(), GetSize()),
+                                      gfx::SurfaceFormat::B8G8R8A8);
+          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
@@ -381,24 +381,28 @@ 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?");
+    nsresult rv = AllocateFrame(0, nsIntRect(nsIntPoint(), GetSize()),
+                                gfx::SurfaceFormat::B8G8R8A8);
+    if (NS_FAILED(rv)) {
       mState = JPEG_ERROR;
-      PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
       PR_LOG(GetJPEGDecoderAccountingLog(), PR_LOG_DEBUG,
              ("} (could not initialize image frame)"));
       return;
     }
 
+    MOZ_ASSERT(mImageData, "Should have a buffer now");
+
     PR_LOG(GetJPEGDecoderAccountingLog(), PR_LOG_DEBUG,
            ("        JPEGDecoderAccounting: nsJPEGDecoder::"
             "Write -- created image frame with %ux%u pixels",
             mInfo.output_width, mInfo.output_height));
 
     mState = JPEG_START_DECOMPRESS;
   }
 
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -136,62 +136,62 @@ 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;
+  nsresult rv = AllocateFrame(mNumFrames, frameRect, format);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  mFrameRect = frameRect;
   mFrameHasNoAlpha = true;
 
   PR_LOG(GetPNGDecoderAccountingLog(), PR_LOG_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;
@@ -644,17 +644,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 =
@@ -667,22 +671,16 @@ nsPNGDecoder::info_callback(png_structp 
   if (interlace_type == PNG_INTERLACE_ADAM7) {
     if (height < INT32_MAX / (width * channels)) {
       decoder->interlacebuf = (uint8_t*)moz_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:
    *
@@ -835,23 +833,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() MOZ_OVERRIDE;
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) MOZ_OVERRIDE;
   virtual Telemetry::ID SpeedHistogram() MOZ_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/src/Decoder.cpp
+++ b/image/src/Decoder.cpp
@@ -27,18 +27,16 @@ Decoder::Decoder(RasterImage &aImage)
   , mColormap(nullptr)
   , mChunkCount(0)
   , mDecodeFlags(0)
   , mBytesDecoded(0)
   , mDecodeDone(false)
   , mDataError(false)
   , mFrameCount(0)
   , mFailCode(NS_OK)
-  , mNeedsNewFrame(false)
-  , mNeedsToFlushData(false)
   , mInitialized(false)
   , mSizeDecode(false)
   , mInFrame(false)
   , mIsAnimated(false)
 { }
 
 Decoder::~Decoder()
 {
@@ -65,97 +63,48 @@ Decoder::Init()
   }
 
   // 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
-  NS_ABORT_IF_FALSE(!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;
-}
-
 void
 Decoder::Write(const char* aBuffer, uint32_t aCount)
 {
   PROFILER_LABEL("ImageDecoder", "Write",
     js::ProfileEntry::Category::GRAPHICS);
 
+  MOZ_ASSERT(aBuffer, "Should have a buffer");
+  MOZ_ASSERT(aCount > 0, "Should have a non-zero count");
+
   // 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::Finish(ShutdownReason aReason)
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -220,125 +169,55 @@ Decoder::Finish(ShutdownReason aReason)
   mImageMetadata.SetOnImage(&mImage);
 
   if (mDecodeDone) {
     MOZ_ASSERT(HasError() || mCurrentFrame, "Should have an error or a frame");
     mImage.DecodingComplete(mCurrentFrame.get());
   }
 }
 
-void
-Decoder::FinishSharedDecoder()
+nsresult
+Decoder::AllocateFrame(uint32_t aFrameNum,
+                       const nsIntRect& aFrameRect,
+                       gfx::SurfaceFormat aFormat,
+                       uint8_t aPaletteDepth /* = 0 */)
 {
-  MOZ_ASSERT(NS_IsMainThread());
-
-  if (!HasError()) {
-    FinishInternal();
-  }
-}
-
-nsresult
-Decoder::AllocateFrame()
-{
-  MOZ_ASSERT(mNeedsNewFrame);
-
-  mCurrentFrame = EnsureFrame(mNewFrameData.mFrameNum,
-                              mNewFrameData.mFrameRect,
-                              mDecodeFlags,
-                              mNewFrameData.mFormat,
-                              mNewFrameData.mPaletteDepth,
-                              mCurrentFrame.get());
+  mCurrentFrame = AllocateFrameInternal(aFrameNum, aFrameRect, mDecodeFlags,
+                                        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 nsIntRect& aFrameRect,
-                     uint32_t aDecodeFlags,
-                     SurfaceFormat aFormat,
-                     uint8_t aPaletteDepth,
-                     imgFrame* aPreviousFrame)
+Decoder::AllocateFrameInternal(uint32_t aFrameNum,
+                               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, 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?");
-
-  // Remove the old frame from the SurfaceCache.
-  IntSize prevFrameSize = aPreviousFrame->GetImageSize();
-  SurfaceCache::RemoveSurface(ImageKey(&mImage),
-                              RasterSurfaceKey(prevFrameSize, aDecodeFlags, 0));
-  mFrameCount = 0;
-  mInFrame = false;
-
-  // Add the new frame as usual.
-  return InternalAddFrame(aFrameNum, aFrameRect, aDecodeFlags, aFormat,
-                          aPaletteDepth, nullptr);
-}
-
-RawAccessFrameRef
-Decoder::InternalAddFrame(uint32_t aFrameNum,
-                          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();
   }
 
   MOZ_ASSERT(mImageMetadata.HasSize());
   nsIntSize imageSize(mImageMetadata.GetWidth(), mImageMetadata.GetHeight());
   if (imageSize.width <= 0 || imageSize.height <= 0 ||
       aFrameRect.width <= 0 || aFrameRect.height <= 0) {
     NS_WARNING("Trying to add frame with zero or negative size");
@@ -523,28 +402,10 @@ Decoder::PostDecoderError(nsresult aFail
 
   mFailCode = aFailureCode;
 
   // XXXbholley - we should report the image URI here, but imgContainer
   // needs to know its URI first
   NS_WARNING("Image decoding error - This is probably a bug!");
 }
 
-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;
-}
-
 } // namespace image
 } // namespace mozilla
--- a/image/src/Decoder.h
+++ b/image/src/Decoder.h
@@ -27,33 +27,18 @@ public:
   /**
    * Initialize an image decoder. Decoders may not be re-initialized.
    *
    * Notifications Sent: TODO
    */
   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);
-
-  /**
    * Writes data to the decoder.
    *
-   * 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.
-   *
    * @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.
    *
    * Notifications Sent: TODO
    */
   void Write(const char* aBuffer, uint32_t aCount);
@@ -61,24 +46,16 @@ public:
   /**
    * Informs the decoder that all the data has been written.
    *
    * Notifications Sent: TODO
    */
   void Finish(ShutdownReason aReason);
 
   /**
-   * 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;
@@ -151,60 +128,40 @@ public:
   enum DecodeStyle {
       PROGRESSIVE, // produce intermediate frames representing the partial state of the image
       SEQUENTIAL // decode to final image immediately
   };
 
   void SetDecodeFlags(uint32_t aFlags) { mDecodeFlags = aFlags; }
   uint32_t GetDecodeFlags() { return mDecodeFlags; }
 
+  nsIntSize GetSize() const { return mImageMetadata.GetSize(); }
   bool HasSize() const { return mImageMetadata.HasSize(); }
   void SetSizeOnImage();
 
-  void SetSize(const nsIntSize& aSize, const Orientation& aOrientation)
-  {
-    PostSize(aSize.width, aSize.height, aOrientation);
-  }
-
   // Use HistogramCount as an invalid Histogram ID
   virtual Telemetry::ID SpeedHistogram() { return Telemetry::HistogramCount; }
 
   ImageMetadata& GetImageMetadata() { return mImageMetadata; }
 
-  // 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();
-
   already_AddRefed<imgFrame> GetCurrentFrame()
   {
     nsRefPtr<imgFrame> frame = mCurrentFrame.get();
     return frame.forget();
   }
 
   RawAccessFrameRef GetCurrentFrameRef()
   {
     return mCurrentFrame ? mCurrentFrame->RawAccessRef()
                          : RawAccessFrameRef();
   }
 
 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);
@@ -259,42 +216,36 @@ 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; }
-
   /**
-   * 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.
+   * Allocates a new frame, making it our new 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 nsIntRect& aFrameRect,
-                                uint32_t aDecodeFlags,
-                                gfx::SurfaceFormat aFormat,
-                                uint8_t aPaletteDepth,
-                                imgFrame* aPreviousFrame);
+  nsresult AllocateFrame(uint32_t aFrameNum,
+                         const nsIntRect& aFrameRect,
+                         gfx::SurfaceFormat aFormat,
+                         uint8_t aPaletteDepth = 0);
 
-  RawAccessFrameRef InternalAddFrame(uint32_t aFrameNum,
-                                     const nsIntRect& aFrameRect,
-                                     uint32_t aDecodeFlags,
-                                     gfx::SurfaceFormat aFormat,
-                                     uint8_t aPaletteDepth,
-                                     imgFrame* aPreviousFrame);
+  RawAccessFrameRef AllocateFrameInternal(uint32_t aFrameNum,
+                                          const nsIntRect& aFrameRect,
+                                          uint32_t aDecodeFlags,
+                                          gfx::SurfaceFormat aFormat,
+                                          uint8_t aPaletteDepth,
+                                          imgFrame* aPreviousFrame);
+
   /*
    * Member variables.
    *
    */
   RasterImage &mImage;
   RawAccessFrameRef mCurrentFrame;
   ImageMetadata mImageMetadata;
   nsIntRect mInvalidRect; // Tracks an invalidation region in the current frame.
@@ -314,37 +265,16 @@ protected:
   bool mDecodeDone;
   bool mDataError;
 
 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/src/ImageMetadata.h
+++ b/image/src/ImageMetadata.h
@@ -48,16 +48,17 @@ public:
     }
   }
 
   bool HasSize() const { return mSize.isSome(); }
   bool HasOrientation() const { return mOrientation.isSome(); }
 
   int32_t GetWidth() const { return mSize->width; }
   int32_t GetHeight() const { return mSize->height; }
+  nsIntSize GetSize() const { return *mSize; }
   Orientation GetOrientation() const { return *mOrientation; }
 
 private:
   // The hotspot found on cursors, or -1 if none was found.
   int32_t mHotspotX;
   int32_t mHotspotY;
 
   // The loop count for animated images, or -1 for infinite loop.
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -928,19 +928,19 @@ 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 ||
+             aNewFrameCount == mFrameCount + 1,
+             "Skipped a frame?");
 
   mFrameCount = aNewFrameCount;
 
   if (aNewFrameCount == 2) {
     // We're becoming animated, so initialize animation stuff.
     MOZ_ASSERT(!mAnim, "Already have animation state?");
     mAnim = MakeUnique<FrameAnimator>(this, mSize.ToIntSize(), mAnimationMode);
 
@@ -1475,25 +1475,16 @@ RasterImage::InitDecoder(bool aDoSizeDec
       break;
     default:
       NS_ABORT_IF_FALSE(0, "Shouldn't get here!");
   }
 
   // Initialize the decoder
   mDecoder->SetSizeDecode(aDoSizeDecode);
   mDecoder->SetDecodeFlags(mFrameDecodeFlags);
-  if (!aDoSizeDecode) {
-    // 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.
-    mDecoder->SetSize(mSize, mOrientation);
-    mDecoder->NeedNewFrame(0, 0, 0, mSize.width, mSize.height,
-                           SurfaceFormat::B8G8R8A8);
-    mDecoder->AllocateFrame();
-  }
   mDecoder->Init();
   CONTAINER_ENSURE_SUCCESS(mDecoder->GetDecoderError());
 
   if (!aDoSizeDecode) {
     Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Subtract(mDecodeCount);
     mDecodeCount++;
     Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(mDecodeCount);
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/2dcontext/drawing-images-to-the-canvas/2d.drawImage.broken.html.ini
@@ -0,0 +1,5 @@
+[2d.drawImage.broken.html]
+  type: testharness
+  [Canvas test: 2d.drawImage.broken]
+    expected: FAIL
+