Bug 716140 - Heap-allocate DecodeRequests so we know when we're still decoding an image. r=seth
authorJoe Drew <joe@drew.ca>
Fri, 18 Jan 2013 16:47:18 -0500
changeset 132059 4bad4198e33c0f4499e5e6a1e689b1c1b485d43b
parent 132058 096b05d298ad5ad55fed2d98f042c2b80292d597
child 132060 761015312a0babc85f0502ff456f39cb960ad27f
push idunknown
push userunknown
push dateunknown
reviewersseth
bugs716140
milestone22.0a1
Bug 716140 - Heap-allocate DecodeRequests so we know when we're still decoding an image. r=seth
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -363,26 +363,16 @@ RasterImage::RasterImage(imgStatusTracke
                          nsIURI* aURI /* = nullptr */) :
   ImageResource(aStatusTracker, aURI), // invoke superclass's constructor
   mSize(0,0),
   mFrameDecodeFlags(DECODE_FLAGS_DEFAULT),
   mAnim(nullptr),
   mLoopCount(-1),
   mLockCount(0),
   mDecoder(nullptr),
-// We know DecodeRequest won't touch members of RasterImage
-// until this constructor completes
-#ifdef _MSC_VER
-#pragma warning(push)
-#pragma warning(disable : 4355)
-#endif
-  mDecodeRequest(this),
-#ifdef _MSC_VER
-#pragma warning(pop)
-#endif
   mBytesDecoded(0),
   mDecodeCount(0),
 #ifdef DEBUG
   mFramesNotified(0),
 #endif
   mHasSize(false),
   mDecodeOnDraw(false),
   mMultipart(false),
@@ -2629,17 +2619,17 @@ RasterImage::InitDecoder(bool aDoSizeDec
     mDecodeCount++;
     Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(mDecodeCount);
 
     if (mDecodeCount > sMaxDecodeCount) {
       // Don't subtract out 0 from the histogram, because that causes its count
       // to go negative, which is not kosher.
       if (sMaxDecodeCount > 0) {
         Telemetry::GetHistogramById(Telemetry::IMAGE_MAX_DECODE_COUNT)->Subtract(sMaxDecodeCount);
-      }
+  }
       sMaxDecodeCount = mDecodeCount;
       Telemetry::GetHistogramById(Telemetry::IMAGE_MAX_DECODE_COUNT)->Add(sMaxDecodeCount);
     }
   }
 
   return NS_OK;
 }
 
@@ -3371,17 +3361,23 @@ RasterImage::DecodeWorker::Singleton()
   }
 
   return sSingleton;
 }
 
 void
 RasterImage::DecodeWorker::MarkAsASAP(RasterImage* aImg)
 {
-  DecodeRequest* request = &aImg->mDecodeRequest;
+  // We can be marked as ASAP before we've been asked to decode. If we are,
+  // create the request so we have somewhere to write down our status.
+  if (aImg->mDecodeRequest) {
+    CreateRequestForImage(aImg);
+  }
+
+  DecodeRequest* request = aImg->mDecodeRequest;
 
   // If we're already an ASAP request, there's nothing to do here.
   if (request->mIsASAP) {
     return;
   }
 
   request->mIsASAP = true;
 
@@ -3416,19 +3412,29 @@ RasterImage::DecodeWorker::AddDecodeRequ
   if (aRequest->mIsASAP) {
     mASAPDecodeRequests.insertBack(aRequest);
   } else {
     mNormalDecodeRequests.insertBack(aRequest);
   }
 }
 
 void
+RasterImage::DecodeWorker::CreateRequestForImage(RasterImage* aImg)
+{
+  aImg->mDecodeRequest = new DecodeRequest(aImg);
+}
+
+void
 RasterImage::DecodeWorker::RequestDecode(RasterImage* aImg)
 {
-  AddDecodeRequest(&aImg->mDecodeRequest);
+  if (!aImg->mDecodeRequest) {
+    CreateRequestForImage(aImg);
+  }
+
+  AddDecodeRequest(aImg->mDecodeRequest);
   EnsurePendingInEventLoop();
 }
 
 void
 RasterImage::DecodeWorker::DecodeABitOf(RasterImage* aImg)
 {
   DecodeSomeOfImage(aImg);
 
@@ -3449,22 +3455,24 @@ RasterImage::DecodeWorker::EnsurePending
     mPendingInEventLoop = true;
     NS_DispatchToCurrentThread(this);
   }
 }
 
 void
 RasterImage::DecodeWorker::StopDecoding(RasterImage* aImg)
 {
-  DecodeRequest* request = &aImg->mDecodeRequest;
-  if (request->isInList()) {
-    request->remove();
+  // If we haven't got a decode request, we're not currently decoding. (Having
+  // a decode request doesn't imply we *are* decoding, though.)
+  if (aImg->mDecodeRequest) {
+    if (aImg->mDecodeRequest->isInList()) {
+      aImg->mDecodeRequest->remove();
+    }
+    aImg->mDecodeRequest = nullptr;
   }
-  request->mDecodeTime = TimeDuration(0);
-  request->mIsASAP = false;
 }
 
 NS_IMETHODIMP
 RasterImage::DecodeWorker::Run()
 {
   // We just got called back by the event loop; therefore, we're no longer
   // pending.
   mPendingInEventLoop = false;
@@ -3479,17 +3487,17 @@ RasterImage::DecodeWorker::Run()
     DecodeRequest* request = mASAPDecodeRequests.popFirst();
     if (!request)
       request = mNormalDecodeRequests.popFirst();
     if (!request)
       break;
 
     // This has to be a strong pointer, because DecodeSomeOfImage may destroy
     // image->mDecoder, which may be holding the only other reference to image.
-    nsRefPtr<RasterImage> image = request->mImage;
+    RefPtr<RasterImage> image = request->mImage;
     DecodeSomeOfImage(image);
 
     // If we aren't yet finished decoding and we have more data in hand, add
     // this request to the back of the list.
     if (image->mDecoder &&
         !image->mError &&
         !image->IsDecodeFinished() &&
         image->mSourceData.Length() > image->mBytesDecoded) {
@@ -3567,17 +3575,23 @@ RasterImage::DecodeWorker::DecodeSomeOfI
     }
 
     // Yield if we've been decoding for too long. We check this _after_ decoding
     // a chunk to ensure that we don't yield without doing any decoding.
     if (TimeStamp::Now() >= deadline)
       break;
   }
 
-  aImg->mDecodeRequest.mDecodeTime += (TimeStamp::Now() - start);
+  if (!aImg->mDecodeRequest) {
+    MOZ_ASSERT(aDecodeType == DECODE_TYPE_UNTIL_SIZE);
+  }
+
+  if (aImg->mDecodeRequest) {
+    aImg->mDecodeRequest->mDecodeTime += (TimeStamp::Now() - start);
+  }
 
   if (chunkCount && !aImg->mDecoder->IsSizeDecode()) {
     Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, chunkCount);
   }
 
   // Flush invalidations (and therefore paint) now that we've decoded all the
   // chunks we're going to.
   //
@@ -3602,18 +3616,18 @@ RasterImage::DecodeWorker::DecodeSomeOfI
     aImg->mDecoder->FlushInvalidations();
     aImg->mInDecoder = false;
   }
 
   // If the decode finished, shut down the decoder.
   if (aImg->mDecoder && aImg->IsDecodeFinished()) {
 
     // Do some telemetry if this isn't a size decode.
-    DecodeRequest* request = &aImg->mDecodeRequest;
-    if (!aImg->mDecoder->IsSizeDecode()) {
+    DecodeRequest* request = aImg->mDecodeRequest;
+    if (request && !aImg->mDecoder->IsSizeDecode()) {
       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 = aImg->mDecoder->SpeedHistogram();
       if (id < Telemetry::HistogramCount) {
           int32_t KBps = int32_t(request->mImage->mBytesDecoded /
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -28,16 +28,17 @@
 #include "imgFrame.h"
 #include "nsThreadUtils.h"
 #include "DiscardTracker.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/WeakPtr.h"
+#include "mozilla/RefPtr.h"
 #include "gfx2DGlue.h"
 #ifdef DEBUG
   #include "imgIContainerDebug.h"
 #endif
 
 class nsIInputStream;
 
 #define NS_RASTERIMAGE_CID \
@@ -367,27 +368,29 @@ private:
       lastCompositedFrameIndex(-1) {}
     ~Anim() {}
   };
 
   /**
    * DecodeWorker keeps a linked list of DecodeRequests to keep track of the
    * images it needs to decode.
    *
-   * Each RasterImage has a single DecodeRequest member.
+   * Each RasterImage has a pointer to one or zero heap-allocated
+   * DecodeRequests.
    */
-  struct DecodeRequest : public LinkedListElement<DecodeRequest>
+  struct DecodeRequest : public LinkedListElement<DecodeRequest>,
+                         public RefCounted<DecodeRequest>
   {
     DecodeRequest(RasterImage* aImage)
       : mImage(aImage)
       , mIsASAP(false)
     {
     }
 
-    RasterImage* const mImage;
+    RasterImage* mImage;
 
     /* Keeps track of how much time we've burned decoding this particular decode
      * request. */
     TimeDuration mDecodeTime;
 
     /* True if we need to handle this decode as soon as possible. */
     bool mIsASAP;
   };
@@ -476,16 +479,20 @@ private:
       DECODE_TYPE_UNTIL_SIZE
     };
 
     /* Decode some chunks of the given image.  If aDecodeType is UNTIL_SIZE,
      * decode until we have the image's size, then stop. */
     nsresult DecodeSomeOfImage(RasterImage* aImg,
                                DecodeType aDecodeType = DECODE_TYPE_NORMAL);
 
+    /* Create a new DecodeRequest suitable for doing some decoding and set it
+     * as aImg's mDecodeRequest. */
+    void CreateRequestForImage(RasterImage* aImg);
+
   private: /* members */
 
     LinkedList<DecodeRequest> mASAPDecodeRequests;
     LinkedList<DecodeRequest> mNormalDecodeRequests;
 
     /* True if we've posted ourselves to the event loop and expect Run() to
      * be called sometime in the future. */
     bool mPendingInEventLoop;
@@ -662,17 +669,17 @@ private: // data
   // Source data members
   FallibleTArray<char>       mSourceData;
   nsCString                  mSourceDataMimeType;
 
   friend class DiscardTracker;
 
   // Decoder and friends
   nsRefPtr<Decoder>              mDecoder;
-  DecodeRequest                  mDecodeRequest;
+  nsRefPtr<DecodeRequest>        mDecodeRequest;
   uint32_t                       mBytesDecoded;
 
   // How many times we've decoded this image.
   // This is currently only used for statistics
   int32_t                        mDecodeCount;
 
   // Cached value for GetImageContainer.
   nsRefPtr<mozilla::layers::ImageContainer> mImageContainer;