Bug 1079653 (Part 5) - Move decode status tracking onto RasterImage and remove DecodeRequest. r=tn
☠☠ backed out by a13927f78353 ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Wed, 15 Oct 2014 13:52:22 -0700
changeset 210612 aaac8c67129941d2a969ed0df7c76eb30dacd789
parent 210611 59d1754eb01fd84ec139d686fc7b2f79079f17c1
child 210613 f013910bbead6cad6f72a56dcc714cdf9e7b2991
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerstn
bugs1079653
milestone36.0a1
Bug 1079653 (Part 5) - Move decode status tracking onto RasterImage and remove DecodeRequest. r=tn
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -316,16 +316,17 @@ RasterImage::RasterImage(imgStatusTracke
   mLockCount(0),
   mDecodeCount(0),
   mRequestedSampleSize(0),
 #ifdef DEBUG
   mFramesNotified(0),
 #endif
   mDecodingMonitor("RasterImage Decoding Monitor"),
   mDecoder(nullptr),
+  mDecodeStatus(DecodeStatus::INACTIVE),
   mInDecoder(false),
   mStatusDiff(ImageStatusDiff::NoChange()),
   mNotifying(false),
   mHasSize(false),
   mDecodeOnDraw(false),
   mMultipart(false),
   mDiscardable(false),
   mHasSourceData(false),
@@ -1761,17 +1762,17 @@ RasterImage::OnNewSourceData()
   // The decoder was shut down and we didn't flag an error, so we should be decoded
   NS_ABORT_IF_FALSE(mDecoded, "Should be decoded in NewSourceData");
 
   // Reset some flags
   mDecoded = false;
   mHasSourceData = false;
   mHasSize = false;
   mWantFullDecode = true;
-  mDecodeRequest = nullptr;
+  mDecodeStatus = DecodeStatus::INACTIVE;
   mDecodeStatusTracker = nullptr;
 
   if (mAnim) {
     mAnim->SetDoneDecoding(false);
   }
 
   // We always need the size first.
   rv = InitDecoder(/* aDoSizeDecode = */ true);
@@ -1863,17 +1864,17 @@ RasterImage::Discard(bool force)
 
   // Flag that we no longer have decoded frames for this image
   mDecoded = false;
 
   // Notify that we discarded
   if (mStatusTracker)
     mStatusTracker->OnDiscard();
 
-  mDecodeRequest = nullptr;
+  mDecodeStatus = DecodeStatus::INACTIVE;
   mDecodeStatusTracker = nullptr;
 
   if (force)
     DiscardTracker::Remove(&mDiscardTrackerNode);
 
   // Log
   PR_LOG(GetCompressedImageAccountingLog(), PR_LOG_DEBUG,
          ("CompressedImageAccounting: discarded uncompressed image "
@@ -1984,19 +1985,16 @@ RasterImage::InitDecoder(bool aDoSizeDec
   // case. Regardless, we need to lock the last frame. Our invariant is that,
   // while we have a decoder open, the last frame is always locked.
   if (GetNumFrames() > 0) {
     nsRefPtr<imgFrame> curframe = mFrameBlender.RawGetFrame(GetNumFrames() - 1);
     curframe->LockImageData();
   }
 
   // Initialize the decoder
-  if (!mDecodeRequest) {
-    mDecodeRequest = new DecodeRequest();
-  }
   if (!mDecodeStatusTracker) {
     MOZ_ASSERT(mStatusTracker, "Should have an imgStatusTracker");
     mDecodeStatusTracker = mStatusTracker->CloneForRecording();
   }
   MOZ_ASSERT(mDecodeStatusTracker->GetDecoderObserver());
   mDecoder->SetObserver(mDecodeStatusTracker->GetDecoderObserver());
   mDecoder->SetSizeDecode(aDoSizeDecode);
   mDecoder->SetDecodeFlags(mFrameDecodeFlags);
@@ -2228,18 +2226,17 @@ RasterImage::RequestDecodeCore(RequestDe
     // data, so signal that we want a full decode and give up for now.
     if (!mHasSize) {
       mWantFullDecode = true;
       return NS_OK;
     }
   }
 
   // If the image is waiting for decode work to be notified, go ahead and do that.
-  if (mDecodeRequest &&
-      mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_WORK_DONE &&
+  if (mDecodeStatus == DecodeStatus::WORK_DONE &&
       aDecodeType == SYNCHRONOUS_NOTIFY) {
     ReentrantMonitorAutoEnter lock(mDecodingMonitor);
     nsresult rv = FinishedSomeDecoding();
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
   // If we're fully decoded, we have nothing to do. We need this check after
   // DecodeUntilSizeAvailable and FinishedSomeDecoding because they can result
@@ -2265,19 +2262,17 @@ RasterImage::RequestDecodeCore(RequestDe
   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) {
+  if (mDecodeStatus == DecodeStatus::WORK_DONE && aDecodeType != ASYNCHRONOUS) {
     nsresult rv = FinishedSomeDecoding();
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
   // If we're fully decoded, we have nothing to do. We need this check after
   // 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).
@@ -2358,22 +2353,20 @@ RasterImage::SyncDecode()
   ReentrantMonitorAutoEnter lock(mDecodingMonitor);
 
   // We really have no good way of forcing a synchronous decode if we're being
   // called in a re-entrant manner (ie, from an event listener fired by a
   // decoder), because the decoding machinery is already tied up. We thus explicitly
   // disallow this type of call in the API, and check for it in API methods.
   NS_ABORT_IF_FALSE(!mInDecoder, "Yikes, forcing sync in reentrant call!");
 
-  if (mDecodeRequest) {
-    // If the image is waiting for decode work to be notified, go ahead and do that.
-    if (mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_WORK_DONE) {
-      nsresult rv = FinishedSomeDecoding();
-      CONTAINER_ENSURE_SUCCESS(rv);
-    }
+  // If the image is waiting for decode work to be notified, go ahead and do that.
+  if (mDecodeStatus == DecodeStatus::WORK_DONE) {
+    nsresult rv = FinishedSomeDecoding();
+    CONTAINER_ENSURE_SUCCESS(rv);
   }
 
   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;
@@ -3199,46 +3192,44 @@ void
 RasterImage::DecodePool::RequestDecode(RasterImage* aImg)
 {
   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()) {
-    if (aImg->mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_PENDING ||
-        aImg->mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_ACTIVE) {
+    if (aImg->mDecodeStatus == DecodeStatus::PENDING ||
+        aImg->mDecodeStatus == DecodeStatus::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;
     }
 
-    aImg->mDecodeRequest->mRequestStatus = DecodeRequest::REQUEST_PENDING;
-    nsRefPtr<DecodeJob> job = new DecodeJob(aImg->mDecodeRequest, aImg);
+    aImg->mDecodeStatus = DecodeStatus::PENDING;
+    nsRefPtr<DecodeJob> job = new DecodeJob(aImg);
 
     MutexAutoLock threadPoolLock(mThreadPoolMutex);
     if (!gfxPrefs::ImageMTDecodingEnabled() || !mThreadPool) {
       NS_DispatchToMainThread(job);
     } else {
       mThreadPool->Dispatch(job, nsIEventTarget::DISPATCH_NORMAL);
     }
   }
 }
 
 void
 RasterImage::DecodePool::DecodeABitOf(RasterImage* aImg, DecodeStrategy aStrategy)
 {
   MOZ_ASSERT(NS_IsMainThread());
   aImg->mDecodingMonitor.AssertCurrentThreadIn();
 
-  if (aImg->mDecodeRequest) {
-    // If the image is waiting for decode work to be notified, go ahead and do that.
-    if (aImg->mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_WORK_DONE) {
-      aImg->FinishedSomeDecoding();
-    }
+  // If the image is waiting for decode work to be notified, go ahead and do that.
+  if (aImg->mDecodeStatus == DecodeStatus::WORK_DONE) {
+    aImg->FinishedSomeDecoding();
   }
 
   DecodeSomeOfImage(aImg, aStrategy);
 
   aImg->FinishedSomeDecoding();
 
   // If the decoder needs a new frame, enqueue an event to get it; that event
   // will enqueue another decode request when it's done.
@@ -3258,28 +3249,26 @@ RasterImage::DecodePool::DecodeABitOf(Ra
 
 /* static */ void
 RasterImage::DecodePool::StopDecoding(RasterImage* aImg)
 {
   aImg->mDecodingMonitor.AssertCurrentThreadIn();
 
   // 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) {
-    aImg->mDecodeRequest->mRequestStatus = DecodeRequest::REQUEST_STOPPED;
-  }
+  aImg->mDecodeStatus = DecodeStatus::STOPPED;
 }
 
 NS_IMETHODIMP
 RasterImage::DecodePool::DecodeJob::Run()
 {
   ReentrantMonitorAutoEnter lock(mImage->mDecodingMonitor);
 
   // If we were interrupted, we shouldn't do any work.
-  if (mRequest->mRequestStatus == DecodeRequest::REQUEST_STOPPED) {
+  if (mImage->mDecodeStatus == DecodeStatus::STOPPED) {
     DecodeDoneWorker::NotifyFinishedSomeDecoding(mImage,
                                                  mImage->mDecodeStatusTracker);
     return NS_OK;
   }
 
   // If someone came along and synchronously decoded us, there's nothing for us to do.
   if (!mImage->mDecoder || mImage->IsDecodeFinished()) {
     DecodeDoneWorker::NotifyFinishedSomeDecoding(mImage,
@@ -3289,17 +3278,17 @@ RasterImage::DecodePool::DecodeJob::Run(
 
   // If we're a decode job that's been enqueued since a previous decode that
   // 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;
+  mImage->mDecodeStatus = DecodeStatus::ACTIVE;
 
   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()) {
@@ -3308,17 +3297,17 @@ RasterImage::DecodePool::DecodeJob::Run(
 
   size_t maxBytes = mImage->mSourceData.Length() -
                     mImage->mDecoder->BytesDecoded();
   DecodePool::Singleton()->DecodeSomeOfImage(mImage, DECODE_ASYNC,
                                              type, maxBytes);
 
   size_t bytesDecoded = mImage->mDecoder->BytesDecoded() - oldByteCount;
 
-  mRequest->mRequestStatus = DecodeRequest::REQUEST_WORK_DONE;
+  mImage->mDecodeStatus = DecodeStatus::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);
   }
   // If we aren't yet finished decoding and we have more data in hand, add
   // this request to the back of the list.
@@ -3355,24 +3344,22 @@ RasterImage::DecodePool::DecodeJob::~Dec
 }
 
 nsresult
 RasterImage::DecodePool::DecodeUntilSizeAvailable(RasterImage* aImg)
 {
   MOZ_ASSERT(NS_IsMainThread());
   ReentrantMonitorAutoEnter lock(aImg->mDecodingMonitor);
 
-  if (aImg->mDecodeRequest) {
-    // If the image is waiting for decode work to be notified, go ahead and do that.
-    if (aImg->mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_WORK_DONE) {
-      nsresult rv = aImg->FinishedSomeDecoding();
-      if (NS_FAILED(rv)) {
-        aImg->DoError();
-        return rv;
-      }
+  // If the image is waiting for decode work to be notified, go ahead and do that.
+  if (aImg->mDecodeStatus == DecodeStatus::WORK_DONE) {
+    nsresult rv = aImg->FinishedSomeDecoding();
+    if (NS_FAILED(rv)) {
+      aImg->DoError();
+      return rv;
     }
   }
 
   // We use DECODE_ASYNC here because we just want to get the size information
   // here and defer the rest of the work.
   nsresult rv = DecodeSomeOfImage(aImg, DECODE_ASYNC, DECODE_TYPE_UNTIL_SIZE);
   if (NS_FAILED(rv)) {
     return rv;
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -28,16 +28,17 @@
 #include "DecodeStrategy.h"
 #include "DiscardTracker.h"
 #include "Orientation.h"
 #include "nsIObserver.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/ReentrantMonitor.h"
 #include "mozilla/TimeStamp.h"
+#include "mozilla/TypedEnum.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/WeakPtr.h"
 #include "mozilla/UniquePtr.h"
 #ifdef DEBUG
   #include "imgIContainerDebug.h"
 #endif
 
 class nsIInputStream;
@@ -131,16 +132,24 @@ class Image;
 }
 
 namespace image {
 
 class Decoder;
 class FrameAnimator;
 class ScaleRunner;
 
+MOZ_BEGIN_ENUM_CLASS(DecodeStatus, uint8_t)
+  INACTIVE,
+  PENDING,
+  ACTIVE,
+  WORK_DONE,
+  STOPPED
+MOZ_END_ENUM_CLASS(DecodeStatus)
+
 class RasterImage MOZ_FINAL : public ImageResource
                             , public nsIProperties
                             , public SupportsWeakPtr<RasterImage>
 #ifdef DEBUG
                             , public imgIContainerDebug
 #endif
 {
   // (no public constructor - use ImageFactory)
@@ -314,45 +323,16 @@ private:
     mDecodingMonitor.AssertCurrentThreadIn();
     nsRefPtr<imgStatusTracker> statusTracker;
     statusTracker = mDecodeStatusTracker ? mDecodeStatusTracker
                                          : mStatusTracker;
     MOZ_ASSERT(statusTracker);
     return statusTracker.forget();
   }
 
-  nsresult OnImageDataCompleteCore(nsIRequest* aRequest, nsISupports*, nsresult aStatus);
-
-  /**
-   * Each RasterImage has a pointer to one or zero heap-allocated
-   * DecodeRequests.
-   */
-  struct DecodeRequest
-  {
-    explicit DecodeRequest()
-      : mRequestStatus(REQUEST_INACTIVE)
-    { }
-
-    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodeRequest)
-
-    RasterImage* mImage;
-
-    enum DecodeRequestStatus
-    {
-      REQUEST_INACTIVE,
-      REQUEST_PENDING,
-      REQUEST_ACTIVE,
-      REQUEST_WORK_DONE,
-      REQUEST_STOPPED
-    } mRequestStatus;
-
-  private:
-    ~DecodeRequest() {}
-  };
-
   /*
    * DecodePool is a singleton class we use when decoding large images.
    *
    * When we wish to decode an image larger than
    * image.mem.max_bytes_for_sync_decode, we call DecodePool::RequestDecode()
    * for the image.  This adds the image to a queue of pending requests and posts
    * the DecodePool singleton to the event queue, if it's not already pending
    * there.
@@ -430,28 +410,24 @@ private:
                                DecodeType aDecodeType = DECODE_TYPE_UNTIL_TIME,
                                uint32_t bytesToDecode = 0);
 
     /* A decode job dispatched to a thread pool by DecodePool.
      */
     class DecodeJob : public nsRunnable
     {
     public:
-      DecodeJob(DecodeRequest* aRequest, RasterImage* aImg)
-        : mRequest(aRequest)
-        , mImage(aImg)
-      {}
+      DecodeJob(RasterImage* aImage) : mImage(aImage) { }
 
-      NS_IMETHOD Run();
+      NS_IMETHOD Run() MOZ_OVERRIDE;
 
     protected:
       virtual ~DecodeJob();
 
     private:
-      nsRefPtr<DecodeRequest> mRequest;
       nsRefPtr<RasterImage> mImage;
     };
 
   private: /* members */
 
     // mThreadPoolMutex protects mThreadPool. For all RasterImages R,
     // R::mDecodingMonitor must be acquired before mThreadPoolMutex
     // if both are acquired; the other order may cause deadlock.
@@ -628,18 +604,18 @@ private: // data
 
   // BEGIN LOCKED MEMBER VARIABLES
   ReentrantMonitor           mDecodingMonitor;
 
   FallibleTArray<char>       mSourceData;
 
   // Decoder and friends
   nsRefPtr<Decoder>          mDecoder;
-  nsRefPtr<DecodeRequest>    mDecodeRequest;
   nsRefPtr<imgStatusTracker> mDecodeStatusTracker;
+  DecodeStatus               mDecodeStatus;
 
   bool                       mInDecoder;
   // END LOCKED MEMBER VARIABLES
 
   // Notification state. Used to avoid recursive notifications.
   ImageStatusDiff            mStatusDiff;
   bool                       mNotifying:1;