Bug 1079628 - Record the number of bytes decoded on a per-decoder basis. r=tn
authorSeth Fowler <seth@mozilla.com>
Wed, 15 Oct 2014 13:52:20 -0700
changeset 210607 cbb710fac8064c79936b297889be4c3788ee00bd
parent 210606 71c4f958c4aae2113a8d5ead8fb002ed3f7a15ce
child 210608 a8e760faf784c78986c0cbebede0177fd361ad4c
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerstn
bugs1079628
milestone36.0a1
Bug 1079628 - Record the number of bytes decoded on a per-decoder basis. r=tn
image/src/Decoder.cpp
image/src/Decoder.h
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/Decoder.cpp
+++ b/image/src/Decoder.cpp
@@ -15,16 +15,17 @@ namespace mozilla {
 namespace image {
 
 Decoder::Decoder(RasterImage &aImage)
   : mImage(aImage)
   , mCurrentFrame(nullptr)
   , mImageData(nullptr)
   , mColormap(nullptr)
   , mDecodeFlags(0)
+  , mBytesDecoded(0)
   , mDecodeDone(false)
   , mDataError(false)
   , mFrameCount(0)
   , mFailCode(NS_OK)
   , mNeedsNewFrame(false)
   , mInitialized(false)
   , mSizeDecode(false)
   , mInFrame(false)
@@ -88,18 +89,21 @@ void
 Decoder::Write(const char* aBuffer, uint32_t aCount, DecodeStrategy aStrategy)
 {
   PROFILER_LABEL("ImageDecoder", "Write",
     js::ProfileEntry::Category::GRAPHICS);
 
   MOZ_ASSERT(NS_IsMainThread() || aStrategy == DECODE_ASYNC);
 
   // We're strict about decoder errors
-  NS_ABORT_IF_FALSE(!HasDecoderError(),
-                    "Not allowed to make more decoder calls after error!");
+  MOZ_ASSERT(!HasDecoderError(),
+             "Not allowed to make more decoder calls after error!");
+
+  // Keep track of the total number of bytes written.
+  mBytesDecoded += aCount;
 
   // If a data error occured, just ignore future data
   if (HasDataError())
     return;
 
   if (IsSizeDecode() && HasSize()) {
     // More data came in since we found the size. We have nothing to do here.
     return;
--- a/image/src/Decoder.h
+++ b/image/src/Decoder.h
@@ -96,16 +96,18 @@ public:
   }
 
   void SetObserver(imgDecoderObserver* aObserver)
   {
     MOZ_ASSERT(aObserver);
     mObserver = aObserver;
   }
 
+  size_t BytesDecoded() const { return mBytesDecoded; }
+
   // The number of frames we have, including anything in-progress. Thus, this
   // is only 0 if we haven't begun any frames.
   uint32_t GetFrameCount() { return mFrameCount; }
 
   // The number of complete frames we have (ie, not including anything in-progress).
   uint32_t GetCompleteFrameCount() { return mInFrame ? mFrameCount - 1 : mFrameCount; }
 
   // Error tracking
@@ -229,16 +231,17 @@ protected:
   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;
+  size_t mBytesDecoded;
   bool mDecodeDone;
   bool mDataError;
 
 private:
   uint32_t mFrameCount; // Number of frames, including anything in-progress
 
   nsIntRect mInvalidRect; // Tracks an invalidation region in the current frame.
 
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -57,17 +57,19 @@
 #endif
 
 namespace mozilla {
 
 using namespace gfx;
 using namespace layers;
 
 namespace image {
+
 using std::ceil;
+using std::min;
 
 // a mask for flags that will affect the decoding
 #define DECODE_FLAGS_MASK (imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA | imgIContainer::FLAG_DECODE_NO_COLORSPACE_CONVERSION)
 #define DECODE_FLAGS_DEFAULT 0
 
 static uint32_t
 DecodeFlags(uint32_t aFlags)
 {
@@ -314,17 +316,16 @@ RasterImage::RasterImage(imgStatusTracke
   mLockCount(0),
   mDecodeCount(0),
   mRequestedSampleSize(0),
 #ifdef DEBUG
   mFramesNotified(0),
 #endif
   mDecodingMonitor("RasterImage Decoding Monitor"),
   mDecoder(nullptr),
-  mBytesDecoded(0),
   mInDecoder(false),
   mStatusDiff(ImageStatusDiff::NoChange()),
   mNotifying(false),
   mHasSize(false),
   mDecodeOnDraw(false),
   mMultipart(false),
   mDiscardable(false),
   mHasSourceData(false),
@@ -1530,17 +1531,17 @@ RasterImage::AddSourceData(const char *a
   // be extra garbage data at the end of a file.
   if (mDecoded) {
     return NS_OK;
   }
 
   // Starting a new part's frames, let's clean up before we add any
   // This needs to happen just before we start getting EnsureFrame() call(s),
   // so that there's no gap for anything to miss us.
-  if (mMultipart && mBytesDecoded == 0) {
+  if (mMultipart && (!mDecoder || mDecoder->BytesDecoded() == 0)) {
     // Our previous state may have been animated, so let's clean up
     if (mAnimating)
       StopAnimation();
     mAnimationFinished = false;
     if (mAnim) {
       mAnim = nullptr;
     }
     // If there's only one frame, this could cause flickering
@@ -2090,18 +2091,16 @@ RasterImage::ShutdownDecoder(eShutdownIn
   }
 
   // If we finished a full decode, and we're not meant to be storing source
   // data, stop storing it.
   if (!wasSizeDecode && !StoringSourceData()) {
     mSourceData.Clear();
   }
 
-  mBytesDecoded = 0;
-
   return NS_OK;
 }
 
 // Writes the data to the decoder, updating the total number of bytes written.
 nsresult
 RasterImage::WriteToDecoder(const char *aBuffer, uint32_t aCount, DecodeStrategy aStrategy)
 {
   mDecodingMonitor.AssertCurrentThreadIn();
@@ -2112,20 +2111,16 @@ RasterImage::WriteToDecoder(const char *
   // Write
   nsRefPtr<Decoder> kungFuDeathGrip = mDecoder;
   mInDecoder = true;
   mDecoder->Write(aBuffer, aCount, aStrategy);
   mInDecoder = false;
 
   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;
 }
 
 // This function is called in situations where it's clear that we want the
 // frames in decoded form (Draw, LookupFrame, etc).  If we're completely decoded,
 // this method resets the discard timer (if we're discardable), since wanting
 // the frames now is a good indicator of wanting them again soon. If we're not
 // decoded, this method kicks off asynchronous decoding to generate the frames.
@@ -2247,28 +2242,29 @@ RasterImage::RequestDecodeCore(RequestDe
   // synchronous decode if we're already waiting on a full decode).
   if (mDecoded) {
     return NS_OK;
   }
 
   // If we've already got a full decoder running, and have already decoded
   // some bytes, we have nothing to do if we haven't been asked to do some
   // sync decoding
-  if (mDecoder && !mDecoder->IsSizeDecode() && mBytesDecoded &&
+  if (mDecoder && !mDecoder->IsSizeDecode() && mDecoder->BytesDecoded() > 0 &&
       aDecodeType != SYNCHRONOUS_NOTIFY_AND_SOME_DECODE) {
     return NS_OK;
   }
 
   ReentrantMonitorAutoEnter lock(mDecodingMonitor);
 
   // If we don't have any bytes to flush to the decoder, we can't do anything.
-  // mBytesDecoded can be bigger than mSourceData.Length() if we're not storing
-  // the source data.
-  if (mBytesDecoded > mSourceData.Length())
+  // mDecoder->BytesDecoded() can be bigger than mSourceData.Length() if we're
+  // not storing the source data.
+  if (mDecoder && mDecoder->BytesDecoded() > mSourceData.Length()) {
     return NS_OK;
+  }
 
   // After acquiring the lock we may have finished some more decoding, so
   // we need to repeat the following three checks after getting the lock.
 
   // If the image is waiting for decode work to be notified, go ahead and do that.
   if (mDecodeRequest &&
       mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_WORK_DONE &&
       aDecodeType != ASYNCHRONOUS) {
@@ -2280,18 +2276,18 @@ RasterImage::RequestDecodeCore(RequestDe
   // DecodeUntilSizeAvailable and FinishedSomeDecoding because they can result
   // in us finishing an in-progress decode (or kicking off and finishing a
   // synchronous decode if we're already waiting on a full decode).
   if (mDecoded) {
     return NS_OK;
   }
 
   // If we've already got a full decoder running, and have already
-  // decoded some bytes, we have nothing to do
-  if (mDecoder && !mDecoder->IsSizeDecode() && mBytesDecoded) {
+  // decoded some bytes, we have nothing to do.
+  if (mDecoder && !mDecoder->IsSizeDecode() && mDecoder->BytesDecoded() > 0) {
     return NS_OK;
   }
 
   // If we have a size decode open, interrupt it and shut it down; or if
   // the decoder has different flags than what we need
   if (mDecoder && mDecoder->GetDecodeFlags() != mFrameDecodeFlags) {
     nsresult rv = FinishedSomeDecoding(eShutdownIntent_NotNeeded);
     CONTAINER_ENSURE_SUCCESS(rv);
@@ -2299,23 +2295,24 @@ RasterImage::RequestDecodeCore(RequestDe
 
   // If we don't have a decoder, create one
   if (!mDecoder) {
     rv = InitDecoder(/* aDoSizeDecode = */ false);
     CONTAINER_ENSURE_SUCCESS(rv);
 
     rv = FinishedSomeDecoding();
     CONTAINER_ENSURE_SUCCESS(rv);
-
-    MOZ_ASSERT(mDecoder);
   }
 
+  MOZ_ASSERT(mDecoder);
+
   // If we've read all the data we have, we're done
-  if (mHasSourceData && mBytesDecoded == mSourceData.Length())
+  if (mHasSourceData && mDecoder->BytesDecoded() == mSourceData.Length()) {
     return NS_OK;
+  }
 
   // If we can do decoding now, do so.  Small images will decode completely,
   // large images will decode a bit and post themselves to the event loop
   // to finish decoding.
   if (!mDecoded && !mInDecoder && mHasSourceData && aDecodeType == SYNCHRONOUS_NOTIFY_AND_SOME_DECODE) {
     PROFILER_LABEL_PRINTF("RasterImage", "DecodeABitOf",
       js::ProfileEntry::Category::GRAPHICS, "%s", GetURIString().get());
 
@@ -2372,20 +2369,21 @@ RasterImage::SyncDecode()
   nsresult rv;
 
   // If we're decoded already, or decoding until the size was available
   // finished us as a side-effect, no worries
   if (mDecoded)
     return NS_OK;
 
   // If we don't have any bytes to flush to the decoder, we can't do anything.
-  // mBytesDecoded can be bigger than mSourceData.Length() if we're not storing
-  // the source data.
-  if (mBytesDecoded > mSourceData.Length())
+  // mDecoder->BytesDecoded() can be bigger than mSourceData.Length() if we're
+  // not storing the source data.
+  if (mDecoder && mDecoder->BytesDecoded() > mSourceData.Length()) {
     return NS_OK;
+  }
 
   // If we have a decoder open with different flags than what we need, shut it
   // down
   if (mDecoder && mDecoder->GetDecodeFlags() != mFrameDecodeFlags) {
     nsresult rv = FinishedSomeDecoding(eShutdownIntent_NotNeeded);
     CONTAINER_ENSURE_SUCCESS(rv);
 
     if (mDecoded) {
@@ -2406,18 +2404,21 @@ RasterImage::SyncDecode()
   }
 
   // If we don't have a decoder, create one
   if (!mDecoder) {
     rv = InitDecoder(/* aDoSizeDecode = */ false);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
+  MOZ_ASSERT(mDecoder);
+
   // Write everything we have
-  rv = DecodeSomeData(mSourceData.Length() - mBytesDecoded, DECODE_SYNC);
+  rv = DecodeSomeData(mSourceData.Length() - mDecoder->BytesDecoded(),
+                      DECODE_SYNC);
   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 = true;
@@ -2767,58 +2768,57 @@ RasterImage::RequestDiscard()
 
   return NS_OK;
 }
 
 // Flushes up to aMaxBytes to the decoder.
 nsresult
 RasterImage::DecodeSomeData(size_t aMaxBytes, DecodeStrategy aStrategy)
 {
-  // We should have a decoder if we get here
-  NS_ABORT_IF_FALSE(mDecoder, "trying to decode without decoder!");
+  MOZ_ASSERT(mDecoder, "Should have a decoder");
 
   mDecodingMonitor.AssertCurrentThreadIn();
 
   // First, if we've just been called because we allocated a frame on the main
   // thread, let the decoder deal with the data it set aside at that time by
   // passing it a null buffer.
   if (mDecodeRequest->mAllocatedNewFrame) {
     mDecodeRequest->mAllocatedNewFrame = false;
     nsresult rv = WriteToDecoder(nullptr, 0, aStrategy);
     if (NS_FAILED(rv) || mDecoder->NeedsNewFrame()) {
       return rv;
     }
   }
 
-  // If we have nothing else to decode, return
-  if (mBytesDecoded == mSourceData.Length())
+  // If we have nothing else to decode, return.
+  if (mDecoder->BytesDecoded() == mSourceData.Length()) {
     return NS_OK;
-
-  MOZ_ASSERT(mBytesDecoded < mSourceData.Length());
+  }
+
+  MOZ_ASSERT(mDecoder->BytesDecoded() < mSourceData.Length());
 
   // write the proper amount of data
-  size_t bytesToDecode = std::min(aMaxBytes,
-                                  mSourceData.Length() - mBytesDecoded);
-  nsresult rv = WriteToDecoder(mSourceData.Elements() + mBytesDecoded,
-                               bytesToDecode,
-                               aStrategy);
-
-  return rv;
+  size_t bytesToDecode = min(aMaxBytes,
+                             mSourceData.Length() - mDecoder->BytesDecoded());
+  return WriteToDecoder(mSourceData.Elements() + mDecoder->BytesDecoded(),
+                        bytesToDecode,
+                        aStrategy);
+
 }
 
 // There are various indicators that tell us we're finished with the decode
 // task at hand and can shut down the decoder.
 //
 // This method may not be called if there is no decoder.
 bool
 RasterImage::IsDecodeFinished()
 {
   // Precondition
   mDecodingMonitor.AssertCurrentThreadIn();
-  NS_ABORT_IF_FALSE(mDecoder, "Can't call IsDecodeFinished() without decoder!");
+  MOZ_ASSERT(mDecoder, "Should have a decoder");
 
   // The decode is complete if we got what we wanted.
   if (mDecoder->IsSizeDecode()) {
     if (mDecoder->HasSize()) {
       return true;
     }
   } else if (mDecoder->GetDecodeDone()) {
     return true;
@@ -2833,17 +2833,17 @@ RasterImage::IsDecodeFinished()
   }
 
   // Otherwise, if we have all the source data and wrote all the source data,
   // we're done.
   //
   // (NB - This can be the case even for non-erroneous images because
   // Decoder::GetDecodeDone() might not return true until after we call
   // Decoder::Finish() in ShutdownDecoder())
-  if (mHasSourceData && (mBytesDecoded == mSourceData.Length())) {
+  if (mHasSourceData && (mDecoder->BytesDecoded() == mSourceData.Length())) {
     return true;
   }
 
   // If we get here, assume it's not finished.
   return false;
 }
 
 // Indempotent error flagging routine. If a decoder is open, shuts it down.
@@ -3026,17 +3026,17 @@ RasterImage::FinishedSomeDecoding(eShutd
       if (request && !wasSize) {
         Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME,
                               int32_t(request->mDecodeTime.ToMicroseconds()));
 
         // We record the speed for only some decoders. The rest have
         // SpeedHistogram return HistogramCount.
         Telemetry::ID id = decoder->SpeedHistogram();
         if (id < Telemetry::HistogramCount) {
-          int32_t KBps = int32_t(request->mImage->mBytesDecoded /
+          int32_t KBps = int32_t(decoder->BytesDecoded() /
                                  (1024 * request->mDecodeTime.ToSeconds()));
           Telemetry::Accumulate(id, KBps);
         }
       }
 
       // We need to shut down the decoder first, in order to ensure all
       // decoding routines have been finished.
       rv = image->ShutdownDecoder(aIntent);
@@ -3202,17 +3202,18 @@ RasterImage::DecodePool::RequestDecode(R
   MOZ_ASSERT(aImg->mDecoder);
   aImg->mDecodingMonitor.AssertCurrentThreadIn();
 
   // If we're currently waiting on a new frame for this image, we can't do any
   // decoding.
   if (!aImg->mDecoder->NeedsNewFrame()) {
     // No matter whether this is currently being decoded, we need to update the
     // number of bytes we want it to decode.
-    aImg->mDecodeRequest->mBytesToDecode = aImg->mSourceData.Length() - aImg->mBytesDecoded;
+    aImg->mDecodeRequest->mBytesToDecode =
+      aImg->mSourceData.Length() - aImg->mDecoder->BytesDecoded();
 
     if (aImg->mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_PENDING ||
         aImg->mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_ACTIVE) {
       // The image is already in our list of images to decode, or currently being
       // decoded, so we don't have to do anything else.
       return;
     }
 
@@ -3250,17 +3251,17 @@ RasterImage::DecodePool::DecodeABitOf(Ra
   if (aImg->mDecoder && aImg->mDecoder->NeedsNewFrame()) {
     FrameNeededWorker::GetNewFrame(aImg);
   } else {
     // If we aren't yet finished decoding and we have more data in hand, add
     // this request to the back of the priority list.
     if (aImg->mDecoder &&
         !aImg->mError &&
         !aImg->IsDecodeFinished() &&
-        aImg->mSourceData.Length() > aImg->mBytesDecoded) {
+        aImg->mSourceData.Length() > aImg->mDecoder->BytesDecoded()) {
       RequestDecode(aImg);
     }
   }
 }
 
 /* static */ void
 RasterImage::DecodePool::StopDecoding(RasterImage* aImg)
 {
@@ -3294,29 +3295,29 @@ RasterImage::DecodePool::DecodeJob::Run(
   // still needs a new frame, we can't do anything. Wait until the
   // FrameNeededWorker enqueues another frame.
   if (mImage->mDecoder->NeedsNewFrame()) {
     return NS_OK;
   }
 
   mRequest->mRequestStatus = DecodeRequest::REQUEST_ACTIVE;
 
-  size_t oldByteCount = mImage->mBytesDecoded;
+  size_t oldByteCount = mImage->mDecoder->BytesDecoded();
 
   DecodeType type = DECODE_TYPE_UNTIL_DONE_BYTES;
 
   // Multithreaded decoding can be disabled. If we've done so, we don't want to
   // monopolize the main thread, and will allow a timeout in DecodeSomeOfImage.
   if (NS_IsMainThread()) {
     type = DECODE_TYPE_UNTIL_TIME;
   }
 
   DecodePool::Singleton()->DecodeSomeOfImage(mImage, DECODE_ASYNC, type, mRequest->mBytesToDecode);
 
-  size_t bytesDecoded = mImage->mBytesDecoded - oldByteCount;
+  size_t bytesDecoded = mImage->mDecoder->BytesDecoded() - oldByteCount;
 
   mRequest->mRequestStatus = DecodeRequest::REQUEST_WORK_DONE;
 
   // If the decoder needs a new frame, enqueue an event to get it; that event
   // will enqueue another decode request when it's done.
   if (mImage->mDecoder && mImage->mDecoder->NeedsNewFrame()) {
     FrameNeededWorker::GetNewFrame(mImage);
   }
@@ -3437,31 +3438,31 @@ RasterImage::DecodePool::DecodeSomeOfIma
   } else {
     // We're only guaranteed to decode this many bytes, so in particular,
     // gfxPrefs::ImageMemDecodeBytesAtATime should be set high enough for us
     // to read the size from most images.
     maxBytes = gfxPrefs::ImageMemDecodeBytesAtATime();
   }
 
   if (bytesToDecode == 0) {
-    bytesToDecode = aImg->mSourceData.Length() - aImg->mBytesDecoded;
+    bytesToDecode = aImg->mSourceData.Length() - aImg->mDecoder->BytesDecoded();
   }
 
   int32_t chunkCount = 0;
   TimeStamp start = TimeStamp::Now();
   TimeStamp deadline = start + TimeDuration::FromMilliseconds(gfxPrefs::ImageMemMaxMSBeforeYield());
 
   // We keep decoding chunks until:
   //  * we don't have any data left to decode,
   //  * the decode completes,
   //  * we're an UNTIL_SIZE decode and we get the size, or
   //  * we run out of time.
   // We also try to decode at least one "chunk" if we've allocated a new frame,
   // even if we have no more data to send to the decoder.
-  while ((aImg->mSourceData.Length() > aImg->mBytesDecoded &&
+  while ((aImg->mSourceData.Length() > aImg->mDecoder->BytesDecoded() &&
           bytesToDecode > 0 &&
           !aImg->IsDecodeFinished() &&
           !(aDecodeType == DECODE_TYPE_UNTIL_SIZE && aImg->mHasSize) &&
           !aImg->mDecoder->NeedsNewFrame()) ||
          (aImg->mDecodeRequest && aImg->mDecodeRequest->mAllocatedNewFrame)) {
     chunkCount++;
     uint32_t chunkSize = std::min(bytesToDecode, maxBytes);
     nsresult rv = aImg->DecodeSomeData(chunkSize, aStrategy);
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -651,17 +651,16 @@ private: // data
   // BEGIN LOCKED MEMBER VARIABLES
   ReentrantMonitor           mDecodingMonitor;
 
   FallibleTArray<char>       mSourceData;
 
   // Decoder and friends
   nsRefPtr<Decoder>          mDecoder;
   nsRefPtr<DecodeRequest>    mDecodeRequest;
-  size_t                     mBytesDecoded;
 
   bool                       mInDecoder;
   // END LOCKED MEMBER VARIABLES
 
   // Notification state. Used to avoid recursive notifications.
   ImageStatusDiff            mStatusDiff;
   bool                       mNotifying:1;