Bug 1079653 (Part 3) - Make decoders track whether they need to flush data after getting a new frame. r=tn
☠☠ backed out by a13927f78353 ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Wed, 15 Oct 2014 13:52:21 -0700
changeset 210610 dfb0890b02be6dc35a3b76ddb1d5c31a3a9a888a
parent 210609 546f90c14465afd29b88fe16ff27bf5678b285d5
child 210611 59d1754eb01fd84ec139d686fc7b2f79079f17c1
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerstn
bugs1079653
milestone36.0a1
Bug 1079653 (Part 3) - Make decoders track whether they need to flush data after getting a new frame. 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
@@ -22,16 +22,17 @@ Decoder::Decoder(RasterImage &aImage)
   , mChunkCount(0)
   , mDecodeFlags(0)
   , mBytesDecoded(0)
   , mDecodeDone(false)
   , mDataError(false)
   , mFrameCount(0)
   , mFailCode(NS_OK)
   , mNeedsNewFrame(false)
+  , mNeedsToFlushData(false)
   , mInitialized(false)
   , mSizeDecode(false)
   , mInFrame(false)
   , mIsAnimated(false)
 {
 }
 
 Decoder::~Decoder()
@@ -100,16 +101,22 @@ Decoder::Write(const char* aBuffer, uint
 
   // Begin recording telemetry data.
   TimeStamp start = TimeStamp::Now();
   mChunkCount++;
 
   // Keep track of the total number of bytes written.
   mBytesDecoded += aCount;
 
+  // If we're flushing data, clear the flag.
+  if (aBuffer == nullptr && aCount == 0) {
+    MOZ_ASSERT(mNeedsToFlushData, "Flushing when we don't need to");
+    mNeedsToFlushData = false;
+  }
+
   // 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;
   }
@@ -245,16 +252,22 @@ Decoder::AllocateFrame()
   } else if (NS_FAILED(rv)) {
     PostDataError();
   }
 
   // Mark ourselves as not needing another frame before talking to anyone else
   // so they can tell us if they need yet another.
   mNeedsNewFrame = false;
 
+  // If we've received any data at all, we may have pending data that needs to
+  // be flushed now that we have a frame to decode into.
+  if (mBytesDecoded > 0) {
+    mNeedsToFlushData = true;
+  }
+
   return rv;
 }
 
 void
 Decoder::FlushInvalidations()
 {
   NS_ABORT_IF_FALSE(!HasDecoderError(),
                     "Not allowed to make more decoder calls after error!");
--- a/image/src/Decoder.h
+++ b/image/src/Decoder.h
@@ -39,16 +39,21 @@ public:
    */
   void InitSharedDecoder(uint8_t* imageData, uint32_t imageDataLength,
                          uint32_t* colormap, uint32_t colormapSize,
                          imgFrame* currentFrame);
 
   /**
    * Writes data to the decoder.
    *
+   * If aBuffer is null and aCount is 0, Write() flushes any buffered data to
+   * the decoder. Data is buffered if the decoder wasn't able to completely
+   * decode it because it needed a new frame.  If it's necessary to flush data,
+   * NeedsToFlushData() will return true.
+   *
    * @param aBuffer buffer containing the data to be written
    * @param aCount the number of bytes to write
    *
    * Any errors are reported by setting the appropriate state on the decoder.
    *
    * Notifications Sent: TODO
    */
   void Write(const char* aBuffer, uint32_t aCount, DecodeStrategy aStrategy);
@@ -159,16 +164,21 @@ public:
   // will be called again with nullptr and 0 as arguments.
   void NeedNewFrame(uint32_t frameNum, uint32_t x_offset, uint32_t y_offset,
                     uint32_t width, uint32_t height,
                     gfx::SurfaceFormat format,
                     uint8_t palette_depth = 0);
 
   virtual bool NeedsNewFrame() const { return mNeedsNewFrame; }
 
+  // Returns true if we may have stored data that we need to flush now that we
+  // have a new frame to decode into. Callers can use Write() to actually
+  // flush the data; see the documentation for that method.
+  bool NeedsToFlushData() const { return mNeedsToFlushData; }
+
   // Try to allocate a frame as described in mNewFrameData and return the
   // status code from that attempt. Clears mNewFrameData.
   virtual nsresult AllocateFrame();
 
   already_AddRefed<imgFrame> GetCurrentFrame() const
   {
     nsRefPtr<imgFrame> frame = mCurrentFrame;
     return frame.forget();
@@ -278,16 +288,17 @@ private:
     uint32_t mOffsetY;
     uint32_t mWidth;
     uint32_t mHeight;
     gfx::SurfaceFormat mFormat;
     uint8_t mPaletteDepth;
   };
   NewFrameData mNewFrameData;
   bool mNeedsNewFrame;
+  bool mNeedsToFlushData;
   bool mInitialized;
   bool mSizeDecode;
   bool mInFrame;
   bool mIsAnimated;
 };
 
 } // namespace image
 } // namespace mozilla
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -2391,21 +2391,19 @@ RasterImage::SyncDecode()
       // with the new flags. If we can't discard then there isn't
       // anything we can do.
       if (!CanForciblyDiscardAndRedecode())
         return NS_ERROR_NOT_AVAILABLE;
       ForceDiscard();
     }
   }
 
-  // If we're currently waiting on a new frame for this image, we have to create
-  // it now.
+  // If we're currently waiting on a new frame for this image, create it now.
   if (mDecoder && mDecoder->NeedsNewFrame()) {
     mDecoder->AllocateFrame();
-    mDecodeRequest->mAllocatedNewFrame = true;
   }
 
   // If we don't have a decoder, create one
   if (!mDecoder) {
     rv = InitDecoder(/* aDoSizeDecode = */ false);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
@@ -2775,18 +2773,17 @@ RasterImage::DecodeSomeData(size_t aMaxB
 {
   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;
+  if (mDecoder->NeedsToFlushData()) {
     nsresult rv = WriteToDecoder(nullptr, 0, aStrategy);
     if (NS_FAILED(rv) || mDecoder->NeedsNewFrame()) {
       return rv;
     }
   }
 
   // If we have nothing else to decode, return.
   if (mDecoder->BytesDecoded() == mSourceData.Length()) {
@@ -2822,18 +2819,17 @@ RasterImage::IsDecodeFinished()
     }
   } else if (mDecoder->GetDecodeDone()) {
     return true;
   }
 
   // If the decoder returned because it needed a new frame and we haven't
   // written to it since then, the decoder may be storing data that it hasn't
   // decoded yet.
-  if (mDecoder->NeedsNewFrame() ||
-      (mDecodeRequest && mDecodeRequest->mAllocatedNewFrame)) {
+  if (mDecoder->NeedsNewFrame() || mDecoder->NeedsToFlushData()) {
     return false;
   }
 
   // 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
@@ -3412,19 +3408,17 @@ RasterImage::DecodePool::DecodeSomeOfIma
   // example, a synchronous decode request came while the worker was pending).
   if (!aImg->mDecoder || aImg->mDecoded)
     return NS_OK;
 
   // If we're doing synchronous decodes, and we're waiting on a new frame for
   // this image, get it now.
   if (aStrategy == DECODE_SYNC && aImg->mDecoder->NeedsNewFrame()) {
     MOZ_ASSERT(NS_IsMainThread());
-
     aImg->mDecoder->AllocateFrame();
-    aImg->mDecodeRequest->mAllocatedNewFrame = true;
   }
 
   // If we're not synchronous, we can't allocate a frame right now.
   else if (aImg->mDecoder->NeedsNewFrame()) {
     return NS_OK;
   }
 
   nsRefPtr<Decoder> decoderKungFuDeathGrip = aImg->mDecoder;
@@ -3455,17 +3449,17 @@ RasterImage::DecodePool::DecodeSomeOfIma
   //  * 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->mDecoder->BytesDecoded() &&
           bytesToDecode > 0 &&
           !aImg->IsDecodeFinished() &&
           !(aDecodeType == DECODE_TYPE_UNTIL_SIZE && aImg->mHasSize) &&
           !aImg->mDecoder->NeedsNewFrame()) ||
-         (aImg->mDecodeRequest && aImg->mDecodeRequest->mAllocatedNewFrame)) {
+         aImg->mDecoder->NeedsToFlushData()) {
     uint32_t chunkSize = std::min(bytesToDecode, maxBytes);
     nsresult rv = aImg->DecodeSomeData(chunkSize, aStrategy);
     if (NS_FAILED(rv)) {
       aImg->DoError();
       return rv;
     }
 
     bytesToDecode -= chunkSize;
@@ -3546,17 +3540,16 @@ RasterImage::FrameNeededWorker::Run()
 {
   ReentrantMonitorAutoEnter lock(mImage->mDecodingMonitor);
   nsresult rv = NS_OK;
 
   // If we got a synchronous decode in the mean time, we don't need to do
   // anything.
   if (mImage->mDecoder && mImage->mDecoder->NeedsNewFrame()) {
     rv = mImage->mDecoder->AllocateFrame();
-    mImage->mDecodeRequest->mAllocatedNewFrame = true;
   }
 
   if (NS_SUCCEEDED(rv) && mImage->mDecoder) {
     // By definition, we're not done decoding, so enqueue us for more decoding.
     DecodePool::Singleton()->RequestDecode(mImage);
   }
 
   return NS_OK;
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -325,17 +325,16 @@ private:
    * Each RasterImage has a pointer to one or zero heap-allocated
    * DecodeRequests.
    */
   struct DecodeRequest
   {
     explicit DecodeRequest(RasterImage* aImage)
       : mImage(aImage)
       , mRequestStatus(REQUEST_INACTIVE)
-      , mAllocatedNewFrame(false)
     {
       MOZ_ASSERT(aImage, "aImage cannot be null");
       MOZ_ASSERT(aImage->mStatusTracker,
                  "aImage should have an imgStatusTracker");
       mStatusTracker = aImage->mStatusTracker->CloneForRecording();
     }
 
     NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodeRequest)
@@ -350,20 +349,16 @@ private:
     {
       REQUEST_INACTIVE,
       REQUEST_PENDING,
       REQUEST_ACTIVE,
       REQUEST_WORK_DONE,
       REQUEST_STOPPED
     } mRequestStatus;
 
-    /* True if a new frame has been allocated, but DecodeSomeData hasn't yet
-     * been called to flush data to it */
-    bool mAllocatedNewFrame;
-
   private:
     ~DecodeRequest() {}
   };
 
   /*
    * DecodePool is a singleton class we use when decoding large images.
    *
    * When we wish to decode an image larger than