Bug 1187386 (Part 7) - Eliminate remaining dependencies on a non-null mImage in Decoder. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 28 Jul 2015 13:42:23 -0700
changeset 516438 0fc6032db4b3cf74564695777c5cc6a1e83afa58
parent 516437 04af50e295614e6f90fd02388ec1f50350e38230
child 516439 e0fd924a0c5022a84e3f1a8164358a3715280cd0
push id80501
push usermfowler@mozilla.com
push dateTue, 28 Jul 2015 20:44:21 +0000
treeherdertry@8d2d2e5af3aa [default view] [failures only]
reviewerstn
bugs1187386
milestone42.0a1
Bug 1187386 (Part 7) - Eliminate remaining dependencies on a non-null mImage in Decoder. r=tn
image/Decoder.cpp
image/Decoder.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -44,19 +44,19 @@ Decoder::Decoder(RasterImage* aImage)
   , mDecodeDone(false)
   , mDataError(false)
   , mDecodeAborted(false)
   , mShouldReportError(false)
 { }
 
 Decoder::~Decoder()
 {
-  MOZ_ASSERT(mProgress == NoProgress,
+  MOZ_ASSERT(mProgress == NoProgress || !mImage,
              "Destroying Decoder without taking all its progress changes");
-  MOZ_ASSERT(mInvalidRect.IsEmpty(),
+  MOZ_ASSERT(mInvalidRect.IsEmpty() || !mImage,
              "Destroying Decoder without taking all its invalidations");
   mInitialized = false;
 
   if (mImage && !NS_IsMainThread()) {
     // Dispatch mImage to main thread to prevent it from being destructed by the
     // decode thread.
     nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
     NS_WARN_IF_FALSE(mainThread, "Couldn't get the main thread!");
@@ -283,17 +283,18 @@ Decoder::AllocateFrameInternal(uint32_t 
 
   if (aTargetSize.width <= 0 || aTargetSize.height <= 0 ||
       aFrameRect.width <= 0 || aFrameRect.height <= 0) {
     NS_WARNING("Trying to add frame with zero or negative size");
     return RawAccessFrameRef();
   }
 
   const uint32_t bytesPerPixel = aPaletteDepth == 0 ? 4 : 1;
-  if (!SurfaceCache::CanHold(aFrameRect.Size(), bytesPerPixel)) {
+  if (ShouldUseSurfaceCache() &&
+      !SurfaceCache::CanHold(aFrameRect.Size(), bytesPerPixel)) {
     NS_WARNING("Trying to add frame that's too large for the SurfaceCache");
     return RawAccessFrameRef();
   }
 
   nsRefPtr<imgFrame> frame = new imgFrame();
   bool nonPremult =
     aDecodeFlags & imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA;
   if (NS_FAILED(frame->InitForDecoder(aTargetSize, aFrameRect, aFormat,
@@ -303,34 +304,36 @@ Decoder::AllocateFrameInternal(uint32_t 
   }
 
   RawAccessFrameRef ref = frame->RawAccessRef();
   if (!ref) {
     frame->Abort();
     return RawAccessFrameRef();
   }
 
-  InsertOutcome outcome =
-    SurfaceCache::Insert(frame, ImageKey(mImage.get()),
-                         RasterSurfaceKey(aTargetSize,
-                                          aDecodeFlags,
-                                          aFrameNum),
-                         Lifetime::Persistent);
-  if (outcome == InsertOutcome::FAILURE) {
-    // We couldn't insert the surface, almost certainly due to low memory. We
-    // treat this as a permanent error to help the system recover; otherwise, we
-    // might just end up attempting to decode this image again immediately.
-    ref->Abort();
-    return RawAccessFrameRef();
-  } else if (outcome == InsertOutcome::FAILURE_ALREADY_PRESENT) {
-    // Another decoder beat us to decoding this frame. We abort this decoder
-    // rather than treat this as a real error.
-    mDecodeAborted = true;
-    ref->Abort();
-    return RawAccessFrameRef();
+  if (ShouldUseSurfaceCache()) {
+    InsertOutcome outcome =
+      SurfaceCache::Insert(frame, ImageKey(mImage.get()),
+                           RasterSurfaceKey(aTargetSize,
+                                            aDecodeFlags,
+                                            aFrameNum),
+                           Lifetime::Persistent);
+    if (outcome == InsertOutcome::FAILURE) {
+      // We couldn't insert the surface, almost certainly due to low memory. We
+      // treat this as a permanent error to help the system recover; otherwise,
+      // we might just end up attempting to decode this image again immediately.
+      ref->Abort();
+      return RawAccessFrameRef();
+    } else if (outcome == InsertOutcome::FAILURE_ALREADY_PRESENT) {
+      // Another decoder beat us to decoding this frame. We abort this decoder
+      // rather than treat this as a real error.
+      mDecodeAborted = true;
+      ref->Abort();
+      return RawAccessFrameRef();
+    }
   }
 
   nsIntRect refreshArea;
 
   if (aFrameNum == 1) {
     MOZ_ASSERT(aPreviousFrame, "Must provide a previous frame when animated");
     aPreviousFrame->SetRawAccessOnly();
 
@@ -349,17 +352,20 @@ Decoder::AllocateFrameInternal(uint32_t 
     ref->SetRawAccessOnly();
 
     // Some GIFs are huge but only have a small area that they animate. We only
     // need to refresh that small area when frame 0 comes around again.
     refreshArea.UnionRect(refreshArea, frame->GetRect());
   }
 
   mFrameCount++;
-  mImage->OnAddedFrame(mFrameCount, refreshArea);
+
+  if (mImage) {
+    mImage->OnAddedFrame(mFrameCount, refreshArea);
+  }
 
   return ref;
 }
 
 /*
  * Hook stubs. Override these as necessary in decoder implementations.
  */
 
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -232,16 +232,19 @@ public:
 
   /// Did we finish decoding enough to set |RasterImage::mHasBeenDecoded|?
   // XXX(seth): This will be removed in bug 1187401.
   bool GetDecodeTotallyDone() const { return mDecodeDone && !IsMetadataDecode(); }
 
   /// Are we in the middle of a frame right now? Used for assertions only.
   bool InFrame() const { return mInFrame; }
 
+  /// Should we store surfaces created by this decoder in the SurfaceCache?
+  bool ShouldUseSurfaceCache() const { return bool(mImage); }
+
   /**
    * Returns true if this decoder was aborted.
    *
    * This may happen due to a low-memory condition, or because another decoder
    * was racing with this one to decode the same frames with the same flags and
    * this decoder lost the race. Either way, this is not a permanent situation
    * and does not constitute an error, so we don't report any errors when this
    * happens.