Bug 639303. r=joe a=dveditz
authorMats Palmgren <matspal@gmail.com>
Mon, 06 Jun 2011 23:21:55 +0200
changeset 63469 f77eaecf9e00c0613da81d9869d4593b447b69ef
parent 63465 6b705dc0c049042a3c11bc356e271d96719d8fda
child 63472 6f4d01073ef81549578425c9c90b29ded2d0d3f2
push id91
push usermpalmgren@mozilla.com
push dateMon, 06 Jun 2011 21:26:16 +0000
reviewersjoe, dveditz
bugs639303
milestone2.0.2pre
Bug 639303. r=joe a=dveditz
modules/libpr0n/decoders/nsBMPDecoder.cpp
modules/libpr0n/decoders/nsGIFDecoder2.cpp
modules/libpr0n/decoders/nsIconDecoder.cpp
modules/libpr0n/decoders/nsJPEGDecoder.cpp
modules/libpr0n/decoders/nsPNGDecoder.cpp
modules/libpr0n/src/Decoder.h
modules/libpr0n/src/RasterImage.cpp
--- a/modules/libpr0n/decoders/nsBMPDecoder.cpp
+++ b/modules/libpr0n/decoders/nsBMPDecoder.cpp
@@ -191,16 +191,21 @@ nsBMPDecoder::WriteInternal(const char* 
             PostDataError();
             return;
         }
 
         PRUint32 real_height = (mBIH.height > 0) ? mBIH.height : -mBIH.height;
 
         // Post our size to the superclass
         PostSize(mBIH.width, real_height);
+        if (HasError()) {
+          // Setting the size lead to an error; this can happen when for example
+          // a multipart channel sends an image of a different size.
+          return;
+        }
 
         // We have the size. If we're doing a size decode, we got what
         // we came for.
         if (IsSizeDecode())
             return;
 
         // We're doing a real decode.
         mOldLine = mCurLine = real_height;
--- a/modules/libpr0n/decoders/nsGIFDecoder2.cpp
+++ b/modules/libpr0n/decoders/nsGIFDecoder2.cpp
@@ -911,16 +911,22 @@ nsGIFDecoder2::WriteInternal(const char 
             (mGIFStruct.version == 87)) {
           mGIFStruct.screen_height = mGIFStruct.height;
           mGIFStruct.screen_width = mGIFStruct.width;
           mGIFStruct.x_offset = 0;
           mGIFStruct.y_offset = 0;
         }    
         // Create the image container with the right size.
         BeginGIF();
+        if (HasError()) {
+          // Setting the size lead to an error; this can happen when for example
+          // a multipart channel sends an image of a different size.
+          mGIFStruct.state = gif_error;
+          return;
+        }
 
         // If we were doing a size decode, we're done
         if (IsSizeDecode())
           return;
       }
 
       /* Work around more broken GIF files that have zero image
          width or height */
--- a/modules/libpr0n/decoders/nsIconDecoder.cpp
+++ b/modules/libpr0n/decoders/nsIconDecoder.cpp
@@ -94,16 +94,22 @@ nsIconDecoder::WriteInternal(const char 
 
       case iconStateHaveHeight:
 
         // Grab the Height
         mHeight = (PRUint8)*aBuffer;
 
         // Post our size to the superclass
         PostSize(mWidth, mHeight);
+        if (HasError()) {
+          // Setting the size lead to an error; this can happen when for example
+          // a multipart channel sends an image of a different size.
+          mState = iconStateFinished;
+          return;
+        }
 
         // If We're doing a size decode, we're done
         if (IsSizeDecode()) {
           mState = iconStateFinished;
           break;
         }
 
         // Add the frame and signal
--- a/modules/libpr0n/decoders/nsJPEGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsJPEGDecoder.cpp
@@ -237,16 +237,22 @@ nsJPEGDecoder::WriteInternal(const char 
     if (jpeg_read_header(&mInfo, TRUE) == JPEG_SUSPENDED) {
       PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
              ("} (JPEG_SUSPENDED)"));
       return; /* I/O suspension */
     }
 
     // Post our size to the superclass
     PostSize(mInfo.image_width, mInfo.image_height);
+    if (HasError()) {
+      // Setting the size lead to an error; this can happen when for example
+      // a multipart channel sends an image of a different size.
+      mState = JPEG_ERROR;
+      return;
+    }
 
     /* If we're doing a size decode, we're done. */
     if (IsSizeDecode())
       return;
 
     /* We're doing a full decode. */
     JOCTET  *profile;
     PRUint32 profileLength;
--- a/modules/libpr0n/decoders/nsPNGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsPNGDecoder.cpp
@@ -496,16 +496,21 @@ nsPNGDecoder::info_callback(png_structp 
                &interlace_type, &compression_type, &filter_type);
 
   /* Are we too big? */
   if (width > MOZ_PNG_MAX_DIMENSION || height > MOZ_PNG_MAX_DIMENSION)
     longjmp(png_jmpbuf(decoder->mPNG), 1);
 
   // Post our size to the superclass
   decoder->PostSize(width, height);
+  if (decoder->HasError()) {
+    // Setting the size lead to an error; this can happen when for example
+    // a multipart channel sends an image of a different size.
+    longjmp(png_jmpbuf(decoder->mPNG), 1);
+  }
 
   if (color_type == PNG_COLOR_TYPE_PALETTE)
     png_set_expand(png_ptr);
 
   if (color_type == PNG_COLOR_TYPE_GRAY && bit_depth < 8)
     png_set_expand(png_ptr);
 
   if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
--- a/modules/libpr0n/src/Decoder.h
+++ b/modules/libpr0n/src/Decoder.h
@@ -116,16 +116,17 @@ public:
   // The number of complete frames we have (ie, not including anything in-progress).
   PRUint32 GetCompleteFrameCount() { return mInFrame ? mFrameCount - 1 : mFrameCount; }
 
   // Error tracking
   bool HasError() { return HasDataError() || HasDecoderError(); };
   bool HasDataError() { return mDataError; };
   bool HasDecoderError() { return NS_FAILED(mFailCode); };
   nsresult GetDecoderError() { return mFailCode; };
+  void PostResizeError() { PostDataError(); }
 
   // flags.  Keep these in sync with imgIContainer.idl.
   // SetDecodeFlags must be called before Init(), otherwise
   // default flags are assumed.
   enum {
     DECODER_NO_PREMULTIPLY_ALPHA = 0x2,
     DECODER_NO_COLORSPACE_CONVERSION = 0x4
   };
--- a/modules/libpr0n/src/RasterImage.cpp
+++ b/modules/libpr0n/src/RasterImage.cpp
@@ -919,16 +919,21 @@ RasterImage::SetSize(PRInt32 aWidth, PRI
       ((aWidth != mSize.width) || (aHeight != mSize.height))) {
 
     // Alter the warning depending on whether the channel is multipart
     if (!mMultipart)
       NS_WARNING("Image changed size on redecode! This should not happen!");
     else
       NS_WARNING("Multipart channel sent an image of a different size");
 
+    // Make the decoder aware of the error so that it doesn't try to call
+    // FinishInternal during ShutdownDecoder.
+    if (mDecoder)
+      mDecoder->PostResizeError();
+
     DoError();
     return NS_ERROR_UNEXPECTED;
   }
 
   // Set the size and flag that we have it
   mSize.SizeTo(aWidth, aHeight);
   mHasSize = PR_TRUE;
 
@@ -1227,16 +1232,17 @@ RasterImage::AddSourceData(const char *a
   // If we're not storing source data, write it directly to the decoder
   if (!StoringSourceData()) {
     rv = WriteToDecoder(aBuffer, aCount);
     CONTAINER_ENSURE_SUCCESS(rv);
 
     // We're not storing source data, so this data is probably coming straight
     // from the network. In this case, we want to display data as soon as we
     // get it, so we want to flush invalidations after every write.
+    nsRefPtr<Decoder> kungFuDeathGrip = mDecoder;
     mInDecoder = PR_TRUE;
     mDecoder->FlushInvalidations();
     mInDecoder = PR_FALSE;
   }
 
   // Otherwise, we're storing data in the source buffer
   else {
 
@@ -2196,24 +2202,26 @@ RasterImage::ShutdownDecoder(eShutdownIn
 
   // Ensure that the decoder is initialized
   NS_ABORT_IF_FALSE(mDecoder, "Calling ShutdownDecoder() with no active decoder!");
 
   // Figure out what kind of decode we were doing before we get rid of our decoder
   bool wasSizeDecode = mDecoder->IsSizeDecode();
 
   // Finalize the decoder
+  // null out mDecoder, _then_ check for errors on the close (otherwise the
+  // error routine might re-invoke ShutdownDecoder)
+  nsRefPtr<Decoder> decoder = mDecoder;
+  mDecoder = nsnull;
+
   mInDecoder = PR_TRUE;
-  mDecoder->Finish();
+  decoder->Finish();
   mInDecoder = PR_FALSE;
 
-  // null out the decoder, _then_ check for errors on the close (otherwise the
-  // error routine might re-invoke ShutdownDecoder)
-  nsresult decoderStatus = mDecoder->GetDecoderError();
-  mDecoder = nsnull;
+  nsresult decoderStatus = decoder->GetDecoderError();
   if (NS_FAILED(decoderStatus)) {
     DoError();
     return decoderStatus;
   }
 
   // Kill off the worker
   mWorker = nsnull;
 
@@ -2248,27 +2256,31 @@ RasterImage::WriteToDecoder(const char *
   // Our invariant is that, while in the decoder, the last frame is always
   // locked, and all others are unlocked.
   if (mFrames.Length() > 0) {
     imgFrame *curframe = mFrames.ElementAt(mFrames.Length() - 1);
     curframe->LockImageData();
   }
 
   // Write
+  nsRefPtr<Decoder> kungFuDeathGrip = mDecoder;
   mInDecoder = PR_TRUE;
   mDecoder->Write(aBuffer, aCount);
   mInDecoder = PR_FALSE;
 
   // We unlock the current frame, even if that frame is different from the
   // frame we entered the decoder with. (See above.)
   if (mFrames.Length() > 0) {
     imgFrame *curframe = mFrames.ElementAt(mFrames.Length() - 1);
     curframe->UnlockImageData();
   }
 
+  if (!mDecoder)
+    return NS_ERROR_FAILURE;
+    
   CONTAINER_ENSURE_SUCCESS(mDecoder->GetDecoderError());
 
   // Keep track of the total number of bytes written over the lifetime of the
   // decoder
   mBytesDecoded += aCount;
 
   return NS_OK;
 }
@@ -2406,28 +2418,29 @@ RasterImage::SyncDecode()
   rv = WriteToDecoder(mSourceData.Elements() + mBytesDecoded,
                       mSourceData.Length() - mBytesDecoded);
   CONTAINER_ENSURE_SUCCESS(rv);
 
   // When we're doing a sync decode, we want to get as much information from the
   // image as possible. We've send the decoder all of our data, so now's a good
   // time  to flush any invalidations (in case we don't have all the data and what
   // we got left us mid-frame).
+  nsRefPtr<Decoder> kungFuDeathGrip = mDecoder;
   mInDecoder = PR_TRUE;
   mDecoder->FlushInvalidations();
   mInDecoder = PR_FALSE;
 
   // If we finished the decode, shutdown the decoder
-  if (IsDecodeFinished()) {
+  if (mDecoder && IsDecodeFinished()) {
     rv = ShutdownDecoder(eShutdownIntent_Done);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
-  // All good!
-  return NS_OK;
+  // All good if no errors!
+  return mError ? NS_ERROR_FAILURE : NS_OK;
 }
 
 //******************************************************************************
 /* [noscript] void draw(in gfxContext aContext,
  *                      in gfxGraphicsFilter aFilter,
  *                      [const] in gfxMatrix aUserSpaceToImageSpace,
  *                      [const] in gfxRect aFill,
  *                      [const] in nsIntRect aSubimage,
@@ -2648,16 +2661,18 @@ imgDecodeWorker::Run()
   if (image->mError)
     return NS_OK;
 
   // If we don't have a decoder, we must have finished already (for example,
   // a synchronous decode request came while the worker was pending).
   if (!image->mDecoder)
     return NS_OK;
 
+  nsRefPtr<Decoder> decoderKungFuDeathGrip = image->mDecoder;
+
   // Size decodes are cheap and we more or less want them to be
   // synchronous. Write all the data in that case, otherwise write a
   // chunk
   PRUint32 maxBytes = image->mDecoder->IsSizeDecode()
     ? image->mSourceData.Length() : gDecodeBytesAtATime;
 
   // Loop control
   PRBool haveMoreData = PR_TRUE;
@@ -2688,17 +2703,17 @@ imgDecodeWorker::Run()
   // flush once the whole frame is ready.
   if (!image->mHasBeenDecoded) {
     image->mInDecoder = PR_TRUE;
     image->mDecoder->FlushInvalidations();
     image->mInDecoder = PR_FALSE;
   }
 
   // If the decode finished, shutdown the decoder
-  if (image->IsDecodeFinished()) {
+  if (image->mDecoder && image->IsDecodeFinished()) {
     rv = image->ShutdownDecoder(RasterImage::eShutdownIntent_Done);
     if (NS_FAILED(rv)) {
       image->DoError();
       return rv;
     }
   }
 
   // If Conditions 1 & 2 are still true, then the only reason we bailed was