Bug 1079653 (Part 3) - Make decoders track whether they need to flush…

… data after getting a new frame. r=tn

--HG--
extra : rebase_source : c2022c4dd83dbcc87199b4b51335215cbc9adcb0

image/src/Decoder.cpp

@@ -27,6 +27,7 @@ Decoder::Decoder(RasterImage &aImage)
   , mFrameCount(0)
   , mFailCode(NS_OK)
   , mNeedsNewFrame(false)
+  , mNeedsToFlushData(false)
   , mInitialized(false)
   , mSizeDecode(false)
   , mInFrame(false)
@@ -105,6 +106,12 @@ Decoder::Write(const char* aBuffer, uint32_t aCount, DecodeStrategy aStrategy)
   // 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;
@@ -250,6 +257,12 @@ Decoder::AllocateFrame()
   // 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;
 }
 

image/src/Decoder.h

@@ -44,6 +44,11 @@ class Decoder
   /**
    * 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
    *
@@ -164,6 +169,11 @@ class Decoder
 
   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();
@@ -283,6 +293,7 @@ class Decoder
   };
   NewFrameData mNewFrameData;
   bool mNeedsNewFrame;
+  bool mNeedsToFlushData;
   bool mInitialized;
   bool mSizeDecode;
   bool mInFrame;

image/src/RasterImage.cpp

@@ -2396,11 +2396,9 @@ RasterImage::SyncDecode()
     }
   }
 
-  // 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
@@ -2780,8 +2778,7 @@ RasterImage::DecodeSomeData(size_t aMaxBytes, DecodeStrategy aStrategy)
   // 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;
@@ -2827,8 +2824,7 @@ RasterImage::IsDecodeFinished()
   // 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;
   }
 
@@ -3417,9 +3413,7 @@ RasterImage::DecodePool::DecodeSomeOfImage(RasterImage* aImg,
   // 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.
@@ -3460,7 +3454,7 @@ RasterImage::DecodePool::DecodeSomeOfImage(RasterImage* aImg,
           !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)) {
@@ -3551,7 +3545,6 @@ RasterImage::FrameNeededWorker::Run()
   // anything.
   if (mImage->mDecoder && mImage->mDecoder->NeedsNewFrame()) {
     rv = mImage->mDecoder->AllocateFrame();
-    mImage->mDecodeRequest->mAllocatedNewFrame = true;
   }
 
   if (NS_SUCCEEDED(rv) && mImage->mDecoder) {

image/src/RasterImage.h

@@ -330,7 +330,6 @@ class RasterImage MOZ_FINAL : public ImageResource
     explicit DecodeRequest(RasterImage* aImage)
       : mImage(aImage)
       , mRequestStatus(REQUEST_INACTIVE)
-      , mAllocatedNewFrame(false)
     {
       MOZ_ASSERT(aImage, "aImage cannot be null");
       MOZ_ASSERT(aImage->mStatusTracker,
@@ -355,10 +354,6 @@ class RasterImage MOZ_FINAL : public ImageResource
       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() {}
   };