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
authorJoe Drew <joe@drew.ca>
Thu, 14 Jul 2011 14:47:43 -0400
changeset 72836 15af6861b21bf4434b030e45f6faaeda940a3c05
parent 72835 d50618fed642458e81415f8a2f9811773f7d00fe
child 72837 451154c37cb34de3a9298e6b89d9e0e19d6c6770
push id20776
push usereakhgari@mozilla.com
push dateFri, 15 Jul 2011 12:13:35 +0000
treeherdermozilla-central@9349ae9094f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs641748
milestone8.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 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
modules/libpr0n/decoders/nsGIFDecoder2.cpp
modules/libpr0n/decoders/nsICODecoder.cpp
modules/libpr0n/decoders/nsIconDecoder.cpp
modules/libpr0n/decoders/nsJPEGDecoder.cpp
modules/libpr0n/decoders/nsPNGDecoder.cpp
modules/libpr0n/src/RasterImage.cpp
modules/libpr0n/src/RasterImage.h
modules/libpr0n/src/imgFrame.h
--- a/modules/libpr0n/decoders/nsBMPDecoder.cpp
+++ b/modules/libpr0n/decoders/nsBMPDecoder.cpp
@@ -223,29 +223,29 @@ nsBMPDecoder::WriteInternal(const char* 
             mBitFields.red   = 0x7C00;
             mBitFields.green = 0x03E0;
             mBitFields.blue  = 0x001F;
             CalcBitShift();
         }
 
         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
             mRow = (PRUint8*)moz_malloc((mBIH.width * mBIH.bpp)/8 + 4);
             // +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) {
                 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) {
             PostDecoderError(NS_ERROR_FAILURE);
             return;
         }
 
         // Prepare for transparancy
--- a/modules/libpr0n/decoders/nsGIFDecoder2.cpp
+++ b/modules/libpr0n/decoders/nsGIFDecoder2.cpp
@@ -210,23 +210,25 @@ nsresult nsGIFDecoder2::BeginImageFrame(
     format = gfxASurface::ImageFormatARGB32;
   else
     format = gfxASurface::ImageFormatRGB24;
 
   // Use correct format, RGB for first frame, PAL for following frames
   // 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);
   }
 
   if (NS_FAILED(rv))
     return rv;
 
   mImage->SetFrameDisposalMethod(mGIFStruct.images_decoded,
--- a/modules/libpr0n/decoders/nsICODecoder.cpp
+++ b/modules/libpr0n/decoders/nsICODecoder.cpp
@@ -256,17 +256,17 @@ nsICODecoder::WriteInternal(const char* 
     // to make exact calculations here, that's unnecessary.
     // Also, it compensates rounding error.
     if (!mRow) {
       PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
       return;
     }
 
     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);
       return;
     }
 
     // Tell the superclass we're starting a frame
     PostFrameStart();
--- a/modules/libpr0n/decoders/nsIconDecoder.cpp
+++ b/modules/libpr0n/decoders/nsIconDecoder.cpp
@@ -108,17 +108,17 @@ nsIconDecoder::WriteInternal(const char 
 
         // If We're doing a size decode, we're done
         if (IsSizeDecode()) {
           mState = iconStateFinished;
           break;
         }
 
         // 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)) {
           PostDecoderError(rv);
           return;
         }
 
         // Tell the superclass we're starting a frame
--- a/modules/libpr0n/decoders/nsJPEGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsJPEGDecoder.cpp
@@ -380,23 +380,20 @@ nsJPEGDecoder::WriteInternal(const char 
      * Don't allocate a giant and superfluous memory buffer
      * when the image is a sequential JPEG.
      */
     mInfo.buffered_image = jpeg_has_multiple_scans(&mInfo);
 
     /* 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,
              ("} (could not initialize image frame)"));
       return;
     }
 
     PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
--- a/modules/libpr0n/decoders/nsPNGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsPNGDecoder.cpp
@@ -112,17 +112,18 @@ nsPNGDecoder::~nsPNGDecoder()
 }
 
 // CreateFrame() is used for both simple and animated images
 void nsPNGDecoder::CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset,
                                PRInt32 width, PRInt32 height,
                                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
 
   mFrameRect.x = x_offset;
   mFrameRect.y = y_offset;
   mFrameRect.width = width;
   mFrameRect.height = height;
--- a/modules/libpr0n/src/RasterImage.cpp
+++ b/modules/libpr0n/src/RasterImage.cpp
@@ -859,58 +859,16 @@ RasterImage::InternalAddFrame(PRUint32 f
   
   // We may be able to start animating, if we now have enough frames
   EvaluateAnimation();
   
   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)
 {
   if (mError)
     return NS_ERROR_FAILURE;
 
   // Ensure that we have positive values
   // XXX - Why isn't the size unsigned? Should this be changed?
   if ((aWidth < 0) || (aHeight < 0))
@@ -938,60 +896,85 @@ RasterImage::SetSize(PRInt32 aWidth, PRI
   // Set the size and flag that we have it
   mSize.SizeTo(aWidth, aHeight);
   mHasSize = PR_TRUE;
 
   return NS_OK;
 }
 
 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;
 
   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) {
+      rect.height == aHeight && frame->GetFormat() == aFormat &&
+      frame->GetPaletteDepth() == aPaletteDepth) {
+    frame->GetImageData(imageData, imageLength);
+    if (paletteData) {
+      frame->GetPaletteData(paletteData, paletteLength);
+    }
+
     // We can re-use the frame if it has image data.
-    frame->GetImageData(imageData, imageLength);
-    if (*imageData) {
+    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
 RasterImage::FrameUpdated(PRUint32 aFrameNum, nsIntRect &aUpdatedRect)
 {
   NS_ABORT_IF_FALSE(aFrameNum < mFrames.Length(), "Invalid frame index!");
 
   imgFrame *frame = GetImgFrame(aFrameNum);
--- a/modules/libpr0n/src/RasterImage.h
+++ b/modules/libpr0n/src/RasterImage.h
@@ -225,40 +225,42 @@ public:
 
   /**
    * Sets the size of the container. This should only be called by the
    * decoder. This function may be called multiple times, but will throw an
    * error if subsequent calls do not match the first.
    */
   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 */
   nsresult DecodingComplete();
 
   /**
    * Number of times to loop the image.
    * @note -1 means forever.
--- a/modules/libpr0n/src/imgFrame.h
+++ b/modules/libpr0n/src/imgFrame.h
@@ -133,16 +133,18 @@ public:
       return mQuartzSurface;
 #endif
     return mImageSurface;
   }
 
   // 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));
   }
 
   struct SurfaceWithFormat {
     nsRefPtr<gfxDrawable> mDrawable;
     gfxImageSurface::gfxImageFormat mFormat;