Bug 1079653 (Part 4) - Move the recording imgStatusTracker onto RasterImage. r=tn
☠☠ backed out by a13927f78353 ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Wed, 15 Oct 2014 13:52:22 -0700
changeset 210611 59d1754eb01fd84ec139d686fc7b2f79079f17c1
parent 210610 dfb0890b02be6dc35a3b76ddb1d5c31a3a9a888a
child 210612 aaac8c67129941d2a969ed0df7c76eb30dacd789
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerstn
bugs1079653
milestone36.0a1
Bug 1079653 (Part 4) - Move the recording imgStatusTracker onto RasterImage. r=tn
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -1762,16 +1762,17 @@ RasterImage::OnNewSourceData()
   NS_ABORT_IF_FALSE(mDecoded, "Should be decoded in NewSourceData");
 
   // Reset some flags
   mDecoded = false;
   mHasSourceData = false;
   mHasSize = false;
   mWantFullDecode = true;
   mDecodeRequest = nullptr;
+  mDecodeStatusTracker = nullptr;
 
   if (mAnim) {
     mAnim->SetDoneDecoding(false);
   }
 
   // We always need the size first.
   rv = InitDecoder(/* aDoSizeDecode = */ true);
   CONTAINER_ENSURE_SUCCESS(rv);
@@ -1863,16 +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;
+  mDecodeStatusTracker = nullptr;
 
   if (force)
     DiscardTracker::Remove(&mDiscardTrackerNode);
 
   // Log
   PR_LOG(GetCompressedImageAccountingLog(), PR_LOG_DEBUG,
          ("CompressedImageAccounting: discarded uncompressed image "
           "data from RasterImage %p (%s) - %d frames (cached count: %d); "
@@ -1983,21 +1985,24 @@ RasterImage::InitDecoder(bool aDoSizeDec
   // 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(this);
+    mDecodeRequest = new DecodeRequest();
   }
-  MOZ_ASSERT(mDecodeRequest->mStatusTracker);
-  MOZ_ASSERT(mDecodeRequest->mStatusTracker->GetDecoderObserver());
-  mDecoder->SetObserver(mDecodeRequest->mStatusTracker->GetDecoderObserver());
+  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);
   if (!aDoSizeDecode) {
     // We already have the size; tell the decoder so it can preallocate a
     // frame.  By default, we create an ARGB frame with no offset. If decoders
     // need a different type, they need to ask for it themselves.
     mDecoder->NeedNewFrame(0, 0, 0, mSize.width, mSize.height,
                            SurfaceFormat::B8G8R8A8);
@@ -2973,28 +2978,25 @@ RasterImage::RequestDecodeIfNeeded(nsres
   }
 
   // We don't need a full decode right now, so just return the existing status.
   return aStatus;
 }
 
 nsresult
 RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_Done */,
-                                  DecodeRequest* aRequest /* = nullptr */)
+                                  imgStatusTracker* aDecodeTracker /* = nullptr */)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   mDecodingMonitor.AssertCurrentThreadIn();
 
-  nsRefPtr<DecodeRequest> request;
-  if (aRequest) {
-    request = aRequest;
-  } else {
-    request = mDecodeRequest;
-  }
+  nsRefPtr<imgStatusTracker> statusTracker = aDecodeTracker
+                                           ? aDecodeTracker
+                                           : mDecodeStatusTracker.get();
 
   // Ensure that, if the decoder is the last reference to the image, we don't
   // destroy it by destroying the decoder.
   nsRefPtr<RasterImage> image(this);
 
   bool done = false;
   bool wasSize = false;
   nsresult rv = NS_OK;
@@ -3015,17 +3017,17 @@ RasterImage::FinishedSomeDecoding(eShutd
       done = true;
 
       // Hold on to a reference to the decoder until we're done with it
       nsRefPtr<Decoder> decoder = image->mDecoder;
 
       wasSize = decoder->IsSizeDecode();
 
       // Do some telemetry if this isn't a size decode.
-      if (request && !wasSize) {
+      if (!wasSize) {
         Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME,
                               int32_t(decoder->DecodeTime().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(decoder->BytesDecoded() /
@@ -3038,19 +3040,19 @@ RasterImage::FinishedSomeDecoding(eShutd
       // decoding routines have been finished.
       rv = image->ShutdownDecoder(aIntent);
       if (NS_FAILED(rv)) {
         image->DoError();
       }
     }
   }
 
-  ImageStatusDiff diff =
-    request ? image->mStatusTracker->Difference(request->mStatusTracker)
-            : image->mStatusTracker->DecodeStateAsDifference();
+  ImageStatusDiff diff = statusTracker
+                       ? image->mStatusTracker->Difference(statusTracker)
+                       : image->mStatusTracker->DecodeStateAsDifference();
   image->mStatusTracker->ApplyDifference(diff);
 
   if (mNotifying) {
     // Accumulate the status changes. We don't permit recursive notifications
     // because they cause subtle concurrency bugs, so we'll delay sending out
     // the notifications until we pop back to the lowest invocation of
     // FinishedSomeDecoding on the stack.
     NS_WARNING("Recursively notifying in RasterImage::FinishedSomeDecoding!");
@@ -3268,23 +3270,25 @@ RasterImage::DecodePool::StopDecoding(Ra
 
 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) {
-    DecodeDoneWorker::NotifyFinishedSomeDecoding(mImage, mRequest);
+    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, mRequest);
+    DecodeDoneWorker::NotifyFinishedSomeDecoding(mImage,
+                                                 mImage->mDecodeStatusTracker);
     return NS_OK;
   }
 
   // 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;
@@ -3322,17 +3326,18 @@ RasterImage::DecodePool::DecodeJob::Run(
            !mImage->mError &&
            !mImage->mPendingError &&
            !mImage->IsDecodeFinished() &&
            bytesDecoded < maxBytes &&
            bytesDecoded > 0) {
     DecodePool::Singleton()->RequestDecode(mImage);
   } else {
     // Nothing more for us to do - let everyone know what happened.
-    DecodeDoneWorker::NotifyFinishedSomeDecoding(mImage, mRequest);
+    DecodeDoneWorker::NotifyFinishedSomeDecoding(mImage,
+                                                 mImage->mDecodeStatusTracker);
   }
 
   return NS_OK;
 }
 
 RasterImage::DecodePool::DecodeJob::~DecodeJob()
 {
   if (gfxPrefs::ImageMTDecodingEnabled()) {
@@ -3493,37 +3498,33 @@ RasterImage::DecodePool::DecodeSomeOfIma
     aImg->mInDecoder = true;
     aImg->mDecoder->FlushInvalidations();
     aImg->mInDecoder = false;
   }
 
   return NS_OK;
 }
 
-RasterImage::DecodeDoneWorker::DecodeDoneWorker(RasterImage* image, DecodeRequest* request)
- : mImage(image)
- , mRequest(request)
-{}
-
 void
-RasterImage::DecodeDoneWorker::NotifyFinishedSomeDecoding(RasterImage* image, DecodeRequest* request)
+RasterImage::DecodeDoneWorker::NotifyFinishedSomeDecoding(RasterImage* aImage,
+                                                          imgStatusTracker* aTracker)
 {
-  image->mDecodingMonitor.AssertCurrentThreadIn();
-
-  nsCOMPtr<nsIRunnable> worker = new DecodeDoneWorker(image, request);
+  aImage->mDecodingMonitor.AssertCurrentThreadIn();
+
+  nsCOMPtr<nsIRunnable> worker = new DecodeDoneWorker(aImage, aTracker);
   NS_DispatchToMainThread(worker);
 }
 
 NS_IMETHODIMP
 RasterImage::DecodeDoneWorker::Run()
 {
   MOZ_ASSERT(NS_IsMainThread());
   ReentrantMonitorAutoEnter lock(mImage->mDecodingMonitor);
 
-  mImage->FinishedSomeDecoding(eShutdownIntent_Done, mRequest);
+  mImage->FinishedSomeDecoding(eShutdownIntent_Done, mTracker);
 
   return NS_OK;
 }
 
 RasterImage::FrameNeededWorker::FrameNeededWorker(RasterImage* image)
  : mImage(image)
 {}
 
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -308,46 +308,36 @@ public:
 
   // Decode strategy
 
 private:
   already_AddRefed<imgStatusTracker> CurrentStatusTracker()
   {
     mDecodingMonitor.AssertCurrentThreadIn();
     nsRefPtr<imgStatusTracker> statusTracker;
-    statusTracker = mDecodeRequest ? mDecodeRequest->mStatusTracker
-                                   : mStatusTracker;
+    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(RasterImage* aImage)
-      : mImage(aImage)
-      , mRequestStatus(REQUEST_INACTIVE)
-    {
-      MOZ_ASSERT(aImage, "aImage cannot be null");
-      MOZ_ASSERT(aImage->mStatusTracker,
-                 "aImage should have an imgStatusTracker");
-      mStatusTracker = aImage->mStatusTracker->CloneForRecording();
-    }
+    explicit DecodeRequest()
+      : mRequestStatus(REQUEST_INACTIVE)
+    { }
 
     NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodeRequest)
 
-    // The status tracker that is associated with a given decode request, to
-    // ensure their lifetimes are linked.
-    nsRefPtr<imgStatusTracker> mStatusTracker;
-
     RasterImage* mImage;
 
     enum DecodeRequestStatus
     {
       REQUEST_INACTIVE,
       REQUEST_PENDING,
       REQUEST_ACTIVE,
       REQUEST_WORK_DONE,
@@ -474,27 +464,31 @@ private:
   public:
     /**
      * Called by the DecodePool with an image when it's done some significant
      * portion of decoding that needs to be notified about.
      *
      * Ensures the decode state accumulated by the decoding process gets
      * applied to the image.
      */
-    static void NotifyFinishedSomeDecoding(RasterImage* image, DecodeRequest* request);
+    static void NotifyFinishedSomeDecoding(RasterImage* aImage,
+                                           imgStatusTracker* aTracker);
 
     NS_IMETHOD Run();
 
   private: /* methods */
-    DecodeDoneWorker(RasterImage* image, DecodeRequest* request);
+    DecodeDoneWorker(RasterImage* aImage, imgStatusTracker* aTracker)
+      : mImage(aImage)
+      , mTracker(aTracker)
+    { }
 
   private: /* members */
 
     nsRefPtr<RasterImage> mImage;
-    nsRefPtr<DecodeRequest> mRequest;
+    nsRefPtr<imgStatusTracker> mTracker;
   };
 
   class FrameNeededWorker : public nsRunnable
   {
   public:
     /**
      * Called by the DecodeJob with an image when it's been told by the
      * decoder that it needs a new frame to be allocated on the main thread.
@@ -509,18 +503,18 @@ private:
   private: /* methods */
     explicit FrameNeededWorker(RasterImage* image);
 
   private: /* members */
 
     nsRefPtr<RasterImage> mImage;
   };
 
-  nsresult FinishedSomeDecoding(eShutdownIntent intent = eShutdownIntent_Done,
-                                DecodeRequest* request = nullptr);
+  nsresult FinishedSomeDecoding(eShutdownIntent aIntent = eShutdownIntent_Done,
+                                imgStatusTracker* aDecodeTracker = nullptr);
 
   void DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef,
                                     gfxContext* aContext,
                                     const nsIntSize& aSize,
                                     const ImageRegion& aRegion,
                                     GraphicsFilter aFilter,
                                     uint32_t aFlags);
 
@@ -635,16 +629,17 @@ private: // data
   // BEGIN LOCKED MEMBER VARIABLES
   ReentrantMonitor           mDecodingMonitor;
 
   FallibleTArray<char>       mSourceData;
 
   // Decoder and friends
   nsRefPtr<Decoder>          mDecoder;
   nsRefPtr<DecodeRequest>    mDecodeRequest;
+  nsRefPtr<imgStatusTracker> mDecodeStatusTracker;
 
   bool                       mInDecoder;
   // END LOCKED MEMBER VARIABLES
 
   // Notification state. Used to avoid recursive notifications.
   ImageStatusDiff            mStatusDiff;
   bool                       mNotifying:1;