Bug 514033 - Error recovery for imagelib - part 8 - Remove GIFWrite.r=joe,a=blocker
authorBobby Holley <bobbyholley@gmail.com>
Sun, 12 Sep 2010 08:22:30 -0700
changeset 53668 f5b73f397fed5d871ddf613917fbfbe08b7aa4f1
parent 53667 fb21557442d39a5aa898c9d6a33df7667d9a934d
child 53669 0d39ca861f0abf0ef4edb6450ecfe316385eb7cf
push idunknown
push userunknown
push dateunknown
reviewersjoe, blocker
bugs514033
milestone2.0b6pre
Bug 514033 - Error recovery for imagelib - part 8 - Remove GIFWrite.r=joe,a=blocker
modules/libpr0n/decoders/nsGIFDecoder2.cpp
modules/libpr0n/decoders/nsGIFDecoder2.h
--- 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)
-  , mEnded(PR_FALSE)
 {
   // Clear out the structure, excluding the arrays
   memset(&mGIFStruct, 0, sizeof(mGIFStruct));
 
   // Start with the version (GIF89a|GIF87a)
   mGIFStruct.state = gif_type;
   mGIFStruct.bytes_to_consume = 6;
 }
@@ -130,22 +129,24 @@ nsGIFDecoder2::nsGIFDecoder2()
 nsGIFDecoder2::~nsGIFDecoder2()
 {
   PR_FREEIF(mGIFStruct.local_colormap);
 }
 
 void
 nsGIFDecoder2::FinishInternal()
 {
-  // Send notifications if appropriate
+  // If the GIF got cut off, handle it anyway
   if (!IsSizeDecode() && !IsError()) {
     if (mCurrentFrame == mGIFStruct.images_decoded)
       EndImageFrame();
     EndGIF(/* aSuccess = */ PR_TRUE);
   }
+
+  mImage->SetLoopCount(mGIFStruct.loop_count);
 }
 
 // Push any new rows according to mCurrentPass/mLastFlushedPass and
 // mCurrentRow/mLastFlushedRow.  Note: caller is responsible for
 // updating mlastFlushed{Row,Pass}.
 void
 nsGIFDecoder2::FlushImageData(PRUint32 fromRow, PRUint32 rows)
 {
@@ -167,54 +168,16 @@ nsGIFDecoder2::FlushImageData()
       FlushImageData(mLastFlushedRow + 1, mGIFStruct.height - (mLastFlushedRow + 1));
       break;
 
     default:   // more than one pass on - push the whole frame
       FlushImageData(0, mGIFStruct.height);
   }
 }
 
-void
-nsGIFDecoder2::WriteInternal(const char *aBuffer, PRUint32 aCount)
-{
-  // Don't forgive previously flagged errors
-  if (IsError())
-    return;
-
-  // 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) {
-    FlushImageData();
-    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 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 an error
-    else
-      PostDataError();
-  }
-}
-
 //******************************************************************************
 // GIF decoder callback methods. Part of public API for GIF2
 //******************************************************************************
 
 //******************************************************************************
 void nsGIFDecoder2::BeginGIF()
 {
   if (mGIFOpen)
@@ -227,32 +190,29 @@ void nsGIFDecoder2::BeginGIF()
   // If we're doing a size decode, we have what we came for
   if (IsSizeDecode())
     return;
 }
 
 //******************************************************************************
 void nsGIFDecoder2::EndGIF(PRBool aSuccess)
 {
-  if (mEnded)
+  if (!mGIFOpen)
     return;
 
   if (aSuccess)
     mImage->DecodingComplete();
 
   if (mObserver) {
     mObserver->OnStopContainer(nsnull, mImage);
     mObserver->OnStopDecode(nsnull, aSuccess ? NS_OK : NS_ERROR_FAILURE,
                             nsnull);
   }
 
-  mImage->SetLoopCount(mGIFStruct.loop_count);
-
   mGIFOpen = PR_FALSE;
-  mEnded = PR_TRUE;
 }
 
 //******************************************************************************
 nsresult nsGIFDecoder2::BeginImageFrame(gfx_depth aDepth)
 {
   PRUint32 imageDataLength;
   nsresult rv;
   gfxASurface::gfxImageFormat format;
@@ -653,25 +613,26 @@ static void ConvertColormap(PRUint32 *aC
   // copy remaining pixel(s)
   // NB: can't use 32-bit reads, they might read off the end of the buffer
   while (c--) {
     from -= 3;
     *--to = GFX_PACKED_PIXEL(0xFF, from[0], from[1], from[2]);
   }
 }
 
-/******************************************************************************/
-/*
- * process data arriving from the stream for the gif decoder
- */
+void
+nsGIFDecoder2::WriteInternal(const char *aBuffer, PRUint32 aCount)
+{
+  // Don't forgive previously flagged errors
+  if (IsError())
+    return;
 
-nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
-{
-  if (!buf || !len)
-    return NS_ERROR_FAILURE;
+  // These variables changed names, and renaming would make a much bigger patch :(
+  const PRUint8 *buf = (const PRUint8 *)aBuffer;
+  PRUint32 len = aCount;
 
   const PRUint8 *q = buf;
 
   // Add what we have sofar to the block
   // If previous call to me left something in the hold first complete current block
   // Or if we are filling the colormaps, first complete the colormap
   PRUint8* p = (mGIFStruct.state == gif_global_colormap) ? (PRUint8*)mGIFStruct.global_colormap :
                (mGIFStruct.state == gif_image_colormap) ? (PRUint8*)mColormap :
@@ -680,17 +641,17 @@ nsresult nsGIFDecoder2::GifWrite(const P
     // Add what we have sofar to the block
     PRUint32 l = PR_MIN(len, mGIFStruct.bytes_to_consume);
     memcpy(p+mGIFStruct.bytes_in_hold, buf, l);
 
     if (l < mGIFStruct.bytes_to_consume) {
       // Not enough in 'buf' to complete current block, get more
       mGIFStruct.bytes_in_hold += l;
       mGIFStruct.bytes_to_consume -= l;
-      return NS_OK;
+      return;
     }
     // Reset hold buffer count
     mGIFStruct.bytes_in_hold = 0;
     // Point 'q' to complete block in hold (or in colormap)
     q = p;
   }
 
   // Invariant:
@@ -970,17 +931,17 @@ nsresult nsGIFDecoder2::GifWrite(const P
           mGIFStruct.x_offset = 0;
           mGIFStruct.y_offset = 0;
         }    
         // Create the image container with the right size.
         BeginGIF();
 
         // If we were doing a size decode, we're done
         if (IsSizeDecode())
-          return NS_OK;
+          return;
       }
 
       /* Work around more broken GIF files that have zero image
          width or height */
       if (!mGIFStruct.height || !mGIFStruct.width) {
         mGIFStruct.height = mGIFStruct.screen_height;
         mGIFStruct.width = mGIFStruct.screen_width;
         if (!mGIFStruct.height || !mGIFStruct.width) {
@@ -1102,48 +1063,58 @@ nsresult nsGIFDecoder2::GifWrite(const P
         /* See if there are any more images in this sequence. */
         EndImageFrame();
         GETN(1, gif_image_start);
       }
       break;
 
     case gif_done:
       EndGIF(/* aSuccess = */ PR_TRUE);
-      return NS_OK;
-      break;
+      goto done;
 
     case gif_error:
+      PostDataError();
       EndGIF(/* aSuccess = */ PR_FALSE);
-      return NS_ERROR_FAILURE;
-      break;
+      return;
 
     // Handle out of memory errors
     case gif_oom:
-      return NS_ERROR_OUT_OF_MEMORY;
+      PostDecoderError(NS_ERROR_OUT_OF_MEMORY);
+      EndGIF(/* aSuccess = */ PR_FALSE);
+      return;
 
     // We shouldn't ever get here.
     default:
       break;
     }
   }
 
   // if an error state is set but no data remains, code flow reaches here
   if (mGIFStruct.state == gif_error) {
+      PostDataError();
       EndGIF(/* aSuccess = */ PR_FALSE);
-      return NS_ERROR_FAILURE;
+      return;
   }
   
   // Copy the leftover into mGIFStruct.hold
   mGIFStruct.bytes_in_hold = len;
   if (len) {
     // Add what we have sofar to the block
     PRUint8* p = (mGIFStruct.state == gif_global_colormap) ? (PRUint8*)mGIFStruct.global_colormap :
                  (mGIFStruct.state == gif_image_colormap) ? (PRUint8*)mColormap :
                  mGIFStruct.hold;
     memcpy(p, buf, len);
     mGIFStruct.bytes_to_consume -= len;
   }
 
-  return NS_OK;
+// We want to flush before returning if we're on the first frame
+done:
+  if (!mGIFStruct.images_decoded) {
+    FlushImageData();
+    mLastFlushedRow = mCurrentRow;
+    mLastFlushedPass = mCurrentPass;
+  }
+
+  return;
 }
 
 } // namespace imagelib
 } // namespace mozilla
--- a/modules/libpr0n/decoders/nsGIFDecoder2.h
+++ b/modules/libpr0n/decoders/nsGIFDecoder2.h
@@ -93,17 +93,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 mEnded;
 
   gif_struct mGIFStruct;
 };
 
 } // namespace imagelib
 } // namespace mozilla
 
 #endif