Bug 514033 - Error recovery for imagelib - part 2 - Notify the superclass error reporting framework when an error occurs.r=joe
authorBobby Holley <bobbyholley@gmail.com>
Sun, 12 Sep 2010 08:22:27 -0700
changeset 53662 03486e7a7b322faa3e8860e1f0a6e662251f827d
parent 53661 bdad0efef3000038c5b22fe796ef189157c07a28
child 53663 fc39a0377a13a9fb4978d826dbc3d6aa83f06405
push idunknown
push userunknown
push dateunknown
reviewersjoe
bugs514033
milestone2.0b6pre
Bug 514033 - Error recovery for imagelib - part 2 - Notify the superclass error reporting framework when an error occurs.r=joe
modules/libpr0n/decoders/nsBMPDecoder.cpp
modules/libpr0n/decoders/nsBMPDecoder.h
modules/libpr0n/decoders/nsGIFDecoder2.cpp
modules/libpr0n/decoders/nsGIFDecoder2.h
modules/libpr0n/decoders/nsICODecoder.cpp
modules/libpr0n/decoders/nsICODecoder.h
modules/libpr0n/decoders/nsIconDecoder.cpp
modules/libpr0n/decoders/nsIconDecoder.h
modules/libpr0n/decoders/nsJPEGDecoder.cpp
modules/libpr0n/decoders/nsPNGDecoder.cpp
modules/libpr0n/decoders/nsPNGDecoder.h
--- a/modules/libpr0n/decoders/nsBMPDecoder.cpp
+++ b/modules/libpr0n/decoders/nsBMPDecoder.cpp
@@ -66,17 +66,16 @@ nsBMPDecoder::nsBMPDecoder()
 {
     mColors = nsnull;
     mRow = nsnull;
     mCurPos = mPos = mNumColors = mRowBytes = 0;
     mOldLine = mCurLine = 1; // Otherwise decoder will never start
     mState = eRLEStateInitial;
     mStateData = 0;
     mLOH = WIN_HEADER_LENGTH;
-    mError = PR_FALSE;
 }
 
 nsBMPDecoder::~nsBMPDecoder()
 {
   delete[] mColors;
   if (mRow)
       free(mRow);
 }
@@ -95,17 +94,17 @@ nsBMPDecoder::InitInternal()
 
 nsresult
 nsBMPDecoder::FinishInternal()
 {
     // We should never make multiple frames
     NS_ABORT_IF_FALSE(GetFrameCount() <= 1, "Multiple BMP frames?");
 
     // Send notifications if appropriate
-    if (!IsSizeDecode() && !mError && (GetFrameCount() == 1)) {
+    if (!IsSizeDecode() && !IsError() && (GetFrameCount() == 1)) {
         PostFrameStop();
         mImage->DecodingComplete();
         if (mObserver) {
             mObserver->OnStopContainer(nsnull, mImage);
             mObserver->OnStopDecode(nsnull, NS_OK, nsnull);
         }
     }
     return NS_OK;
@@ -150,17 +149,17 @@ NS_METHOD nsBMPDecoder::CalcBitShift()
     mBitFields.blueLeftShift = 8 - length;
     return NS_OK;
 }
 
 nsresult
 nsBMPDecoder::WriteInternal(const char* aBuffer, PRUint32 aCount)
 {
     // No forgiveness
-    if (mError)
+    if (IsError())
       return NS_ERROR_FAILURE;
 
     // aCount=0 means EOF, mCurLine=0 means we're past end of image
     if (!aCount || !mCurLine)
         return NS_OK;
 
     nsresult rv;
     if (mPos < BFH_LENGTH) { /* In BITMAPFILEHEADER */
@@ -170,17 +169,17 @@ nsBMPDecoder::WriteInternal(const char* 
         memcpy(mRawBuf + mPos, aBuffer, toCopy);
         mPos += toCopy;
         aCount -= toCopy;
         aBuffer += toCopy;
     }
     if (mPos == BFH_LENGTH) {
         ProcessFileHeader();
         if (mBFH.signature[0] != 'B' || mBFH.signature[1] != 'M') {
-            mError = PR_TRUE;
+            PostDataError();
             return NS_ERROR_FAILURE;
         }
         if (mBFH.bihsize == OS2_BIH_LENGTH)
             mLOH = OS2_HEADER_LENGTH;
     }
     if (mPos >= BFH_LENGTH && mPos < mLOH) { /* In BITMAPINFOHEADER */
         PRUint32 toCopy = mLOH - mPos;
         if (toCopy > aCount)
@@ -192,25 +191,25 @@ nsBMPDecoder::WriteInternal(const char* 
     }
     if (mPos == mLOH) {
         ProcessInfoHeader();
         PR_LOG(gBMPLog, PR_LOG_DEBUG, ("BMP image is %lix%lix%lu. compression=%lu\n",
             mBIH.width, mBIH.height, mBIH.bpp, mBIH.compression));
         // Verify we support this bit depth
         if (mBIH.bpp != 1 && mBIH.bpp != 4 && mBIH.bpp != 8 &&
             mBIH.bpp != 16 && mBIH.bpp != 24 && mBIH.bpp != 32) {
-          mError = PR_TRUE;
+          PostDataError();
           return NS_ERROR_UNEXPECTED;
         }
 
         // BMPs with negative width are invalid
         // Reject extremely wide images to keep the math sane
         const PRInt32 k64KWidth = 0x0000FFFF;
         if (mBIH.width < 0 || mBIH.width > k64KWidth) {
-            mError = PR_TRUE;
+            PostDataError();
             return NS_ERROR_FAILURE;
         }
 
         PRUint32 real_height = (mBIH.height > 0) ? mBIH.height : -mBIH.height;
 
         // Post our size to the superclass
         PostSize(mBIH.width, real_height);
 
@@ -225,17 +224,17 @@ nsBMPDecoder::WriteInternal(const char* 
         if (mBIH.bpp <= 8) {
             mNumColors = 1 << mBIH.bpp;
             if (mBIH.colors && mBIH.colors < mNumColors)
                 mNumColors = mBIH.colors;
 
             // Always allocate 256 even though mNumColors might be smaller
             mColors = new colorTable[256];
             if (!mColors) {
-                mError = PR_TRUE;
+                PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
                 return NS_ERROR_OUT_OF_MEMORY;
             }
 
             memset(mColors, 0, 256 * sizeof(colorTable));
         }
         else if (mBIH.compression != BI_BITFIELDS && mBIH.bpp == 16) {
             // Use default 5-5-5 format
             mBitFields.red   = 0x7C00;
@@ -250,34 +249,34 @@ nsBMPDecoder::WriteInternal(const char* 
                                      (PRUint8**)&mImageData, &imageLength);
         } else {
             // mRow is not used for RLE encoded images
             mRow = (PRUint8*)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) {
-                mError = PR_TRUE;
+                PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
                 return NS_ERROR_OUT_OF_MEMORY;
             }
             rv = mImage->AppendFrame(0, 0, mBIH.width, real_height, gfxASurface::ImageFormatRGB24,
                                      (PRUint8**)&mImageData, &imageLength);
         }
         NS_ENSURE_SUCCESS(rv, rv);
         if (!mImageData) {
-            mError = PR_TRUE;
+            PostDecoderError(NS_ERROR_FAILURE);
             return NS_ERROR_FAILURE;
         }
 
         // Prepare for transparancy
         if ((mBIH.compression == BI_RLE8) || (mBIH.compression == BI_RLE4)) {
             if (((mBIH.compression == BI_RLE8) && (mBIH.bpp != 8)) 
              || ((mBIH.compression == BI_RLE4) && (mBIH.bpp != 4) && (mBIH.bpp != 1))) {
                 PR_LOG(gBMPLog, PR_LOG_DEBUG, ("BMP RLE8/RLE4 compression only supports 8/4 bits per pixel\n"));
-                mError = PR_TRUE;
+                PostDataError();
                 return NS_ERROR_FAILURE;
             }
             // Clear the image, as the RLE may jump over areas
             memset(mImageData, 0, imageLength);
         }
 
         // Tell the superclass we're starting a frame
         PostFrameStart();
@@ -411,17 +410,17 @@ nsBMPDecoder::WriteInternal(const char* 
 
                 }
             } while (aCount > 0);
         } 
         else if ((mBIH.compression == BI_RLE8) || (mBIH.compression == BI_RLE4)) {
             if (((mBIH.compression == BI_RLE8) && (mBIH.bpp != 8)) 
              || ((mBIH.compression == BI_RLE4) && (mBIH.bpp != 4) && (mBIH.bpp != 1))) {
                 PR_LOG(gBMPLog, PR_LOG_DEBUG, ("BMP RLE8/RLE4 compression only supports 8/4 bits per pixel\n"));
-                mError = PR_TRUE;
+                PostDataError();
                 return NS_ERROR_FAILURE;
             }
 
             while (aCount > 0) {
                 PRUint8 byte;
 
                 switch(mState) {
                     case eRLEStateInitial:
@@ -479,17 +478,17 @@ nsBMPDecoder::WriteInternal(const char* 
                             default : // absolute mode
                                 // Save the number of pixels to read
                                 mStateData = byte;
                                 if (mCurPos + mStateData > (PRUint32)mBIH.width) {
                                     // We can work around bitmaps that specify one
                                     // pixel too many, but only if their width is odd.
                                     mStateData -= mBIH.width & 1;
                                     if (mCurPos + mStateData > (PRUint32)mBIH.width) {
-                                        mError = PR_TRUE;
+                                        PostDataError();
                                         return NS_ERROR_FAILURE;
                                     }
                                 }
 
                                 // See if we will need to skip a byte
                                 // to word align the pixel data
                                 // mStateData is a number of pixels
                                 // so allow for the RLE compression type
@@ -564,18 +563,18 @@ nsBMPDecoder::WriteInternal(const char* 
                                 aCount--;
                                 mState = eRLEStateInitial;
                             }
                         }
                         // else state is still eRLEStateAbsoluteMode
                         continue;
 
                     default :
-                        NS_NOTREACHED("BMP RLE decompression: unknown state!");
-                        mError = PR_TRUE;
+                        NS_ABORT_IF_FALSE(0, "BMP RLE decompression: unknown state!");
+                        PostDecoderError(NS_ERROR_UNEXPECTED);
                         return NS_ERROR_FAILURE;
                 }
                 // Because of the use of the continue statement
                 // we only get here for eol, eof or y delta
                 if (mCurLine == 0) { // Finished last line
                     break;
                 }
             }
--- a/modules/libpr0n/decoders/nsBMPDecoder.h
+++ b/modules/libpr0n/decoders/nsBMPDecoder.h
@@ -173,17 +173,16 @@ private:
     PRUint8 *mRow;      ///< Holds one raw line of the image
     PRUint32 mRowBytes; ///< How many bytes of the row were already received
     PRInt32 mCurLine;   ///< Index of the line of the image that's currently being decoded
     PRInt32 mOldLine;   ///< Previous index of the line 
     PRInt32 mCurPos;    ///< Index in the current line of the image
 
     ERLEState mState;   ///< Maintains the current state of the RLE decoding
     PRUint32 mStateData;///< Decoding information that is needed depending on mState
-    PRBool mError;      ///< Did we hit an error?
 
     /** Set mBFH from the raw data in mRawBuf, converting from little-endian
      * data to native data as necessary */
     void ProcessFileHeader();
     /** Set mBIH from the raw data in mRawBuf, converting from little-endian
      * data to native data as necessary */
     void ProcessInfoHeader();
 };
--- a/modules/libpr0n/decoders/nsGIFDecoder2.cpp
+++ b/modules/libpr0n/decoders/nsGIFDecoder2.cpp
@@ -112,17 +112,16 @@ nsGIFDecoder2::nsGIFDecoder2()
   , mLastFlushedRow(-1)
   , mImageData(nsnull)
   , mOldColor(0)
   , mCurrentFrame(-1)
   , mCurrentPass(0)
   , mLastFlushedPass(0)
   , mGIFOpen(PR_FALSE)
   , mSawTransparency(PR_FALSE)
-  , mError(PR_FALSE)
   , mEnded(PR_FALSE)
 {
   // Clear out the structure, excluding the arrays
   memset(&mGIFStruct, 0, sizeof(mGIFStruct));
 }
 
 nsGIFDecoder2::~nsGIFDecoder2()
 {
@@ -142,17 +141,17 @@ nsGIFDecoder2::InitInternal()
 
   return NS_OK;
 }
 
 nsresult
 nsGIFDecoder2::FinishInternal()
 {
   // Send notifications if appropriate
-  if (!IsSizeDecode() && !mError) {
+  if (!IsSizeDecode() && !IsError()) {
     if (mCurrentFrame == mGIFStruct.images_decoded)
       EndImageFrame();
     EndGIF(/* aSuccess = */ PR_TRUE);
   }
 
   return NS_OK;
 }
 
@@ -189,17 +188,17 @@ nsGIFDecoder2::FlushImageData()
   }
   return rv;
 }
 
 nsresult
 nsGIFDecoder2::WriteInternal(const char *aBuffer, PRUint32 aCount)
 {
   // Don't forgive previously flagged errors
-  if (mError)
+  if (IsError())
     return NS_ERROR_FAILURE;
 
   // Push the data to the GIF decoder
   nsresult rv = GifWrite((const unsigned char *)aBuffer, aCount);
 
   // Flushing is only needed for first frame
   if (NS_SUCCEEDED(rv) && !mGIFStruct.images_decoded) {
     rv = FlushImageData();
@@ -207,33 +206,33 @@ nsGIFDecoder2::WriteInternal(const char 
     mLastFlushedRow = mCurrentRow;
     mLastFlushedPass = mCurrentPass;
   }
 
   // We do some fine-grained error control here. If we have at least one frame
   // of an animated gif, we still want to display it (mostly for legacy reasons).
   // libpr0n code is strict, so we have to lie and tell it we were successful. So
   // if we have something to salvage, we send off final decode notifications, and
-  // pretend that we're decoded. Otherwise, we set mError.
+  // pretend that we're decoded. Otherwise, we set a data error.
   if (NS_FAILED(rv)) {
 
     // Determine if we want to salvage the situation.
     // If we're salvaging, send off notifications.
     // Note that we need to make sure that we have 2 frames, since that tells us
     // that the first frame is complete (the second could be in any state).
     if (mImage && mImage->GetNumFrames() > 1) {
       EndGIF(/* aSuccess = */ PR_TRUE);
     }
 
-    // Otherwise, set mError
+    // Otherwise, set an error
     else
-      mError = PR_TRUE;
+      PostDataError();
   }
 
-  return mError ? NS_ERROR_FAILURE : NS_OK;
+  return IsError() ? NS_ERROR_FAILURE : NS_OK;
 }
 
 //******************************************************************************
 // GIF decoder callback methods. Part of public API for GIF2
 //******************************************************************************
 
 //******************************************************************************
 void nsGIFDecoder2::BeginGIF()
--- a/modules/libpr0n/decoders/nsGIFDecoder2.h
+++ b/modules/libpr0n/decoders/nsGIFDecoder2.h
@@ -94,17 +94,16 @@ private:
   // of decoding it, and -1 otherwise.
   PRInt32 mCurrentFrame;
 
   PRUint8 mCurrentPass;
   PRUint8 mLastFlushedPass;
   PRUint8 mColorMask;        // Apply this to the pixel to keep within colormap
   PRPackedBool mGIFOpen;
   PRPackedBool mSawTransparency;
-  PRPackedBool mError;
   PRPackedBool mEnded;
 
   gif_struct mGIFStruct;
 };
 
 } // namespace imagelib
 } // namespace mozilla
 
--- a/modules/libpr0n/decoders/nsICODecoder.cpp
+++ b/modules/libpr0n/decoders/nsICODecoder.cpp
@@ -74,17 +74,16 @@ PRUint32 nsICODecoder::CalcAlphaRowSize(
 
 nsICODecoder::nsICODecoder()
 {
   mPos = mNumColors = mRowBytes = mImageOffset = mCurrIcon = mNumIcons = 0;
   mCurLine = 1; // Otherwise decoder will never start
   mColors = nsnull;
   mRow = nsnull;
   mHaveAlphaData = mDecodingAndMask = PR_FALSE;
-  mError = PR_FALSE;
 }
 
 nsICODecoder::~nsICODecoder()
 {
   mPos = 0;
 
   delete[] mColors;
 
@@ -115,17 +114,17 @@ nsresult
 nsICODecoder::FinishInternal()
 {
   nsresult rv = NS_OK;
 
   // We should never make multiple frames
   NS_ABORT_IF_FALSE(GetFrameCount() <= 1, "Multiple ICO frames?");
 
   // Send notifications if appropriate
-  if (!IsSizeDecode() && !mError && (GetFrameCount() == 1)) {
+  if (!IsSizeDecode() && !IsError() && (GetFrameCount() == 1)) {
 
     // Invalidate
     nsIntRect r(0, 0, mDirEntry.mWidth, mDirEntry.mHeight);
     PostInvalidation(r);
 
     PostFrameStop();
     mImage->DecodingComplete();
     if (mObserver) {
@@ -136,26 +135,26 @@ nsICODecoder::FinishInternal()
 
   return rv;
 }
 
 nsresult
 nsICODecoder::WriteInternal(const char* aBuffer, PRUint32 aCount)
 {
   // No forgiveness
-  if (mError)
+  if (IsError())
     return NS_ERROR_FAILURE;
 
   if (!aCount) // aCount=0 means EOF
     return NS_OK;
 
   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)) {
-        mError = PR_TRUE;
+        PostDataError();
         return NS_ERROR_FAILURE;
       }
       mIsCursor = (*aBuffer == 2);
     }
     mPos++; aBuffer++; aCount--;
   }
 
   if (mPos == ICONCOUNTOFFSET && aCount >= 2) {
@@ -189,17 +188,17 @@ nsICODecoder::WriteInternal(const char* 
       ProcessDirEntry(e);
       if ((e.mWidth == PREFICONSIZE && e.mHeight == PREFICONSIZE && e.mBitCount >= colorDepth)
            || (mCurrIcon == mNumIcons && mImageOffset == 0)) {
         mImageOffset = e.mImageOffset;
 
         // ensure mImageOffset is >= the size of the direntry headers (bug #245631)
         PRUint32 minImageOffset = DIRENTRYOFFSET + mNumIcons*sizeof(mDirEntryArray);
         if (mImageOffset < minImageOffset) {
-          mError = PR_TRUE;
+          PostDataError();
           return NS_ERROR_FAILURE;
         }
 
         colorDepth = e.mBitCount;
         memcpy(&mDirEntry, &e, sizeof(IconDirEntry));
       }
     }
   }
@@ -243,23 +242,23 @@ nsICODecoder::WriteInternal(const char* 
           break;
         case 4:
           mNumColors = 16;
           break;
         case 8:
           mNumColors = 256;
           break;
         default:
-          mError = PR_TRUE;
+          PostDataError();
           return NS_ERROR_FAILURE;
       }
 
       mColors = new colorTable[mNumColors];
       if (!mColors) {
-        mError = PR_TRUE;
+        PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
         return NS_ERROR_OUT_OF_MEMORY;
       }
     }
 
     if (mIsCursor) {
       nsCOMPtr<nsISupportsPRUint32> intwrapx = do_CreateInstance("@mozilla.org/supports-PRUint32;1");
       nsCOMPtr<nsISupportsPRUint32> intwrapy = do_CreateInstance("@mozilla.org/supports-PRUint32;1");
 
@@ -273,17 +272,17 @@ nsICODecoder::WriteInternal(const char* 
     }
 
     mCurLine = mDirEntry.mHeight;
     mRow = (PRUint8*)malloc((mDirEntry.mWidth * 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) {
-      mError = PR_TRUE;
+      PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
     PRUint32 imageLength;
     rv = mImage->AppendFrame(0, 0, mDirEntry.mWidth, mDirEntry.mHeight,
                              gfxASurface::ImageFormatARGB32, (PRUint8**)&mImageData, &imageLength);
     NS_ENSURE_SUCCESS(rv, rv);
 
@@ -320,20 +319,22 @@ nsICODecoder::WriteInternal(const char* 
   if (!mDecodingAndMask && (mPos >= (mImageOffset + BITMAPINFOSIZE + mNumColors*4))) {
     if (mPos == (mImageOffset + BITMAPINFOSIZE + mNumColors*4)) {
       // Increment mPos to avoid reprocessing the info header.
       mPos++;
     }
 
     // Ensure memory has been allocated before decoding. If we get this far 
     // without allocated memory, the file is most likely invalid.
+    // XXXbholley - If null values can be triggered by bad input, why are we
+    // asserting here?
     NS_ASSERTION(mRow, "mRow is null");
     NS_ASSERTION(mImageData, "mImageData is null");
     if (!mRow || !mImageData) {
-      mError = PR_TRUE;
+      PostDataError();
       return NS_ERROR_FAILURE;
     }
 
     PRUint32 rowSize = (mBIH.bpp * mDirEntry.mWidth + 7) / 8; // +7 to round up
     if (rowSize % 4)
       rowSize += (4 - (rowSize % 4)); // Pad to DWORD Boundary
     PRUint32 toCopy;
     do {
@@ -409,17 +410,17 @@ nsICODecoder::WriteInternal(const char* 
                   }                        
                   SetPixel(d, p[2], p[1], p[0], mHaveAlphaData ? p[3] : 0xFF);
                   p += 4;
                   --lpos;
                 }
                 break;
               default:
                 // This is probably the wrong place to check this...
-                mError = PR_TRUE;
+                PostDataError();
                 return NS_ERROR_FAILURE;
             }
 
             if (mCurLine == 0)
               mDecodingAndMask = PR_TRUE;
               
             mRowBytes = 0;
         }
@@ -431,26 +432,26 @@ nsICODecoder::WriteInternal(const char* 
     PRUint32 rowSize = CalcAlphaRowSize();
 
     if (mPos == (1 + mImageOffset + BITMAPINFOSIZE + mNumColors*4)) {
       mPos++;
       mRowBytes = 0;
       mCurLine = mDirEntry.mHeight;
       mRow = (PRUint8*)realloc(mRow, rowSize);
       if (!mRow) {
-        mError = PR_TRUE;
+        PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
         return NS_ERROR_OUT_OF_MEMORY;
       }
     }
 
     // Ensure memory has been allocated before decoding.
     NS_ASSERTION(mRow, "mRow is null");
     NS_ASSERTION(mImageData, "mImageData is null");
     if (!mRow || !mImageData) {
-      mError = PR_TRUE;
+      PostDataError();
       return NS_ERROR_FAILURE;
     }
 
     while (mCurLine > 0 && aCount > 0) {
       PRUint32 toCopy = PR_MIN(rowSize - mRowBytes, aCount);
       if (toCopy) {
         memcpy(mRow + mRowBytes, aBuffer, toCopy);
         aCount -= toCopy;
--- a/modules/libpr0n/decoders/nsICODecoder.h
+++ b/modules/libpr0n/decoders/nsICODecoder.h
@@ -107,15 +107,14 @@ private:
   PRUint32 mRowBytes; // How many bytes of the row were already received
   PRInt32 mCurLine;
 
   PRUint32* mImageData;
 
   PRPackedBool mHaveAlphaData;
   PRPackedBool mIsCursor;
   PRPackedBool mDecodingAndMask;
-  PRPackedBool mError;
 };
 
 } // namespace imagelib
 } // namespace mozilla
 
 #endif
--- a/modules/libpr0n/decoders/nsIconDecoder.cpp
+++ b/modules/libpr0n/decoders/nsIconDecoder.cpp
@@ -87,16 +87,19 @@ nsIconDecoder::FinishInternal()
   return NS_OK;
 }
 
 nsresult
 nsIconDecoder::WriteInternal(const char *aBuffer, PRUint32 aCount)
 {
   nsresult rv;
 
+  if (IsError())
+    return NS_IMAGELIB_ERROR_FAILURE;
+
   // We put this here to avoid errors about crossing initialization with case
   // jumps on linux.
   PRUint32 bytesToRead = 0;
 
   // Performance isn't critical here, so our update rectangle is 
   // always the full icon
   nsIntRect r(0, 0, mWidth, mHeight);
 
@@ -128,17 +131,17 @@ nsIconDecoder::WriteInternal(const char 
           break;
         }
 
         // Add the frame and signal
         rv = mImage->AppendFrame(0, 0, mWidth, mHeight,
                                  gfxASurface::ImageFormatARGB32,
                                  &mImageData, &mPixBytesTotal);
         if (NS_FAILED(rv)) {
-          mState = iconStateError;
+          PostDecoderError(rv);
           return rv;
         }
 
         // Tell the superclass we're starting a frame
         PostFrameStart();
 
         // Book Keeping
         aBuffer++;
@@ -170,20 +173,16 @@ nsIconDecoder::WriteInternal(const char 
         break;
 
       case iconStateFinished:
 
         // Consume all excess data silently
         aCount = 0;
 
         break;
-
-      case iconStateError:
-        return NS_IMAGELIB_ERROR_FAILURE;
-        break;
     }
   }
 
   return NS_OK;
 }
 
 void
 nsIconDecoder::NotifyDone(PRBool aSuccess)
--- a/modules/libpr0n/decoders/nsIconDecoder.h
+++ b/modules/libpr0n/decoders/nsIconDecoder.h
@@ -91,16 +91,15 @@ public:
   PRBool mNotifiedDone;
   void NotifyDone(PRBool aSuccess);
 };
 
 enum {
   iconStateStart      = 0,
   iconStateHaveHeight = 1,
   iconStateReadPixels = 2,
-  iconStateFinished   = 3,
-  iconStateError      = 4
+  iconStateFinished   = 3
 };
 
 } // namespace imagelib
 } // namespace mozilla
 
 #endif // nsIconDecoder_h__
--- a/modules/libpr0n/decoders/nsJPEGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsJPEGDecoder.cpp
@@ -150,16 +150,17 @@ nsJPEGDecoder::InitInternal()
   mInfo.err = jpeg_std_error(&mErr.pub);
   /*   mInfo.err = jpeg_std_error(&mErr.pub); */
   mErr.pub.error_exit = my_error_exit;
   /* Establish the setjmp return context for my_error_exit to use. */
   if (setjmp(mErr.setjmp_buffer)) {
     /* If we get here, the JPEG code has signaled an error.
      * We need to clean up the JPEG object, close the input file, and return.
      */
+    PostDecoderError(NS_ERROR_FAILURE);
     return NS_ERROR_FAILURE;
   }
 
   /* Step 1: allocate and initialize JPEG decompression object */
   jpeg_create_decompress(&mInfo);
   /* Set the source manager */
   mInfo.src = &mSourceMgr;
 
@@ -212,26 +213,28 @@ nsJPEGDecoder::WriteInternal(const char 
 {
   mSegment = (const JOCTET *)aBuffer;
   mSegmentLen = aCount;
 
   /* Return here if there is a fatal error within libjpeg. */
   nsresult error_code;
   if ((error_code = setjmp(mErr.setjmp_buffer)) != 0) {
     if (error_code == NS_ERROR_FAILURE) {
+      PostDataError();
       /* Error due to corrupt stream - return NS_OK and consume silently
          so that libpr0n doesn't throw away a partial image load */
       mState = JPEG_SINK_NON_JPEG_TRAILER;
       PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
              ("} (setjmp returned NS_ERROR_FAILURE)"));
       return NS_OK;
     } else {
       /* Error due to reasons external to the stream (probably out of
          memory) - let libpr0n attempt to clean up, even though
          mozilla is seconds away from falling flat on its face. */
+      PostDecoderError(error_code);
       mState = JPEG_ERROR;
       PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
              ("} (setjmp returned an error)"));
       return error_code;
     }
   }
 
   PR_LOG(gJPEGlog, PR_LOG_DEBUG,
@@ -292,32 +295,34 @@ nsJPEGDecoder::WriteInternal(const char 
         break;
       case JCS_CMYK:
       case JCS_YCCK:
 	  // qcms doesn't support cmyk
           mismatch = PR_TRUE;
         break;
       default:
         mState = JPEG_ERROR;
+        PostDataError();
         PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
                ("} (unknown colorpsace (1))"));
         return NS_ERROR_UNEXPECTED;
       }
 
       if (!mismatch) {
         qcms_data_type type;
         switch (mInfo.out_color_space) {
         case JCS_GRAYSCALE:
           type = QCMS_DATA_GRAY_8;
           break;
         case JCS_RGB:
           type = QCMS_DATA_RGB_8;
           break;
         default:
           mState = JPEG_ERROR;
+          PostDataError();
           PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
                  ("} (unknown colorpsace (2))"));
           return NS_ERROR_UNEXPECTED;
         }
 #if 0
         /* We don't currently support CMYK profiles. The following
          * code dealt with lcms types. Add something like this
          * back when we gain support for CMYK.
@@ -357,16 +362,17 @@ nsJPEGDecoder::WriteInternal(const char 
         break;
       case JCS_CMYK:
       case JCS_YCCK:
         /* libjpeg can convert from YCCK to CMYK, but not to RGB */
         mInfo.out_color_space = JCS_CMYK;
         break;
       default:
         mState = JPEG_ERROR;
+        PostDataError();
         PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
                ("} (unknown colorpsace (3))"));
         return NS_ERROR_UNEXPECTED;
         break;
       }
     }
 
     /*
@@ -381,16 +387,17 @@ nsJPEGDecoder::WriteInternal(const char 
 
     // 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))) {
       mState = JPEG_ERROR;
+      PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
       PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
              ("} (could not initialize image frame)"));
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
     PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
            ("        JPEGDecoderAccounting: nsJPEGDecoder::Write -- created image frame with %ux%u pixels",
             mInfo.image_width, mInfo.image_height));
@@ -546,19 +553,17 @@ nsJPEGDecoder::WriteInternal(const char 
   }
   case JPEG_SINK_NON_JPEG_TRAILER:
     PR_LOG(gJPEGlog, PR_LOG_DEBUG,
            ("[this=%p] nsJPEGDecoder::ProcessData -- entering JPEG_SINK_NON_JPEG_TRAILER case\n", this));
 
     break;
 
   case JPEG_ERROR:
-    PR_LOG(gJPEGlog, PR_LOG_DEBUG,
-           ("[this=%p] nsJPEGDecoder::ProcessData -- entering JPEG_ERROR case\n", this));
-    return NS_ERROR_FAILURE;
+    NS_ABORT_IF_FALSE(0, "Should always return immediately after error and not re-enter decoder");
   }
 
   PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
          ("} (end of function)"));
   return NS_OK;
 }
 
 void
--- a/modules/libpr0n/decoders/nsPNGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsPNGDecoder.cpp
@@ -81,17 +81,17 @@ static PRLogModuleInfo *gPNGDecoderAccou
 static const PRUint8 pngSignatureBytes[] =
                { 137, 80, 78, 71, 13, 10, 26, 10 };
 
 nsPNGDecoder::nsPNGDecoder() :
   mPNG(nsnull), mInfo(nsnull),
   mCMSLine(nsnull), interlacebuf(nsnull),
   mInProfile(nsnull), mTransform(nsnull),
   mHeaderBuf(nsnull), mHeaderBytesRead(0),
-  mChannels(0), mError(PR_FALSE), mFrameIsHidden(PR_FALSE),
+  mChannels(0), mFrameIsHidden(PR_FALSE),
   mNotifiedDone(PR_FALSE)
 {
 }
 
 nsPNGDecoder::~nsPNGDecoder()
 {
   if (mPNG)
     png_destroy_read_struct(&mPNG, mInfo ? &mInfo : NULL, NULL);
@@ -237,34 +237,39 @@ nsPNGDecoder::InitInternal()
 
   // Fire OnStartDecode at init time to support bug 512435
   if (!IsSizeDecode() && mObserver)
     mObserver->OnStartDecode(nsnull);
 
   // For size decodes, we only need a small buffer
   if (IsSizeDecode()) {
     mHeaderBuf = (PRUint8 *)nsMemory::Alloc(BYTES_NEEDED_FOR_DIMENSIONS);
-    if (!mHeaderBuf)
+    if (!mHeaderBuf) {
+      PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
       return NS_ERROR_OUT_OF_MEMORY;
+    }
     return NS_OK;
   }
 
   /* For full decodes, do png init stuff */
 
   /* Initialize the container's source image header. */
   /* Always decode to 24 bit pixdepth */
 
   mPNG = png_create_read_struct(PNG_LIBPNG_VER_STRING,
                                 NULL, nsPNGDecoder::error_callback,
                                 nsPNGDecoder::warning_callback);
-  if (!mPNG)
+  if (!mPNG) {
+    PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
     return NS_ERROR_OUT_OF_MEMORY;
+  }
 
   mInfo = png_create_info_struct(mPNG);
   if (!mInfo) {
+    PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
     png_destroy_read_struct(&mPNG, NULL, NULL);
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
 #ifdef PNG_HANDLE_AS_UNKNOWN_SUPPORTED
   /* Ignore unused chunks */
   if (gfxPlatform::GetCMSMode() == eCMSMode_Off)
     png_set_keep_unknown_chunks(mPNG, 1, color_chunks, 2);
@@ -303,17 +308,17 @@ nsPNGDecoder::FinishInternal()
 nsresult
 nsPNGDecoder::WriteInternal(const char *aBuffer, PRUint32 aCount)
 {
   // We use gotos, so we need to declare variables here
   PRUint32 width = 0;
   PRUint32 height = 0;
 
   // No forgiveness if we previously hit an error
-  if (mError)
+  if (IsError())
     goto error;
 
   // If we only want width/height, we don't need to go through libpng
   if (IsSizeDecode()) {
 
     // Are we done?
     if (mHeaderBytesRead == BYTES_NEEDED_FOR_DIMENSIONS)
       return NS_OK;
@@ -323,51 +328,61 @@ nsPNGDecoder::WriteInternal(const char *
                                   mHeaderBytesRead);
     memcpy(mHeaderBuf + mHeaderBytesRead, aBuffer, bytesToRead);
     mHeaderBytesRead += bytesToRead;
 
     // If we're done now, verify the data and set up the container
     if (mHeaderBytesRead == BYTES_NEEDED_FOR_DIMENSIONS) {
 
       // Check that the signature bytes are right
-      if (memcmp(mHeaderBuf, pngSignatureBytes, sizeof(pngSignatureBytes)))
+      if (memcmp(mHeaderBuf, pngSignatureBytes, sizeof(pngSignatureBytes))) {
+        PostDataError();
         goto error;
+      }
 
       // Grab the width and height, accounting for endianness (thanks libpng!)
       width = png_get_uint_32(mHeaderBuf + WIDTH_OFFSET);
       height = png_get_uint_32(mHeaderBuf + HEIGHT_OFFSET);
 
       // Too big?
-      if ((width > MOZ_PNG_MAX_DIMENSION) || (height > MOZ_PNG_MAX_DIMENSION))
+      if ((width > MOZ_PNG_MAX_DIMENSION) || (height > MOZ_PNG_MAX_DIMENSION)) {
+        PostDataError();
         goto error;
+      }
 
       // Post our size to the superclass
       PostSize(width, height);
     }
   }
 
   // Otherwise, we're doing a standard decode
   else {
 
     // libpng uses setjmp/longjmp for error handling - set the buffer
     if (setjmp(png_jmpbuf(mPNG))) {
+
+      // We might not really know what caused the error, but it makes more
+      // sense to blame the data.
+      if (!IsError())
+        PostDataError();
+
       png_destroy_read_struct(&mPNG, &mInfo, NULL);
       goto error;
     }
 
     // Pass the data off to libpng
     png_process_data(mPNG, mInfo, (unsigned char *)aBuffer, aCount);
 
   }
 
   return NS_OK;
 
   // Consolidate error handling
   error:
-  mError = PR_TRUE;
+  NS_ABORT_IF_FALSE(IsError(), "Should only get here if we flagged an error!");
   return NS_ERROR_FAILURE;
 }
 
 void
 nsPNGDecoder::NotifyDone(PRBool aSuccess)
 {
   // We should only be called once
   NS_ABORT_IF_FALSE(!mNotifiedDone, "Calling NotifyDone twice!");
@@ -786,19 +801,17 @@ nsPNGDecoder::row_callback(png_structp p
           *cptr32++ = GFX_PACKED_PIXEL(line[3], line[0], line[1], line[2]);
           if (line[3] != 0xff)
             rowHasNoAlpha = PR_FALSE;
           line += 4;
         }
       }
       break;
       default:
-        NS_ERROR("Unknown PNG format!");
-        NS_ABORT();
-        break;
+        longjmp(png_jmpbuf(decoder->mPNG), 1);
     }
 
     if (!rowHasNoAlpha)
       decoder->mFrameHasNoAlpha = PR_FALSE;
 
     PRUint32 numFrames = decoder->mImage->GetNumFrames();
     if (numFrames <= 1) {
       // Only do incremental image display for the first frame
@@ -849,17 +862,17 @@ nsPNGDecoder::end_callback(png_structp p
    * Most people won't do much here, perhaps setting a flag that
    * marks the image as finished.
    */
 
   nsPNGDecoder *decoder =
                static_cast<nsPNGDecoder*>(png_get_progressive_ptr(png_ptr));
 
   // We shouldn't get here if we've hit an error
-  NS_ABORT_IF_FALSE(!decoder->mError, "Finishing up PNG but hit error!");
+  NS_ABORT_IF_FALSE(!decoder->IsError(), "Finishing up PNG but hit error!");
 
 #ifdef PNG_APNG_SUPPORTED
   if (png_get_valid(png_ptr, info_ptr, PNG_INFO_acTL)) {
     PRInt32 num_plays = png_get_num_plays(png_ptr, info_ptr);
     decoder->mImage->SetLoopCount(num_plays - 1);
   }
 #endif
 
--- a/modules/libpr0n/decoders/nsPNGDecoder.h
+++ b/modules/libpr0n/decoders/nsPNGDecoder.h
@@ -86,17 +86,16 @@ public:
 
   gfxASurface::gfxImageFormat format;
 
   // For size decodes
   PRUint8 *mHeaderBuf;
   PRUint32 mHeaderBytesRead;
 
   PRUint8 mChannels;
-  PRPackedBool mError;
   PRPackedBool mFrameHasNoAlpha;
   PRPackedBool mFrameIsHidden;
   PRPackedBool mNotifiedDone;
 
   /*
    * libpng callbacks
    *
    * We put these in the class so that they can access protected members.