Bug 716140 - Make mImageData and mColormap (and their sizes) protected members of mozilla::image::Decoder rather than the leaf classes.

--HG--
extra : rebase_source : d4f506a669d5b18a33676434ad0cc512216956d5

image/decoders/nsBMPDecoder.cpp

@@ -40,7 +40,6 @@ nsBMPDecoder::nsBMPDecoder(RasterImage &aImage)
 {
   mColors = nullptr;
   mRow = nullptr;
-  mImageData = nullptr;
   mCurPos = mPos = mNumColors = mRowBytes = 0;
   mOldLine = mCurLine = 1; // Otherwise decoder will never start
   mState = eRLEStateInitial;
@@ -89,7 +88,7 @@ nsBMPDecoder::GetHeight() const
 uint32_t* 
 nsBMPDecoder::GetImageData() 
 {
-  return mImageData;
+  return reinterpret_cast<uint32_t*>(mImageData);
 }
 
 // Obtains the size of the compressed image resource
@@ -429,7 +428,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
                 if (rowSize == mRowBytes) {
                     // Collected a whole row into mRow, process it
                     uint8_t* p = mRow;
-                    uint32_t* d = mImageData + PIXEL_OFFSET(mCurLine, 0);
+                    uint32_t* d = reinterpret_cast<uint32_t*>(mImageData) + PIXEL_OFFSET(mCurLine, 0);
                     uint32_t lpos = mBIH.width;
                     switch (mBIH.bpp) {
                       case 1:
@@ -487,7 +486,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
                               // 4 has been right all along.  And we know it
                               // has been set to 0 the whole time, so that 
                               // means that everything is transparent so far.
-                              uint32_t* start = mImageData + GetWidth() * (mCurLine - 1);
+                              uint32_t* start = reinterpret_cast<uint32_t*>(mImageData) + GetWidth() * (mCurLine - 1);
                               uint32_t heightDifference = GetHeight() - mCurLine + 1;
                               uint32_t pixelCount = GetWidth() * heightDifference;
 
@@ -547,7 +546,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
                             mState = eRLEStateInitial;
                             uint32_t pixelsNeeded = std::min<uint32_t>(mBIH.width - mCurPos, mStateData);
                             if (pixelsNeeded) {
-                                uint32_t* d = mImageData + PIXEL_OFFSET(mCurLine, mCurPos);
+                                uint32_t* d = reinterpret_cast<uint32_t*>(mImageData) + PIXEL_OFFSET(mCurLine, mCurPos);
                                 mCurPos += pixelsNeeded;
                                 if (mBIH.compression == BI_RLE8) {
                                     do {
@@ -635,7 +634,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
                             // represents the number of pixels 
                             // that follow, each of which contains 
                             // the color index of a single pixel.
-                            uint32_t* d = mImageData + PIXEL_OFFSET(mCurLine, mCurPos);
+                            uint32_t* d = reinterpret_cast<uint32_t*>(mImageData) + PIXEL_OFFSET(mCurLine, mCurPos);
                             uint32_t* oldPos = d;
                             if (mBIH.compression == BI_RLE8) {
                                 while (aCount > 0 && mStateData > 0) {

image/decoders/nsBMPDecoder.h

@@ -67,7 +67,6 @@ private:
 
     bitFields mBitFields;
 
-    uint32_t *mImageData; ///< Pointer to the image data for the frame
     uint8_t *mRow;      ///< Holds one raw line of the image
     uint32_t mRowBytes; ///< How many bytes of the row were already received
     int32_t mCurLine;   ///< Index of the line of the image that's currently being decoded: [height,1]

image/decoders/nsGIFDecoder2.cpp

@@ -76,8 +76,6 @@ nsGIFDecoder2::nsGIFDecoder2(RasterImage &aImage)
   : Decoder(aImage)
   , mCurrentRow(-1)
   , mLastFlushedRow(-1)
-  , mImageData(nullptr)
-  , mColormap(nullptr)
   , mOldColor(0)
   , mCurrentFrame(-1)
   , mCurrentPass(0)

image/decoders/nsGIFDecoder2.h

@@ -49,9 +49,6 @@ private:
   int32_t mCurrentRow;
   int32_t mLastFlushedRow;
 
-  uint8_t *mImageData;       // Pointer to image data in either Cairo or 8bit format
-  uint32_t *mColormap;       // Current colormap to be used in Cairo format
-  uint32_t mColormapSize;
   uint32_t mOldColor;        // The old value of the transparent pixel
 
   // The frame number of the currently-decoding frame when we're in the middle

image/decoders/nsIconDecoder.cpp

@@ -21,8 +21,6 @@ nsIconDecoder::nsIconDecoder(RasterImage &aImage)
    mWidth(-1),
    mHeight(-1),
    mPixBytesRead(0),
-   mPixBytesTotal(0),
-   mImageData(nullptr),
    mState(iconStateStart)
 {
   // Nothing to do
@@ -41,10 +39,6 @@ nsIconDecoder::WriteInternal(const char *aBuffer, uint32_t aCount)
   uint32_t bytesToRead = 0;
   nsresult rv;
 
-  // Performance isn't critical here, so our update rectangle is 
-  // always the full icon
-  nsIntRect r(0, 0, mWidth, mHeight);
-
   // Loop until the input data is gone
   while (aCount > 0) {
     switch (mState) {
@@ -81,7 +75,7 @@ nsIconDecoder::WriteInternal(const char *aBuffer, uint32_t aCount)
         // Add the frame and signal
         rv = mImage.EnsureFrame(0, 0, 0, mWidth, mHeight,
                                 gfxASurface::ImageFormatARGB32,
-                                &mImageData, &mPixBytesTotal);
+                                &mImageData, &mImageDataLength);
         if (NS_FAILED(rv)) {
           PostDecoderError(rv);
           return;
@@ -97,13 +91,18 @@ nsIconDecoder::WriteInternal(const char *aBuffer, uint32_t aCount)
         break;
 
       case iconStateReadPixels:
+      {
 
         // How many bytes are we reading?
-        bytesToRead = std::min(aCount, mPixBytesTotal - mPixBytesRead);
+        bytesToRead = std::min(aCount, mImageDataLength - mPixBytesRead);
 
         // Copy the bytes
         memcpy(mImageData + mPixBytesRead, aBuffer, bytesToRead);
 
+        // Performance isn't critical here, so our update rectangle is
+        // always the full icon
+        nsIntRect r(0, 0, mWidth, mHeight);
+
         // Invalidate
         PostInvalidation(r);
 
@@ -113,12 +112,13 @@ nsIconDecoder::WriteInternal(const char *aBuffer, uint32_t aCount)
         mPixBytesRead += bytesToRead;
 
         // If we've got all the pixel bytes, we're finished
-        if (mPixBytesRead == mPixBytesTotal) {
+        if (mPixBytesRead == mImageDataLength) {
           PostFrameStop();
           PostDecodeDone();
           mState = iconStateFinished;
         }
         break;
+      }
 
       case iconStateFinished:
 

image/decoders/nsIconDecoder.h

@@ -46,8 +46,6 @@ public:
   uint8_t mWidth;
   uint8_t mHeight;
   uint32_t mPixBytesRead;
-  uint32_t mPixBytesTotal;
-  uint8_t* mImageData;
   uint32_t mState;
 };
 

image/decoders/nsJPEGDecoder.h

@@ -65,8 +65,6 @@ protected:
   void OutputScanlines(bool* suspend);
 
 public:
-  uint8_t *mImageData;
-
   struct jpeg_decompress_struct mInfo;
   struct jpeg_source_mgr mSourceMgr;
   decoder_error_mgr mErr;

image/decoders/nsPNGDecoder.h

@@ -72,7 +72,6 @@ public:
   nsIntRect mFrameRect;
   uint8_t *mCMSLine;
   uint8_t *interlacebuf;
-  uint8_t *mImageData;
   qcms_profile *mInProfile;
   qcms_transform *mTransform;
 

image/decoders/nsWBMPDecoder.cpp

@@ -70,7 +70,6 @@ nsWBMPDecoder::nsWBMPDecoder(RasterImage &aImage)
  : Decoder(aImage),
    mWidth(0),
    mHeight(0),
-   mImageData(nullptr),
    mRow(nullptr),
    mRowBytes(0),
    mCurLine(0),
@@ -175,11 +174,10 @@ nsWBMPDecoder::WriteInternal(const char *aBuffer, uint32_t aCount)
             return;
           }
 
-          uint32_t imageLength;
           // Add the frame and signal
           nsresult rv = mImage.EnsureFrame(0, 0, 0, mWidth, mHeight,
                                            gfxASurface::ImageFormatRGB24,
-                                           (uint8_t**)&mImageData, &imageLength);
+                                           (uint8_t**)&mImageData, &mImageDataLength);
 
           if (NS_FAILED(rv) || !mImageData) {
             PostDecoderError(NS_ERROR_FAILURE);
@@ -237,7 +235,7 @@ nsWBMPDecoder::WriteInternal(const char *aBuffer, uint32_t aCount)
           // If there is a filled buffered row of raw data, process the row.
           if (rowSize == mRowBytes) {
             uint8_t *p = mRow;
-            uint32_t *d = mImageData + (mWidth * mCurLine); // position of the first pixel at mCurLine
+            uint32_t *d = reinterpret_cast<uint32_t*>(mImageData) + (mWidth * mCurLine); // position of the first pixel at mCurLine
             uint32_t lpos = 0;
 
             while (lpos < mWidth) {

image/decoders/nsWBMPDecoder.h

@@ -49,8 +49,6 @@ private:
   uint32_t mWidth;
   uint32_t mHeight;
 
-  uint32_t *mImageData;
-
   uint8_t* mRow;                    // Holds one raw line of the image
   uint32_t mRowBytes;               // How many bytes of the row were already received
   uint32_t mCurLine;                // The current line being decoded (0 to mHeight - 1)

image/src/Decoder.cpp

@@ -15,6 +15,8 @@ namespace image {
 
 Decoder::Decoder(RasterImage &aImage)
   : mImage(aImage)
+  , mImageData(nullptr)
+  , mColormap(nullptr)
   , mDecodeFlags(0)
   , mDecodeDone(false)
   , mDataError(false)

image/src/Decoder.h

@@ -192,6 +192,11 @@ protected:
   RefPtr<imgDecoderObserver> mObserver;
   ImageMetadata mImageMetadata;
 
+  uint8_t* mImageData;       // Pointer to image data in either Cairo or 8bit format
+  uint32_t mImageDataLength;
+  uint32_t* mColormap;       // Current colormap to be used in Cairo format
+  uint32_t mColormapSize;
+
   uint32_t mDecodeFlags;
   bool mDecodeDone;
   bool mDataError;

image/src/RasterImage.cpp

@@ -1848,7 +1848,7 @@ RasterImage::DoImageDataComplete()
   // directly to the decoder in the AddSourceData() calls. This means we're
   // done, so we can shut down the decoder.
   if (!StoringSourceData()) {
-    RasterImage::FinishedSomeDecoding(this);
+    FinishedSomeDecoding();
   }
 
   // If there's a decoder open, synchronously decode the beginning of the image
@@ -2826,7 +2826,7 @@ RasterImage::RequestDecodeCore(RequestDecodeType aDecodeType)
   if (mDecoder &&
       (mDecoder->IsSizeDecode() || mDecoder->GetDecodeFlags() != mFrameDecodeFlags))
   {
-    RasterImage::FinishedSomeDecoding(this, eShutdownIntent_NotNeeded);
+    FinishedSomeDecoding(eShutdownIntent_NotNeeded);
   }
 
   // If we don't have a decoder, create one 
@@ -2885,7 +2885,7 @@ RasterImage::SyncDecode()
   if (mDecoder &&
       (mDecoder->IsSizeDecode() || mDecoder->GetDecodeFlags() != mFrameDecodeFlags))
   {
-    RasterImage::FinishedSomeDecoding(this, eShutdownIntent_NotNeeded);
+    FinishedSomeDecoding(eShutdownIntent_NotNeeded);
   }
 
   // If we don't have a decoder, create one 
@@ -2916,7 +2916,7 @@ RasterImage::SyncDecode()
     nsRefPtr<DecodeRequest> request = mDecodeRequest;
     nsresult rv = ShutdownDecoder(eShutdownIntent_Done);
     CONTAINER_ENSURE_SUCCESS(rv);
-    RasterImage::FinishedSomeDecoding(this, eShutdownIntent_Done, request);
+    FinishedSomeDecoding(eShutdownIntent_Done, request);
   }
 
   // All good if no errors!
@@ -3205,7 +3205,7 @@ RasterImage::UnlockImage()
     PR_LOG(GetCompressedImageAccountingLog(), PR_LOG_DEBUG,
            ("RasterImage[0x%p] canceling decode because image "
             "is now unlocked.", this));
-    RasterImage::FinishedSomeDecoding(this, eShutdownIntent_NotNeeded);
+    FinishedSomeDecoding(eShutdownIntent_NotNeeded);
     ForceDiscard();
     return NS_OK;
   }
@@ -3303,7 +3303,7 @@ RasterImage::DoError()
 
   // If we're mid-decode, shut down the decoder.
   if (mDecoder) {
-    FinishedSomeDecoding(this, eShutdownIntent_Error);
+    FinishedSomeDecoding(eShutdownIntent_Error);
   }
 
   // Put the container in an error state
@@ -3363,9 +3363,8 @@ RasterImage::GetFramesNotified(uint32_t *aFramesNotified)
 }
 #endif
 
-/* static */ void
+void
-RasterImage::FinishedSomeDecoding(RasterImage* aImage,
+RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_Done */,
-                                  eShutdownIntent aIntent /* = eShutdownIntent_Done */,
                                   DecodeRequest* aRequest /* = nullptr */)
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -3374,12 +3373,12 @@ RasterImage::FinishedSomeDecoding(RasterImage* aImage,
   if (aRequest) {
     request = aRequest;
   } else {
-    request = aImage->mDecodeRequest;
+    request = mDecodeRequest;
   }
 
   // Ensure that, if the decoder is the last reference to the image, we don't
   // destroy it by destroying the decoder.
-  nsRefPtr<RasterImage> image = aImage;
+  nsRefPtr<RasterImage> image(this);
 
   bool done = false;
 
@@ -3388,7 +3387,8 @@ RasterImage::FinishedSomeDecoding(RasterImage* aImage,
       Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, request->mChunkCount);
     }
 
-    // If the decode finished, tell the image and shut down the decoder.
+    // If the decode finished, or we're specifically being told to shut down,
+    // tell the image and shut down the decoder.
     if (image->mDecoder->GetDecodeDone() || image->IsDecodeFinished() ||
         aIntent != eShutdownIntent_Done) {
       done = true;
@@ -3420,7 +3420,7 @@ RasterImage::FinishedSomeDecoding(RasterImage* aImage,
     }
   }
 
-  // If it have any frames, tell the image what happened to the most recent
+  // If it has any frames, tell the image what happened to the most recent
   // one. This will be better when frames are published to decoders instead of
   // decoders requesting them.
   if (image->GetNumFrames()) {
@@ -3430,7 +3430,7 @@ RasterImage::FinishedSomeDecoding(RasterImage* aImage,
     } else {
       rect = image->CurrentStatusTracker().GetInvalidRect();
     }
-    aImage->FrameUpdated(image->GetNumFrames() - 1, rect);
+    image->FrameUpdated(image->GetNumFrames() - 1, rect);
   }
 
   // Then, tell the observers what happened in the decoder.
@@ -3461,14 +3461,14 @@ RasterImage::DecodeWorker::~DecodeWorker()
   // Shut down all the decoders since we're going away.
   DecodeRequest* request = mASAPDecodeRequests.getFirst();
   while (request) {
-    RasterImage::FinishedSomeDecoding(request->mImage, eShutdownIntent_NotNeeded);
+    request->mImage->FinishedSomeDecoding(eShutdownIntent_NotNeeded);
 
     request = request->getNext();
   }
 
   request = mNormalDecodeRequests.getFirst();
   while (request) {
-    RasterImage::FinishedSomeDecoding(request->mImage, eShutdownIntent_NotNeeded);
+    request->mImage->FinishedSomeDecoding(eShutdownIntent_NotNeeded);
 
     request = request->getNext();
   }
@@ -3544,7 +3544,7 @@ RasterImage::DecodeWorker::DecodeABitOf(RasterImage* aImg)
   MOZ_ASSERT(NS_IsMainThread());
 
   DecodeSomeOfImage(aImg);
-  RasterImage::FinishedSomeDecoding(aImg);
+  aImg->FinishedSomeDecoding();
 
   // If we aren't yet finished decoding and we have more data in hand, add
   // this request to the back of the priority list.
@@ -3617,11 +3617,11 @@ RasterImage::DecodeWorker::Run()
 
       // If we have a new frame, let everybody know about it.
       if (image->mDecoder->GetFrameCount() != oldCount) {
-        DecodeDoneWorker::DidSomeDecoding(image, request);
+        DecodeDoneWorker::NotifyFinishedSomeDecoding(image, request);
       }
     } else {
       // Nothing more for us to do - let everyone know what happened.
-      DecodeDoneWorker::DidSomeDecoding(image, request);
+      DecodeDoneWorker::NotifyFinishedSomeDecoding(image, request);
     }
 
   } while ((TimeStamp::Now() - eventStart).ToMilliseconds() <= gMaxMSBeforeYield);
@@ -3644,7 +3644,7 @@ RasterImage::DecodeWorker::DecodeUntilSizeAvailable(RasterImage* aImg)
 
   nsresult rv = DecodeSomeOfImage(aImg, DECODE_TYPE_UNTIL_SIZE);
 
-  RasterImage::FinishedSomeDecoding(aImg);
+  aImg->FinishedSomeDecoding();
 
   return rv;
 }
@@ -3753,7 +3753,7 @@ RasterImage::DecodeDoneWorker::DecodeDoneWorker(RasterImage* image, DecodeReques
 {}
 
 void
-RasterImage::DecodeDoneWorker::DidSomeDecoding(RasterImage* image, DecodeRequest* request)
+RasterImage::DecodeDoneWorker::NotifyFinishedSomeDecoding(RasterImage* image, DecodeRequest* request)
 {
   nsCOMPtr<DecodeDoneWorker> worker = new DecodeDoneWorker(image, request);
   NS_DispatchToMainThread(worker);
@@ -3762,7 +3762,7 @@ RasterImage::DecodeDoneWorker::DidSomeDecoding(RasterImage* image, DecodeRequest
 NS_IMETHODIMP
 RasterImage::DecodeDoneWorker::Run()
 {
-  RasterImage::FinishedSomeDecoding(mImage, eShutdownIntent_Done, mRequest);
+  mImage->FinishedSomeDecoding(eShutdownIntent_Done, mRequest);
 
   // If we didn't finish decoding yet, try again
   if (mImage->mDecoder && !mImage->IsDecodeFinished() &&

image/src/RasterImage.h

@@ -533,7 +533,7 @@ private:
      * Ensures the decode state accumulated by the decoding process gets
      * applied to the image.
      */
-    static void DidSomeDecoding(RasterImage* image, DecodeRequest* request);
+    static void NotifyFinishedSomeDecoding(RasterImage* image, DecodeRequest* request);
 
     NS_IMETHOD Run();
 
@@ -546,9 +546,8 @@ private:
     nsRefPtr<DecodeRequest> mRequest;
   };
 
-  static void FinishedSomeDecoding(RasterImage* image,
+  void FinishedSomeDecoding(eShutdownIntent intent = eShutdownIntent_Done,
-                                   eShutdownIntent intent = eShutdownIntent_Done,
+                            DecodeRequest* request = nullptr);
-                                   DecodeRequest* request = nullptr);
 
   void DrawWithPreDownscaleIfNeeded(imgFrame *aFrame,
                                     gfxContext *aContext,

image/src/imgStatusTracker.cpp

@@ -526,14 +526,12 @@ imgStatusTracker::SyncAndSyncNotifyDifference(imgStatusTracker* other)
   mImageStatus = other->mImageStatus;
   mIsMultipart = other->mIsMultipart;
   mHadLastPart = other->mHadLastPart;
+  mImageStatus |= other->mImageStatus;
 
   // The error state is sticky and overrides all other bits.
-  if (mImageStatus == imgIRequest::STATUS_ERROR ||
+  if (mImageStatus & imgIRequest::STATUS_ERROR) {
-      other->mImageStatus == imgIRequest::STATUS_ERROR) {
     mImageStatus = imgIRequest::STATUS_ERROR;
   } else {
-    mImageStatus |= other->mImageStatus;
-
     // Unset the bits that can get unset as part of the decoding process.
     if (!(other->mImageStatus & imgIRequest::STATUS_DECODE_STARTED)) {
       mImageStatus &= ~imgIRequest::STATUS_DECODE_STARTED;