Bug 587371 - Image.h PRUint32 getters shouldn't be COM-y. r=joe,a=blocker
authorBobby Holley <bobbyholley@gmail.com>
Sat, 14 Aug 2010 18:23:47 -0400
changeset 51190 3aeb7495b4f15b892162932c65f22fb4daf0364f
parent 51189 7886feb71709c5b17bf74f1f6fcf15c0e1e5499b
child 51191 ba16605752098bfb2805bf1f26bfe6f2b8973c63
push idunknown
push userunknown
push dateunknown
reviewersjoe, blocker
bugs587371
milestone2.0b5pre
Bug 587371 - Image.h PRUint32 getters shouldn't be COM-y. r=joe,a=blocker
modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp
modules/libpr0n/decoders/png/nsPNGDecoder.cpp
modules/libpr0n/src/Image.h
modules/libpr0n/src/RasterImage.cpp
modules/libpr0n/src/RasterImage.h
modules/libpr0n/src/imgRequest.cpp
modules/libpr0n/src/imgStatusTracker.cpp
--- a/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp
+++ b/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp
@@ -207,18 +207,17 @@ nsGIFDecoder2::FlushImageData(PRUint32 f
   nsresult rv = mImageContainer->FrameUpdated(mGIFStruct.images_decoded, r);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   // Offset to the frame position
   // Only notify observer(s) for first frame
   if (!mGIFStruct.images_decoded && mObserver) {
-    PRUint32 imgCurFrame;
-    mImageContainer->GetCurrentFrameIndex(&imgCurFrame);
+    PRUint32 imgCurFrame = mImageContainer->GetCurrentFrameIndex();
     mObserver->OnDataAvailable(nsnull, imgCurFrame == PRUint32(mGIFStruct.images_decoded), &r);
   }
   return NS_OK;
 }
 
 nsresult
 nsGIFDecoder2::FlushImageData()
 {
@@ -263,25 +262,21 @@ nsGIFDecoder2::Write(const char *aBuffer
 
   // 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.
   if (NS_FAILED(rv)) {
 
-    // Determine if we want to salvage the situation
-    PRUint32 numFrames = 0;
-    if (mImageContainer)
-      mImageContainer->GetNumFrames(&numFrames);
-
-    // If we're salvaging, send off notifications
+    // 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 (numFrames > 1) {
+    if (mImageContainer && mImageContainer->GetNumFrames() > 1) {
       EndGIF(/* aSuccess = */ PR_TRUE);
     }
 
     // Otherwise, set mError
     else
       mError = PR_TRUE;
   }
 
@@ -335,18 +330,17 @@ nsresult nsGIFDecoder2::BeginImageFrame(
 {
   if (!mGIFStruct.images_decoded) {
     // Send a onetime OnDataAvailable (Display Refresh) for the first frame
     // if it has a y-axis offset.  Otherwise, the area may never be refreshed
     // and the placeholder will remain on the screen. (Bug 37589)
     if (mGIFStruct.y_offset > 0) {
       PRInt32 imgWidth;
       mImageContainer->GetWidth(&imgWidth);
-      PRUint32 imgCurFrame;
-      mImageContainer->GetCurrentFrameIndex(&imgCurFrame);
+      PRUint32 imgCurFrame = mImageContainer->GetCurrentFrameIndex();
       nsIntRect r(0, 0, imgWidth, mGIFStruct.y_offset);
       if (mObserver)
         mObserver->OnDataAvailable(nsnull,
                                    imgCurFrame == PRUint32(mGIFStruct.images_decoded),
                                    &r);
     }
   }
 
@@ -395,18 +389,17 @@ void nsGIFDecoder2::EndImageFrame()
     // Only need to flush first frame
     (void) FlushImageData();
 
     // If the first frame is smaller in height than the entire image, send a
     // OnDataAvailable (Display Refresh) for the area it does not have data for.
     // This will clear the remaining bits of the placeholder. (Bug 37589)
     const PRUint32 realFrameHeight = mGIFStruct.height + mGIFStruct.y_offset;
     if (realFrameHeight < mGIFStruct.screen_height) {
-      PRUint32 imgCurFrame;
-      mImageContainer->GetCurrentFrameIndex(&imgCurFrame);
+      PRUint32 imgCurFrame = mImageContainer->GetCurrentFrameIndex();
       nsIntRect r(0, realFrameHeight,
                   mGIFStruct.screen_width,
                   mGIFStruct.screen_height - realFrameHeight);
       if (mObserver)
         mObserver->OnDataAvailable(nsnull,
                                   imgCurFrame == PRUint32(mGIFStruct.images_decoded),
                                   &r);
     }
--- a/modules/libpr0n/decoders/png/nsPNGDecoder.cpp
+++ b/modules/libpr0n/decoders/png/nsPNGDecoder.cpp
@@ -140,21 +140,18 @@ void nsPNGDecoder::CreateFrame(png_uint_
   mFrameRect.width = width;
   mFrameRect.height = height;
 
 #ifdef PNG_APNG_SUPPORTED
   if (png_get_valid(mPNG, mInfo, PNG_INFO_acTL))
     SetAnimFrameInfo();
 #endif
 
-  PRUint32 numFrames = 0;
-  mImage->GetNumFrames(&numFrames);
-
   if (mObserver)
-    mObserver->OnStartFrame(nsnull, numFrames - 1);
+    mObserver->OnStartFrame(nsnull, mImage->GetNumFrames() - 1);
 
   PR_LOG(gPNGDecoderAccountingLog, PR_LOG_DEBUG,
          ("PNGDecoderAccounting: nsPNGDecoder::CreateFrame -- created "
           "image frame with %dx%d pixels in container %p",
           width, height,
           mImage.get ()));
 
   mFrameHasNoAlpha = PR_TRUE;
@@ -182,18 +179,17 @@ void nsPNGDecoder::SetAnimFrameInfo()
       delay_den = 100; // so says the APNG spec
 
     // Need to cast delay_num to float to have a proper division and
     // the result to int to avoid compiler warning
     timeout = static_cast<PRInt32>
               (static_cast<PRFloat64>(delay_num) * 1000 / delay_den);
   }
 
-  PRUint32 numFrames = 0;
-  mImage->GetNumFrames(&numFrames);
+  PRUint32 numFrames = mImage->GetNumFrames();
 
   mImage->SetFrameTimeout(numFrames - 1, timeout);
 
   if (dispose_op == PNG_DISPOSE_OP_PREVIOUS)
       mImage->SetFrameDisposalMethod(numFrames - 1,
                                      RasterImage::kDisposeRestorePrevious);
   else if (dispose_op == PNG_DISPOSE_OP_BACKGROUND)
       mImage->SetFrameDisposalMethod(numFrames - 1,
@@ -209,30 +205,29 @@ void nsPNGDecoder::SetAnimFrameInfo()
 }
 #endif
 
 // set timeout and frame disposal method for the current frame
 void nsPNGDecoder::EndImageFrame()
 {
   PRUint32 numFrames = 1;
 #ifdef PNG_APNG_SUPPORTED
-  mImage->GetNumFrames(&numFrames);
+  numFrames = mImage->GetNumFrames();
 
   // We can't use mPNG->num_frames_read as it may be one ahead.
   if (numFrames > 1) {
     // Tell the image renderer that the frame is complete
     if (mFrameHasNoAlpha)
       mImage->SetFrameHasNoAlpha(numFrames - 1);
 
     if (NS_FAILED(mImage->FrameUpdated(numFrames - 1, mFrameRect))) {
       mError = PR_TRUE;
       // allow the call out to the observers.
     }
-    PRUint32 curFrame;
-    mImage->GetCurrentFrameIndex(&curFrame);
+    PRUint32 curFrame = mImage->GetCurrentFrameIndex();
     if (mObserver)
       mObserver->OnDataAvailable(nsnull, curFrame == numFrames - 1,
                                  &mFrameRect);
   }
 #endif
 
   mImage->EndFrameDecode(numFrames - 1);
   if (mObserver)
@@ -856,27 +851,25 @@ row_callback(png_structp png_ptr, png_by
         NS_ERROR("Unknown PNG format!");
         NS_ABORT();
         break;
     }
 
     if (!rowHasNoAlpha)
       decoder->mFrameHasNoAlpha = PR_FALSE;
 
-    PRUint32 numFrames = 0;
-    decoder->mImage->GetNumFrames(&numFrames);
+    PRUint32 numFrames = decoder->mImage->GetNumFrames();
     if (numFrames <= 1) {
       // Only do incremental image display for the first frame
       nsIntRect r(0, row_num, width, 1);
       if (NS_FAILED(decoder->mImage->FrameUpdated(numFrames - 1, r))) {
         decoder->mError = PR_TRUE;  /* bail */
         return;
       }
-      PRUint32 curFrame;
-      decoder->mImage->GetCurrentFrameIndex(&curFrame);
+      PRUint32 curFrame = decoder->mImage->GetCurrentFrameIndex();
       if (decoder->mObserver)
         decoder->mObserver->OnDataAvailable(nsnull,
                                             curFrame == numFrames - 1, &r);
     }
   }
 }
 
 // got the header of a new frame that's coming
--- a/modules/libpr0n/src/Image.h
+++ b/modules/libpr0n/src/Image.h
@@ -89,28 +89,28 @@ public:
    * conflicts with native types in xpidl.
    */
   virtual nsresult GetCurrentFrameRect(nsIntRect& aRect) = 0;
 
   /**
    * The index of the current frame that would be drawn if the image was to be
    * drawn now.
    */
-  virtual nsresult GetCurrentFrameIndex(PRUint32* aCurrentFrameIdx) = 0;
+  virtual PRUint32 GetCurrentFrameIndex() = 0;
 
   /**
    * The total number of frames in this image.
    */
-  virtual nsresult GetNumFrames(PRUint32* aNumFrames) = 0;
+  virtual PRUint32 GetNumFrames() = 0;
 
   /**
    * The size, in bytes, occupied by the significant data portions of the image.
    * This includes both compressed source data and decoded frames.
    */
-  virtual nsresult GetDataSize(PRUint32* aDataSize) = 0;
+  virtual PRUint32 GetDataSize() = 0;
 
 protected:
   Image();
 
   // Member data shared by all implementations of this abstract class
   imgStatusTracker   mStatusTracker;
   PRPackedBool       mInitialized;   // Have we been initalized?
 };
--- a/modules/libpr0n/src/RasterImage.cpp
+++ b/modules/libpr0n/src/RasterImage.cpp
@@ -486,40 +486,26 @@ RasterImage::GetCurrentFrameRect(nsIntRe
   else {
     aRect.MoveTo(0, 0);
     aRect.SizeTo(0, 0);
   }
 
   return NS_OK;
 }
 
-nsresult
-RasterImage::GetCurrentFrameIndex(PRUint32 *aCurrentFrameIdx)
+PRUint32
+RasterImage::GetCurrentFrameIndex()
 {
-  if (mError)
-    return NS_ERROR_FAILURE;
-
-  NS_ENSURE_ARG_POINTER(aCurrentFrameIdx);
-  
-  *aCurrentFrameIdx = GetCurrentImgFrameIndex();
-
-  return NS_OK;
+  return GetCurrentImgFrameIndex();
 }
 
-nsresult
-RasterImage::GetNumFrames(PRUint32 *aNumFrames)
+PRUint32
+RasterImage::GetNumFrames()
 {
-  if (mError)
-    return NS_ERROR_FAILURE;
-
-  NS_ENSURE_ARG_POINTER(aNumFrames);
-
-  *aNumFrames = mFrames.Length();
-  
-  return NS_OK;
+  return mFrames.Length();
 }
 
 //******************************************************************************
 /* readonly attribute boolean animated; */
 NS_IMETHODIMP
 RasterImage::GetAnimated(PRBool *aAnimated)
 {
   if (mError)
@@ -658,35 +644,34 @@ RasterImage::GetFrame(PRUint32 aWhichFra
     framesurf = imgsurf;
   }
 
   *_retval = framesurf.forget().get();
 
   return rv;
 }
 
-nsresult
-RasterImage::GetDataSize(PRUint32 *_retval)
+PRUint32
+RasterImage::GetDataSize()
 {
   if (mError)
-    return NS_ERROR_FAILURE;
-
-  NS_ENSURE_ARG_POINTER(_retval);
+    return 0;
 
   // Start with 0
-  *_retval = 0;
+  PRUint32 size = 0;
 
   // Account for any compressed source data
-  *_retval += GetSourceDataSize();
-  NS_ABORT_IF_FALSE(StoringSourceData() || (*_retval == 0),
+  size += GetSourceDataSize();
+  NS_ABORT_IF_FALSE(StoringSourceData() || (size == 0),
                     "Non-zero source data size when we aren't storing it?");
 
   // Account for any uncompressed frames
-  *_retval += GetDecodedDataSize();
-  return NS_OK;
+  size += GetDecodedDataSize();
+
+  return size;
 }
 
 PRUint32
 RasterImage::GetDecodedDataSize()
 {
   PRUint32 val = 0;
   for (PRUint32 i = 0; i < mFrames.Length(); ++i) {
     imgFrame *frame = mFrames.SafeElementAt(i, nsnull);
--- a/modules/libpr0n/src/RasterImage.h
+++ b/modules/libpr0n/src/RasterImage.h
@@ -161,19 +161,19 @@ public:
   // C++-only version of imgIContainer::GetType, for convenience
   virtual PRUint16 GetType() { return imgIContainer::TYPE_RASTER; }
 
   // Methods inherited from Image
   nsresult Init(imgIDecoderObserver *aObserver,
                 const char* aMimeType,
                 PRUint32 aFlags);
   nsresult GetCurrentFrameRect(nsIntRect& aRect);
-  nsresult GetCurrentFrameIndex(PRUint32* aCurrentFrameIdx);
-  nsresult GetNumFrames(PRUint32* aNumFrames);
-  nsresult GetDataSize(PRUint32* aDataSize);
+  PRUint32 GetCurrentFrameIndex();
+  PRUint32 GetNumFrames();
+  PRUint32 GetDataSize();
 
   // Raster-specific methods
   static NS_METHOD WriteToRasterImage(nsIInputStream* aIn, void* aClosure,
                                       const char* aFromRawSegment,
                                       PRUint32 aToOffset, PRUint32 aCount,
                                       PRUint32* aWriteCount);
 
   PRUint32 GetDecodedDataSize();
--- a/modules/libpr0n/src/imgRequest.cpp
+++ b/modules/libpr0n/src/imgRequest.cpp
@@ -477,19 +477,17 @@ void imgRequest::SetIsInCache(PRBool inc
 {
   LOG_FUNC_WITH_PARAM(gImgLog, "imgRequest::SetIsCacheable", "incache", incache);
   mIsInCache = incache;
 }
 
 void imgRequest::UpdateCacheEntrySize()
 {
   if (mCacheEntry) {
-    PRUint32 imageSize = 0;
-    mImage->GetDataSize(&imageSize);
-    mCacheEntry->SetDataSize(imageSize);
+    mCacheEntry->SetDataSize(mImage->GetDataSize());
 
 #ifdef DEBUG_joe
     nsCAutoString url;
     mURI->GetSpec(url);
     printf("CACHEPUT: %d %s %d\n", time(NULL), url.get(), imageSize);
 #endif
   }
 }
--- a/modules/libpr0n/src/imgStatusTracker.cpp
+++ b/modules/libpr0n/src/imgStatusTracker.cpp
@@ -218,22 +218,18 @@ imgStatusTracker::SyncNotify(imgRequestP
   if (mState & stateHasSize)
     proxy->OnStartContainer(mImage);
 
   // OnStartDecode
   if (mState & stateDecodeStarted)
     proxy->OnStartDecode();
 
   // Send frame messages (OnStartFrame, OnDataAvailable, OnStopFrame)
-  PRUint32 nframes = 0;
-  mImage->GetNumFrames(&nframes);
-
-  if (nframes > 0) {
-    PRUint32 frame;
-    mImage->GetCurrentFrameIndex(&frame);
+  if (mImage->GetNumFrames() > 0) {
+    PRUint32 frame = mImage->GetCurrentFrameIndex();
     proxy->OnStartFrame(frame);
 
     // OnDataAvailable
     // XXX - Should only send partial rects here, but that needs to
     // wait until we fix up the observer interface
     nsIntRect r;
     mImage->GetCurrentFrameRect(r);
     proxy->OnDataAvailable(frame, &r);