From 94aab005de31e972ee3079e1f6d4425b99bad05b Mon Sep 17 00:00:00 2001
From: Bobby Holley <bobbyholley@gmail.com>
Date: Sun, 22 Aug 2010 22:30:46 -0400
Subject: [PATCH] Bug 513681 - part 16 - Move end-of-decode logic into Finish()
 and cleanup into destructor, abolishing Shutdown().r=joe,a=blocker

---
 modules/libpr0n/decoders/nsBMPDecoder.cpp  |  8 +--
 modules/libpr0n/decoders/nsBMPDecoder.h    |  2 +-
 modules/libpr0n/decoders/nsGIFDecoder2.cpp |  8 +--
 modules/libpr0n/decoders/nsGIFDecoder2.h   |  2 +-
 modules/libpr0n/decoders/nsICODecoder.cpp  | 77 +++++++++++-----------
 modules/libpr0n/decoders/nsICODecoder.h    |  2 +-
 modules/libpr0n/decoders/nsIconDecoder.cpp |  6 +-
 modules/libpr0n/decoders/nsIconDecoder.h   |  2 +-
 modules/libpr0n/decoders/nsJPEGDecoder.cpp | 21 ++----
 modules/libpr0n/decoders/nsJPEGDecoder.h   |  2 +-
 modules/libpr0n/decoders/nsPNGDecoder.cpp  | 10 ++-
 modules/libpr0n/decoders/nsPNGDecoder.h    |  2 +-
 modules/libpr0n/src/Decoder.cpp            | 22 ++-----
 modules/libpr0n/src/Decoder.h              | 12 +---
 modules/libpr0n/src/RasterImage.cpp        | 14 ++--
 15 files changed, 75 insertions(+), 115 deletions(-)

diff --git a/modules/libpr0n/decoders/nsBMPDecoder.cpp b/modules/libpr0n/decoders/nsBMPDecoder.cpp
index 3918db47513..9ec2339af6a 100644
--- a/modules/libpr0n/decoders/nsBMPDecoder.cpp
+++ b/modules/libpr0n/decoders/nsBMPDecoder.cpp
@@ -94,17 +94,13 @@ nsBMPDecoder::InitInternal()
 }
 
 nsresult
-nsBMPDecoder::ShutdownInternal(PRUint32 aFlags)
+nsBMPDecoder::FinishInternal()
 {
-    PR_LOG(gBMPLog, PR_LOG_DEBUG, ("nsBMPDecoder::Close()\n"));
-
     // We should never make multiple frames
     NS_ABORT_IF_FALSE(GetFrameCount() <= 1, "Multiple BMP frames?");
 
     // Send notifications if appropriate
-    if (!IsSizeDecode() &&
-        !mError && !(aFlags & CLOSE_FLAG_DONTNOTIFY) &&
-        (GetFrameCount() == 1)) {
+    if (!IsSizeDecode() && !mError && (GetFrameCount() == 1)) {
         PostFrameStop();
         mImage->DecodingComplete();
         if (mObserver) {
diff --git a/modules/libpr0n/decoders/nsBMPDecoder.h b/modules/libpr0n/decoders/nsBMPDecoder.h
index b45f2dba39e..c2394b873ad 100644
--- a/modules/libpr0n/decoders/nsBMPDecoder.h
+++ b/modules/libpr0n/decoders/nsBMPDecoder.h
@@ -148,7 +148,7 @@ public:
 
     virtual nsresult InitInternal();
     virtual nsresult WriteInternal(const char* aBuffer, PRUint32 aCount);
-    virtual nsresult ShutdownInternal(PRUint32 aFlags);
+    virtual nsresult FinishInternal();
 
 private:
 
diff --git a/modules/libpr0n/decoders/nsGIFDecoder2.cpp b/modules/libpr0n/decoders/nsGIFDecoder2.cpp
index bbd757f1897..015a8efa88f 100644
--- a/modules/libpr0n/decoders/nsGIFDecoder2.cpp
+++ b/modules/libpr0n/decoders/nsGIFDecoder2.cpp
@@ -126,6 +126,7 @@ nsGIFDecoder2::nsGIFDecoder2()
 
 nsGIFDecoder2::~nsGIFDecoder2()
 {
+  PR_FREEIF(mGIFStruct.local_colormap);
 }
 
 nsresult
@@ -143,18 +144,15 @@ nsGIFDecoder2::InitInternal()
 }
 
 nsresult
-nsGIFDecoder2::ShutdownInternal(PRUint32 aFlags)
+nsGIFDecoder2::FinishInternal()
 {
   // Send notifications if appropriate
-  if (!IsSizeDecode() &&
-      !mError && !(aFlags & CLOSE_FLAG_DONTNOTIFY)) {
+  if (!IsSizeDecode() && !mError) {
     if (mCurrentFrame == mGIFStruct.images_decoded)
       EndImageFrame();
     EndGIF(/* aSuccess = */ PR_TRUE);
   }
 
-  PR_FREEIF(mGIFStruct.local_colormap);
-
   return NS_OK;
 }
 
diff --git a/modules/libpr0n/decoders/nsGIFDecoder2.h b/modules/libpr0n/decoders/nsGIFDecoder2.h
index 5e9938e5f86..647a8613576 100644
--- a/modules/libpr0n/decoders/nsGIFDecoder2.h
+++ b/modules/libpr0n/decoders/nsGIFDecoder2.h
@@ -63,7 +63,7 @@ public:
 
   virtual nsresult InitInternal();
   virtual nsresult WriteInternal(const char* aBuffer, PRUint32 aCount);
-  virtual nsresult ShutdownInternal(PRUint32 aFlags);
+  virtual nsresult FinishInternal();
 
 private:
   /* These functions will be called when the decoder has a decoded row,
diff --git a/modules/libpr0n/decoders/nsICODecoder.cpp b/modules/libpr0n/decoders/nsICODecoder.cpp
index a55571f84e8..338a58e483b 100644
--- a/modules/libpr0n/decoders/nsICODecoder.cpp
+++ b/modules/libpr0n/decoders/nsICODecoder.cpp
@@ -84,46 +84,6 @@ nsICODecoder::nsICODecoder()
 
 nsICODecoder::~nsICODecoder()
 {
-}
-
-nsresult
-nsICODecoder::InitInternal()
-{
-  // Fire OnStartDecode at init time to support bug 512435
-  if (!IsSizeDecode() && mObserver)
-    mObserver->OnStartDecode(nsnull);
-
-  return NS_OK;
-}
-
-nsresult
-nsICODecoder::ShutdownInternal(PRUint32 aFlags)
-{
-  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 && !(aFlags & CLOSE_FLAG_DONTNOTIFY) &&
-      (GetFrameCount() == 1)) {
-    // Tell the image that it's data has been updated 
-    nsIntRect r(0, 0, mDirEntry.mWidth, mDirEntry.mHeight);
-    rv = mImage->FrameUpdated(0, r);
-
-
-    if (mObserver) {
-      mObserver->OnDataAvailable(nsnull, PR_TRUE, &r);
-    }
-    PostFrameStop();
-    mImage->DecodingComplete();
-    if (mObserver) {
-      mObserver->OnStopContainer(nsnull, 0);
-      mObserver->OnStopDecode(nsnull, NS_OK, nsnull);
-    }
-  }
-
   mPos = 0;
 
   delete[] mColors;
@@ -139,6 +99,43 @@ nsICODecoder::ShutdownInternal(PRUint32 aFlags)
     mRow = nsnull;
   }
   mDecodingAndMask = PR_FALSE;
+}
+
+nsresult
+nsICODecoder::InitInternal()
+{
+  // Fire OnStartDecode at init time to support bug 512435
+  if (!IsSizeDecode() && mObserver)
+    mObserver->OnStartDecode(nsnull);
+
+  return NS_OK;
+}
+
+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)) {
+    // Tell the image that it's data has been updated 
+    nsIntRect r(0, 0, mDirEntry.mWidth, mDirEntry.mHeight);
+    rv = mImage->FrameUpdated(0, r);
+
+
+    if (mObserver) {
+      mObserver->OnDataAvailable(nsnull, PR_TRUE, &r);
+    }
+    PostFrameStop();
+    mImage->DecodingComplete();
+    if (mObserver) {
+      mObserver->OnStopContainer(nsnull, 0);
+      mObserver->OnStopDecode(nsnull, NS_OK, nsnull);
+    }
+  }
 
   return rv;
 }
diff --git a/modules/libpr0n/decoders/nsICODecoder.h b/modules/libpr0n/decoders/nsICODecoder.h
index b87a0a80144..702037d1752 100644
--- a/modules/libpr0n/decoders/nsICODecoder.h
+++ b/modules/libpr0n/decoders/nsICODecoder.h
@@ -78,7 +78,7 @@ public:
 
   virtual nsresult InitInternal();
   virtual nsresult WriteInternal(const char* aBuffer, PRUint32 aCount);
-  virtual nsresult ShutdownInternal(PRUint32 aFlags);
+  virtual nsresult FinishInternal();
 
 private:
   // Private helper methods
diff --git a/modules/libpr0n/decoders/nsIconDecoder.cpp b/modules/libpr0n/decoders/nsIconDecoder.cpp
index fa07112f179..f76e973a440 100644
--- a/modules/libpr0n/decoders/nsIconDecoder.cpp
+++ b/modules/libpr0n/decoders/nsIconDecoder.cpp
@@ -77,13 +77,11 @@ nsIconDecoder::InitInternal()
 }
 
 nsresult
-nsIconDecoder::ShutdownInternal(PRUint32 aFlags)
+nsIconDecoder::FinishInternal()
 {
   // If we haven't notified of completion yet for a full/success decode, we
   // didn't finish. Notify in error mode
-  if (!(aFlags & CLOSE_FLAG_DONTNOTIFY) &&
-      !IsSizeDecode() &&
-      !mNotifiedDone)
+  if (!IsSizeDecode() && !mNotifiedDone)
     NotifyDone(/* aSuccess = */ PR_FALSE);
 
   return NS_OK;
diff --git a/modules/libpr0n/decoders/nsIconDecoder.h b/modules/libpr0n/decoders/nsIconDecoder.h
index 4983d7601cf..b5e050dc824 100644
--- a/modules/libpr0n/decoders/nsIconDecoder.h
+++ b/modules/libpr0n/decoders/nsIconDecoder.h
@@ -79,7 +79,7 @@ public:
 
   virtual nsresult InitInternal();
   virtual nsresult WriteInternal(const char* aBuffer, PRUint32 aCount);
-  virtual nsresult ShutdownInternal(PRUint32 aFlags);
+  virtual nsresult FinishInternal();
 
   PRUint8 mWidth;
   PRUint8 mHeight;
diff --git a/modules/libpr0n/decoders/nsJPEGDecoder.cpp b/modules/libpr0n/decoders/nsJPEGDecoder.cpp
index ff8ad7f62f9..6649dd99443 100644
--- a/modules/libpr0n/decoders/nsJPEGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsJPEGDecoder.cpp
@@ -123,6 +123,10 @@ nsJPEGDecoder::nsJPEGDecoder()
 
 nsJPEGDecoder::~nsJPEGDecoder()
 {
+  // Step 8: Release JPEG decompression object
+  mInfo.src = nsnull;
+  jpeg_destroy_decompress(&mInfo);
+
   PR_FREEIF(mBackBuffer);
   if (mTransform)
     qcms_transform_release(mTransform);
@@ -176,11 +180,8 @@ nsJPEGDecoder::InitInternal()
 }
 
 nsresult
-nsJPEGDecoder::ShutdownInternal(PRUint32 aFlags)
+nsJPEGDecoder::FinishInternal()
 {
-  PR_LOG(gJPEGlog, PR_LOG_DEBUG,
-         ("[this=%p] nsJPEGDecoder::Close\n", this));
-
   /* If we're not in any sort of error case, flush the decoder.
    *
    * XXXbholley - It seems wrong that this should be necessary, but at the
@@ -189,15 +190,9 @@ nsJPEGDecoder::ShutdownInternal(PRUint32 aFlags)
    */
   if ((mState != JPEG_DONE && mState != JPEG_SINK_NON_JPEG_TRAILER) &&
       (mState != JPEG_ERROR) &&
-      !IsSizeDecode() &&
-      !(aFlags & CLOSE_FLAG_DONTNOTIFY))
+      !IsSizeDecode())
     this->Write(nsnull, 0);
 
-  /* Step 8: Release JPEG decompression object */
-  mInfo.src = nsnull;
-
-  jpeg_destroy_decompress(&mInfo);
-
   /* If we already know we're in an error state, don't
      bother flagging another one here. */
   if (mState == JPEG_ERROR)
@@ -205,9 +200,7 @@ nsJPEGDecoder::ShutdownInternal(PRUint32 aFlags)
 
   /* If we're doing a full decode and haven't notified of completion yet,
    * we must not have got everything we wanted. Send error notifications. */
-  if (!(aFlags & CLOSE_FLAG_DONTNOTIFY) &&
-      !IsSizeDecode() &&
-      !mNotifiedDone)
+  if (!IsSizeDecode() && !mNotifiedDone)
     NotifyDone(/* aSuccess = */ PR_FALSE);
 
   /* Otherwise, no problems. */
diff --git a/modules/libpr0n/decoders/nsJPEGDecoder.h b/modules/libpr0n/decoders/nsJPEGDecoder.h
index f40e3c95f23..9dbf4282dc1 100644
--- a/modules/libpr0n/decoders/nsJPEGDecoder.h
+++ b/modules/libpr0n/decoders/nsJPEGDecoder.h
@@ -91,7 +91,7 @@ public:
 
   virtual nsresult InitInternal();
   virtual nsresult WriteInternal(const char* aBuffer, PRUint32 aCount);
-  virtual nsresult ShutdownInternal(PRUint32 aFlags);
+  virtual nsresult FinishInternal();
 
   void NotifyDone(PRBool aSuccess);
 
diff --git a/modules/libpr0n/decoders/nsPNGDecoder.cpp b/modules/libpr0n/decoders/nsPNGDecoder.cpp
index 526ab958bd9..5550d92843d 100644
--- a/modules/libpr0n/decoders/nsPNGDecoder.cpp
+++ b/modules/libpr0n/decoders/nsPNGDecoder.cpp
@@ -93,6 +93,8 @@ nsPNGDecoder::nsPNGDecoder() :
 
 nsPNGDecoder::~nsPNGDecoder()
 {
+  if (mPNG)
+    png_destroy_read_struct(&mPNG, mInfo ? &mInfo : NULL, NULL);
   if (mCMSLine)
     nsMemory::Free(mCMSLine);
   if (interlacebuf)
@@ -294,16 +296,12 @@ nsPNGDecoder::InitInternal()
 }
 
 nsresult
-nsPNGDecoder::ShutdownInternal(PRUint32 aFlags)
+nsPNGDecoder::FinishInternal()
 {
-  if (mPNG)
-    png_destroy_read_struct(&mPNG, mInfo ? &mInfo : NULL, NULL);
 
   // If we're a full/success decode but haven't sent stop notifications yet,
   // we didn't get all the data we needed. Send error notifications.
-  if (!(aFlags & CLOSE_FLAG_DONTNOTIFY) &&
-      !IsSizeDecode() &&
-      !mNotifiedDone)
+  if (!IsSizeDecode() && !mNotifiedDone)
     NotifyDone(/* aSuccess = */ PR_FALSE);
 
   return NS_OK;
diff --git a/modules/libpr0n/decoders/nsPNGDecoder.h b/modules/libpr0n/decoders/nsPNGDecoder.h
index 3c3bf9a99d6..edc10592fb5 100644
--- a/modules/libpr0n/decoders/nsPNGDecoder.h
+++ b/modules/libpr0n/decoders/nsPNGDecoder.h
@@ -64,7 +64,7 @@ public:
 
   virtual nsresult InitInternal();
   virtual nsresult WriteInternal(const char* aBuffer, PRUint32 aCount);
-  virtual nsresult ShutdownInternal(PRUint32 aFlags);
+  virtual nsresult FinishInternal();
 
   void CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset,
                    PRInt32 width, PRInt32 height,
diff --git a/modules/libpr0n/src/Decoder.cpp b/modules/libpr0n/src/Decoder.cpp
index 8f3fc2e7550..8b47cb78bc3 100644
--- a/modules/libpr0n/src/Decoder.cpp
+++ b/modules/libpr0n/src/Decoder.cpp
@@ -60,7 +60,8 @@ Decoder::Init(imgIContainer *aImage,
 NS_IMETHODIMP
 Decoder::Close(PRUint32 aFlags)
 {
-  return Shutdown(aFlags);
+  NS_ABORT_IF_FALSE(0, "Not reached!");
+  return NS_OK;
 }
 
 NS_IMETHODIMP Decoder::Flush()
@@ -78,6 +79,7 @@ Decoder::Decoder()
 
 Decoder::~Decoder()
 {
+  NS_WARN_IF_FALSE(!mInFrame, "Shutting down decoder mid-frame!");
   mInitialized = false;
 }
 
@@ -91,6 +93,9 @@ Decoder::Init(RasterImage* aImage, imgIDecoderObserver* aObserver)
   // We should always have an image
   NS_ABORT_IF_FALSE(aImage, "Can't initialize decoder without an image!");
 
+  // No re-initializing
+  NS_ABORT_IF_FALSE(mImage == nsnull, "Can't re-initialize a decoder!");
+
   // Save our paremeters
   mImage = aImage;
   mObserver = aObserver;
@@ -116,20 +121,6 @@ Decoder::Finish()
   return FinishInternal();
 }
 
-nsresult
-Decoder::Shutdown(PRUint32 aFlags)
-{
-  // Implementation-specific shutdown
-  nsresult rv = ShutdownInternal(aFlags);
-
-  // Get rid of our strong references
-  mImage = nsnull;
-  mObserver = nsnull;
-
-  NS_ABORT_IF_FALSE(!mInFrame, "Shutting down mid-frame!");
-  return rv;
-}
-
 /*
  * Hook stubs. Override these as necessary in decoder implementations.
  */
@@ -137,7 +128,6 @@ Decoder::Shutdown(PRUint32 aFlags)
 nsresult Decoder::InitInternal() {return NS_OK; }
 nsresult Decoder::WriteInternal(const char* aBuffer, PRUint32 aCount) {return NS_OK; }
 nsresult Decoder::FinishInternal() {return NS_OK; }
-nsresult Decoder::ShutdownInternal(PRUint32 aFlags) {return NS_OK; }
 
 /*
  * Progress Notifications
diff --git a/modules/libpr0n/src/Decoder.h b/modules/libpr0n/src/Decoder.h
index 16bb41d4109..6d4c3272779 100644
--- a/modules/libpr0n/src/Decoder.h
+++ b/modules/libpr0n/src/Decoder.h
@@ -64,7 +64,7 @@ public:
    */
 
   /**
-   * Initialize an image decoder.
+   * Initialize an image decoder. Decoders may not be re-initialized.
    *
    * @param aContainer The image container to decode to.
    * @param aObserver The observer for decode notification events.
@@ -94,15 +94,6 @@ public:
    */
   nsresult Finish();
 
-  /**
-   * Shuts down the decoder.
-   *
-   * Notifications Sent: None
-   *
-   * XXX - These flags will go away later in the patch stack
-   */
-  nsresult Shutdown(PRUint32 aFlags);
-
   // We're not COM-y, so we don't get refcounts by default
   // XXX - This is uncommented in a later patch when we stop inheriting imgIDecoder
   // NS_INLINE_DECL_REFCOUNTING(Decoder)
@@ -134,7 +125,6 @@ protected:
   virtual nsresult InitInternal();
   virtual nsresult WriteInternal(const char* aBuffer, PRUint32 aCount);
   virtual nsresult FinishInternal();
-  virtual nsresult ShutdownInternal(PRUint32 aFlags);
 
   /*
    * Progress notifications.
diff --git a/modules/libpr0n/src/RasterImage.cpp b/modules/libpr0n/src/RasterImage.cpp
index 68e332cf236..537ffc8dbdd 100644
--- a/modules/libpr0n/src/RasterImage.cpp
+++ b/modules/libpr0n/src/RasterImage.cpp
@@ -2171,13 +2171,13 @@ RasterImage::ShutdownDecoder(eShutdownIntent aIntent)
   // Figure out what kind of decode we were doing before we get rid of our decoder
   bool wasSizeDecode = mDecoder->IsSizeDecode();
 
-  // Close the decoder with the appropriate flags
-  mInDecoder = PR_TRUE;
-  PRUint32 closeFlags = (aIntent == eShutdownIntent_Error)
-                          ? (PRUint32) imgIDecoder::CLOSE_FLAG_DONTNOTIFY
-                          : 0;
-  nsresult rv = mDecoder->Close(closeFlags);
-  mInDecoder = PR_FALSE;
+  // If we're not in error mode, finalize the decoder
+  nsresult rv = NS_OK;
+  if (aIntent != eShutdownIntent_Error) {
+    mInDecoder = PR_TRUE;
+    rv = mDecoder->Finish();
+    mInDecoder = PR_FALSE;
+  }
 
   // null out the decoder, _then_ check for errors on the close (otherwise the
   // error routine might re-invoke ShutdownDecoder)