Bug 641748 - Remove RasterImage::AppendFrame in favour of RasterImage::EnsureFrame, because decoders get re-initialized when using multipart/x-mixed-replace, and this can lead to the frame count for RasterImages being different from the frame count for decoders. r=jrmuizel

modules/libpr0n/decoders/nsBMPDecoder.cpp

@@ -228,7 +228,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, PRUint32 aCount)
 
         PRUint32 imageLength;
         if ((mBIH.compression == BI_RLE8) || (mBIH.compression == BI_RLE4)) {
-            rv = mImage->AppendFrame(0, 0, mBIH.width, real_height, gfxASurface::ImageFormatARGB32,
+            rv = mImage->EnsureFrame(0, 0, 0, mBIH.width, real_height, gfxASurface::ImageFormatARGB32,
                                      (PRUint8**)&mImageData, &imageLength);
         } else {
             // mRow is not used for RLE encoded images
@@ -240,7 +240,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, PRUint32 aCount)
                 PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
                 return;
             }
-            rv = mImage->AppendFrame(0, 0, mBIH.width, real_height, gfxASurface::ImageFormatRGB24,
+            rv = mImage->EnsureFrame(0, 0, 0, mBIH.width, real_height, gfxASurface::ImageFormatRGB24,
                                      (PRUint8**)&mImageData, &imageLength);
         }
         if (NS_FAILED(rv) || !mImageData) {

modules/libpr0n/decoders/nsGIFDecoder2.cpp

@@ -215,13 +215,15 @@ nsresult nsGIFDecoder2::BeginImageFrame(gfx_depth aDepth)
   // and include transparency to allow for optimization of opaque images
   if (mGIFStruct.images_decoded) {
     // Image data is stored with original depth and palette
-    rv = mImage->AppendPalettedFrame(mGIFStruct.x_offset, mGIFStruct.y_offset,
-                                     mGIFStruct.width, mGIFStruct.height,
-                                     format, aDepth, &mImageData, &imageDataLength,
-                                     &mColormap, &mColormapSize);
+    rv = mImage->EnsureFrame(mGIFStruct.images_decoded,
+                             mGIFStruct.x_offset, mGIFStruct.y_offset,
+                             mGIFStruct.width, mGIFStruct.height,
+                             format, aDepth, &mImageData, &imageDataLength,
+                             &mColormap, &mColormapSize);
   } else {
     // Regardless of depth of input, image is decoded into 24bit RGB
-    rv = mImage->AppendFrame(mGIFStruct.x_offset, mGIFStruct.y_offset,
+    rv = mImage->EnsureFrame(mGIFStruct.images_decoded,
+                             mGIFStruct.x_offset, mGIFStruct.y_offset,
                              mGIFStruct.width, mGIFStruct.height,
                              format, &mImageData, &imageDataLength);
   }

modules/libpr0n/decoders/nsICODecoder.cpp

@@ -261,7 +261,7 @@ nsICODecoder::WriteInternal(const char* aBuffer, PRUint32 aCount)
     }
 
     PRUint32 imageLength;
-    rv = mImage->AppendFrame(0, 0, mDirEntry.mWidth, mDirEntry.mHeight,
+    rv = mImage->EnsureFrame(0, 0, 0, mDirEntry.mWidth, mDirEntry.mHeight,
                              gfxASurface::ImageFormatARGB32, (PRUint8**)&mImageData, &imageLength);
     if (NS_FAILED(rv)) {
       PostDecoderError(rv);

modules/libpr0n/decoders/nsIconDecoder.cpp

@@ -113,7 +113,7 @@ nsIconDecoder::WriteInternal(const char *aBuffer, PRUint32 aCount)
         }
 
         // Add the frame and signal
-        rv = mImage->AppendFrame(0, 0, mWidth, mHeight,
+        rv = mImage->EnsureFrame(0, 0, 0, mWidth, mHeight,
                                  gfxASurface::ImageFormatARGB32,
                                  &mImageData, &mPixBytesTotal);
         if (NS_FAILED(rv)) {

modules/libpr0n/decoders/nsJPEGDecoder.cpp

@@ -385,13 +385,10 @@ nsJPEGDecoder::WriteInternal(const char *aBuffer, PRUint32 aCount)
     /* Used to set up image size so arrays can be allocated */
     jpeg_calc_output_dimensions(&mInfo);
 
-
-    // Use EnsureCleanFrame so we don't create a new frame if we're being
-    // reused for e.g. multipart/x-replace
     PRUint32 imagelength;
-    if (NS_FAILED(mImage->EnsureCleanFrame(0, 0, 0, mInfo.image_width, mInfo.image_height,
-                                           gfxASurface::ImageFormatRGB24,
-                                           &mImageData, &imagelength))) {
+    if (NS_FAILED(mImage->EnsureFrame(0, 0, 0, mInfo.image_width, mInfo.image_height,
+                                      gfxASurface::ImageFormatRGB24,
+                                      &mImageData, &imagelength))) {
       mState = JPEG_ERROR;
       PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
       PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,

modules/libpr0n/decoders/nsPNGDecoder.cpp

@@ -117,7 +117,8 @@ void nsPNGDecoder::CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset,
                                gfxASurface::gfxImageFormat format)
 {
   PRUint32 imageDataLength;
-  nsresult rv = mImage->AppendFrame(x_offset, y_offset, width, height, format,
+  nsresult rv = mImage->EnsureFrame(GetFrameCount(), x_offset, y_offset,
+                                    width, height, format,
                                     &mImageData, &imageDataLength);
   if (NS_FAILED(rv))
     longjmp(png_jmpbuf(mPNG), 5); // NS_ERROR_OUT_OF_MEMORY

modules/libpr0n/src/RasterImage.cpp

@@ -863,48 +863,6 @@ RasterImage::InternalAddFrame(PRUint32 framenum,
   return rv;
 }
 
-nsresult
-RasterImage::AppendFrame(PRInt32 aX, PRInt32 aY, PRInt32 aWidth,
-                         PRInt32 aHeight,
-                         gfxASurface::gfxImageFormat aFormat,
-                         PRUint8 **imageData,
-                         PRUint32 *imageLength)
-{
-  if (mError)
-    return NS_ERROR_FAILURE;
-
-  NS_ENSURE_ARG_POINTER(imageData);
-  NS_ENSURE_ARG_POINTER(imageLength);
-
-  return InternalAddFrame(mFrames.Length(), aX, aY, aWidth, aHeight, aFormat, 
-                          /* aPaletteDepth = */ 0, imageData, imageLength,
-                          /* aPaletteData = */ nsnull, 
-                          /* aPaletteLength = */ nsnull);
-}
-
-nsresult
-RasterImage::AppendPalettedFrame(PRInt32 aX, PRInt32 aY,
-                                 PRInt32 aWidth, PRInt32 aHeight,
-                                 gfxASurface::gfxImageFormat aFormat,
-                                 PRUint8 aPaletteDepth,
-                                 PRUint8 **imageData,
-                                 PRUint32 *imageLength,
-                                 PRUint32 **paletteData,
-                                 PRUint32 *paletteLength)
-{
-  if (mError)
-    return NS_ERROR_FAILURE;
-
-  NS_ENSURE_ARG_POINTER(imageData);
-  NS_ENSURE_ARG_POINTER(imageLength);
-  NS_ENSURE_ARG_POINTER(paletteData);
-  NS_ENSURE_ARG_POINTER(paletteLength);
-
-  return InternalAddFrame(mFrames.Length(), aX, aY, aWidth, aHeight, aFormat, 
-                          aPaletteDepth, imageData, imageLength,
-                          paletteData, paletteLength);
-}
-
 nsresult
 RasterImage::SetSize(PRInt32 aWidth, PRInt32 aHeight)
 {
@@ -943,10 +901,12 @@ RasterImage::SetSize(PRInt32 aWidth, PRInt32 aHeight)
 }
 
 nsresult
-RasterImage::EnsureCleanFrame(PRUint32 aFrameNum, PRInt32 aX, PRInt32 aY,
-                              PRInt32 aWidth, PRInt32 aHeight,
-                              gfxASurface::gfxImageFormat aFormat,
-                              PRUint8 **imageData, PRUint32 *imageLength)
+RasterImage::EnsureFrame(PRUint32 aFrameNum, PRInt32 aX, PRInt32 aY,
+                         PRInt32 aWidth, PRInt32 aHeight,
+                         gfxASurface::gfxImageFormat aFormat,
+                         PRUint8 aPaletteDepth,
+                         PRUint8 **imageData, PRUint32 *imageLength,
+                         PRUint32 **paletteData, PRUint32 *paletteLength)
 {
   if (mError)
     return NS_ERROR_FAILURE;
@@ -954,39 +914,62 @@ RasterImage::EnsureCleanFrame(PRUint32 aFrameNum, PRInt32 aX, PRInt32 aY,
   NS_ENSURE_ARG_POINTER(imageData);
   NS_ENSURE_ARG_POINTER(imageLength);
   NS_ABORT_IF_FALSE(aFrameNum <= mFrames.Length(), "Invalid frame index!");
+
+  if (aPaletteDepth > 0) {
+    NS_ENSURE_ARG_POINTER(paletteData);
+    NS_ENSURE_ARG_POINTER(paletteLength);
+  }
+
   if (aFrameNum > mFrames.Length())
     return NS_ERROR_INVALID_ARG;
 
   // Adding a frame that doesn't already exist.
   if (aFrameNum == mFrames.Length())
     return InternalAddFrame(aFrameNum, aX, aY, aWidth, aHeight, aFormat, 
-                            /* aPaletteDepth = */ 0, imageData, imageLength,
-                            /* aPaletteData = */ nsnull, 
-                            /* aPaletteLength = */ nsnull);
+                            aPaletteDepth, imageData, imageLength,
+                            paletteData, paletteLength);
 
   imgFrame *frame = GetImgFrame(aFrameNum);
   if (!frame)
     return InternalAddFrame(aFrameNum, aX, aY, aWidth, aHeight, aFormat, 
-                            /* aPaletteDepth = */ 0, imageData, imageLength,
-                            /* aPaletteData = */ nsnull, 
-                            /* aPaletteLength = */ nsnull);
+                            aPaletteDepth, imageData, imageLength,
+                            paletteData, paletteLength);
 
   // See if we can re-use the frame that already exists.
   nsIntRect rect = frame->GetRect();
   if (rect.x == aX && rect.y == aY && rect.width == aWidth &&
-      rect.height == aHeight && frame->GetFormat() == aFormat) {
-    // We can re-use the frame if it has image data.
+      rect.height == aHeight && frame->GetFormat() == aFormat &&
+      frame->GetPaletteDepth() == aPaletteDepth) {
     frame->GetImageData(imageData, imageLength);
-    if (*imageData) {
+    if (paletteData) {
+      frame->GetPaletteData(paletteData, paletteLength);
+    }
+
+    // We can re-use the frame if it has image data.
+    if (*imageData && paletteData && *paletteData) {
+      return NS_OK;
+    }
+    if (*imageData && !paletteData) {
       return NS_OK;
     }
   }
 
   DeleteImgFrame(aFrameNum);
-  return InternalAddFrame(aFrameNum, aX, aY, aWidth, aHeight, aFormat, 
-                          /* aPaletteDepth = */ 0, imageData, imageLength,
-                          /* aPaletteData = */ nsnull, 
-                          /* aPaletteLength = */ nsnull);
+  return InternalAddFrame(aFrameNum, aX, aY, aWidth, aHeight, aFormat,
+                          aPaletteDepth, imageData, imageLength,
+                          paletteData, paletteLength);
+}
+
+nsresult
+RasterImage::EnsureFrame(PRUint32 aFramenum, PRInt32 aX, PRInt32 aY,
+                         PRInt32 aWidth, PRInt32 aHeight,
+                         gfxASurface::gfxImageFormat aFormat,
+                         PRUint8** imageData, PRUint32* imageLength)
+{
+  return EnsureFrame(aFramenum, aX, aY, aWidth, aHeight, aFormat,
+                     /* aPaletteDepth = */ 0, imageData, imageLength,
+                     /* aPaletteData = */ nsnull,
+                     /* aPaletteLength = */ nsnull);
 }
 
 void

modules/libpr0n/src/RasterImage.h

@@ -230,30 +230,32 @@ public:
    */
   nsresult SetSize(PRInt32 aWidth, PRInt32 aHeight);
 
-  nsresult EnsureCleanFrame(PRUint32 aFramenum, PRInt32 aX, PRInt32 aY,
-                            PRInt32 aWidth, PRInt32 aHeight,
-                            gfxASurface::gfxImageFormat aFormat,
-                            PRUint8** imageData,
-                            PRUint32* imageLength);
 
   /**
-   * Adds to the end of the list of frames.
+   * Ensures that a given frame number exists with the given parameters, and
+   * returns pointers to the data storage for that frame.
+   * It is not possible to create sparse frame arrays; you can only append
+   * frames to the current frame array.
    */
-  nsresult AppendFrame(PRInt32 aX, PRInt32 aY,
+  nsresult EnsureFrame(PRUint32 aFramenum, PRInt32 aX, PRInt32 aY,
+                       PRInt32 aWidth, PRInt32 aHeight,
+                       gfxASurface::gfxImageFormat aFormat,
+                       PRUint8 aPaletteDepth,
+                       PRUint8** imageData,
+                       PRUint32* imageLength,
+                       PRUint32** paletteData,
+                       PRUint32* paletteLength);
+
+  /**
+   * A shorthand for EnsureFrame, above, with aPaletteDepth = 0 and paletteData
+   * and paletteLength set to null.
+   */
+  nsresult EnsureFrame(PRUint32 aFramenum, PRInt32 aX, PRInt32 aY,
                        PRInt32 aWidth, PRInt32 aHeight,
                        gfxASurface::gfxImageFormat aFormat,
                        PRUint8** imageData,
                        PRUint32* imageLength);
 
-  nsresult AppendPalettedFrame(PRInt32 aX, PRInt32 aY,
-                               PRInt32 aWidth, PRInt32 aHeight,
-                               gfxASurface::gfxImageFormat aFormat,
-                               PRUint8 aPaletteDepth,
-                               PRUint8**  imageData,
-                               PRUint32*  imageLength,
-                               PRUint32** paletteData,
-                               PRUint32*  paletteLength);
-
   void FrameUpdated(PRUint32 aFrameNum, nsIntRect& aUpdatedRect);
 
   /* notification that the entire image has been decoded */

modules/libpr0n/src/imgFrame.h

@@ -138,6 +138,8 @@ public:
   // returns an estimate of the memory used by this imgFrame
   PRUint32 EstimateMemoryUsed() const;
 
+  PRUint8 GetPaletteDepth() const { return mPaletteDepth; }
+
 private: // methods
   PRUint32 PaletteDataLength() const {
     return ((1 << mPaletteDepth) * sizeof(PRUint32));