Bug 1187386 (Part 7) - Eliminate remaining dependencies on a non-null mImage in Decoder. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 31 Jul 2015 07:29:18 -0700
changeset 255702 9c768fc28bb09825dd20dfecd2df3021a1462274
parent 255701 819db042055a9105a1ea3f563d6c5365006892bb
child 255703 829561e0df63f17eaf43ad4c03a1720c7b230507
push id63115
push usermfowler@mozilla.com
push dateFri, 31 Jul 2015 20:11:05 +0000
treeherdermozilla-inbound@9c768fc28bb0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1187386
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.