Bug 1069369 - Remove all manual discarding during frame lookup. r=tn a=kwierso
authorSeth Fowler <seth@mozilla.com>
Fri, 26 Sep 2014 18:50:24 -0700
changeset 207530 818f353b7aa6dd8287bbcee10b2ed0efdfeb34dc
parent 207529 43ab820c7b68d53a492bd3a92384f714f2d0ded9
child 207531 be94c1f4ab58760c77af40b80c7ffdae5cc24de8
child 207540 df78a0e806d73dcc2f5a8253c6da763894452b6e
child 207574 bf65863441bee77d6a677095b4d4406c25949a34
child 207609 8ee8dc6f1c318aa0462fca2de8c447894d2af2e0
push id27559
push usermfowler@mozilla.com
push dateSat, 27 Sep 2014 01:52:33 +0000
treeherdermozilla-central@818f353b7aa6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, kwierso
bugs1069369
milestone35.0a1
first release with
nightly linux32
818f353b7aa6 / 35.0a1 / 20140926185533 / files
nightly linux64
818f353b7aa6 / 35.0a1 / 20140926185533 / files
nightly mac
818f353b7aa6 / 35.0a1 / 20140926185533 / files
nightly win32
818f353b7aa6 / 35.0a1 / 20140926185533 / files
nightly win64
818f353b7aa6 / 35.0a1 / 20140926185533 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1069369 - Remove all manual discarding during frame lookup. r=tn a=kwierso
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -573,55 +573,65 @@ RasterImage::GetType(uint16_t *aType)
 /* [noscript, notxpcom] uint16_t GetType(); */
 NS_IMETHODIMP_(uint16_t)
 RasterImage::GetType()
 {
   return imgIContainer::TYPE_RASTER;
 }
 
 already_AddRefed<imgFrame>
-RasterImage::GetFrameNoDecode(uint32_t aFrameNum)
+RasterImage::LookupFrameNoDecode(uint32_t aFrameNum)
 {
   if (!mAnim) {
     NS_ASSERTION(aFrameNum == 0, "Don't ask for a frame > 0 if we're not animated!");
     return mFrameBlender.GetFrame(0);
   }
   return mFrameBlender.GetFrame(aFrameNum);
 }
 
 DrawableFrameRef
-RasterImage::GetFrame(uint32_t aFrameNum)
+RasterImage::LookupFrame(uint32_t aFrameNum,
+                         uint32_t aFlags,
+                         bool aShouldSyncNotify /* = true */)
 {
   if (mMultipart &&
       aFrameNum == GetCurrentFrameIndex() &&
       mMultipartDecodedFrame) {
     // In the multipart case we prefer to use mMultipartDecodedFrame, which is
     // the most recent one we completely decoded, rather than display the real
     // current frame and risk severe tearing.
     return mMultipartDecodedFrame->DrawableRef();
   }
 
   // Try our best to start decoding if it's necessary.
-  nsresult rv = WantDecodedFrames();
+  nsresult rv = WantDecodedFrames(aFlags, aShouldSyncNotify);
   CONTAINER_ENSURE_TRUE(NS_SUCCEEDED(rv), DrawableFrameRef());
 
-  nsRefPtr<imgFrame> frame = GetFrameNoDecode(aFrameNum);
+  nsRefPtr<imgFrame> frame = LookupFrameNoDecode(aFrameNum);
   if (!frame) {
     return DrawableFrameRef();
   }
 
   DrawableFrameRef ref = frame->DrawableRef();
   if (!ref) {
     // The OS threw this frame away. We need to discard and redecode.
     MOZ_ASSERT(!mAnim, "Animated frames should be locked");
     if (CanForciblyDiscardAndRedecode()) {
       ForceDiscard();
-      WantDecodedFrames();
+      WantDecodedFrames(aFlags, aShouldSyncNotify);
+
+      // See if we managed to entirely redecode the frame.
+      frame = LookupFrameNoDecode(aFrameNum);
+      ref = frame->DrawableRef();
     }
-    return DrawableFrameRef();
+
+    if (!ref) {
+      // We didn't successfully redecode, so just fail.
+      return DrawableFrameRef();
+    }
   }
 
   // We will return a paletted frame if it's not marked as compositing failed
   // so we can catch crashes for reasons we haven't investigated.
   if (ref->GetCompositingFailed()) {
     return DrawableFrameRef();
   }
 
@@ -654,17 +664,17 @@ RasterImage::FrameIsOpaque(uint32_t aWhi
     return false;
   }
 
   if (mError)
     return false;
 
   // See if we can get an image frame.
   nsRefPtr<imgFrame> frame =
-    GetFrameNoDecode(GetRequestedFrameIndex(aWhichFrame));
+    LookupFrameNoDecode(GetRequestedFrameIndex(aWhichFrame));
 
   // If we don't get a frame, the safe answer is "not opaque".
   if (!frame)
     return false;
 
   // Other, the frame is transparent if either:
   //  1. It needs a background.
   //  2. Its size doesn't cover our entire area.
@@ -678,17 +688,17 @@ RasterImage::FrameRect(uint32_t aWhichFr
 {
   if (aWhichFrame > FRAME_MAX_VALUE) {
     NS_WARNING("aWhichFrame outside valid range!");
     return nsIntRect();
   }
 
   // Get the requested frame.
   nsRefPtr<imgFrame> frame =
-    GetFrameNoDecode(GetRequestedFrameIndex(aWhichFrame));
+    LookupFrameNoDecode(GetRequestedFrameIndex(aWhichFrame));
 
   // If we have the frame, use that rectangle.
   if (frame) {
     return frame->GetRect();
   }
 
   // If the frame doesn't exist, we return the empty rectangle. It's not clear
   // whether this is appropriate in general, but at the moment the only
@@ -743,49 +753,39 @@ RasterImage::GetFirstFrameDelay()
   if (NS_FAILED(GetAnimated(&animated)) || !animated)
     return -1;
 
   return mFrameBlender.GetTimeoutForFrame(0);
 }
 
 TemporaryRef<SourceSurface>
 RasterImage::CopyFrame(uint32_t aWhichFrame,
-                       uint32_t aFlags)
+                       uint32_t aFlags,
+                       bool aShouldSyncNotify /* = true */)
 {
   if (aWhichFrame > FRAME_MAX_VALUE)
     return nullptr;
 
   if (mError)
     return nullptr;
 
   // Disallowed in the API
   if (mInDecoder && (aFlags & imgIContainer::FLAG_SYNC_DECODE))
     return nullptr;
 
-  nsresult rv;
-
   if (!ApplyDecodeFlags(aFlags, aWhichFrame))
     return nullptr;
 
-  // If requested, synchronously flush any data we have lying around to the decoder
-  if (aFlags & FLAG_SYNC_DECODE) {
-    rv = SyncDecode();
-    CONTAINER_ENSURE_TRUE(NS_SUCCEEDED(rv), nullptr);
-  }
-
   // Get the frame. If it's not there, it's probably the caller's fault for
   // not waiting for the data to be loaded from the network or not passing
   // FLAG_SYNC_DECODE
-  DrawableFrameRef frameRef = GetFrame(GetRequestedFrameIndex(aWhichFrame));
+  DrawableFrameRef frameRef = LookupFrame(GetRequestedFrameIndex(aWhichFrame),
+                                          aFlags, aShouldSyncNotify);
   if (!frameRef) {
-    // The OS threw this frame away.
-    if (aFlags & FLAG_SYNC_DECODE) {
-      ForceDiscard();
-      return CopyFrame(aWhichFrame, aFlags);
-    }
+    // The OS threw this frame away and we couldn't redecode it right now.
     return nullptr;
   }
 
   // Create a 32-bit image surface of our size, but draw using the frame's
   // rect, implicitly padding the frame out to the image's size.
 
   IntSize size(mSize.width, mSize.height);
   RefPtr<DataSourceSurface> surf =
@@ -827,88 +827,76 @@ RasterImage::CopyFrame(uint32_t aWhichFr
 
 //******************************************************************************
 /* [noscript] SourceSurface getFrame(in uint32_t aWhichFrame,
  *                                   in uint32_t aFlags); */
 NS_IMETHODIMP_(TemporaryRef<SourceSurface>)
 RasterImage::GetFrame(uint32_t aWhichFrame,
                       uint32_t aFlags)
 {
+  return GetFrameInternal(aWhichFrame, aFlags);
+}
+
+TemporaryRef<SourceSurface>
+RasterImage::GetFrameInternal(uint32_t aWhichFrame,
+                              uint32_t aFlags,
+                              bool aShouldSyncNotify /* = true */)
+{
   MOZ_ASSERT(aWhichFrame <= FRAME_MAX_VALUE);
 
   if (aWhichFrame > FRAME_MAX_VALUE)
     return nullptr;
 
   if (mError)
     return nullptr;
 
   // Disallowed in the API
   if (mInDecoder && (aFlags & imgIContainer::FLAG_SYNC_DECODE))
     return nullptr;
 
   if (!ApplyDecodeFlags(aFlags, aWhichFrame))
     return nullptr;
 
-  // If the caller requested a synchronous decode, do it
-  if (aFlags & FLAG_SYNC_DECODE) {
-    nsresult rv = SyncDecode();
-    CONTAINER_ENSURE_TRUE(NS_SUCCEEDED(rv), nullptr);
-  }
-
   // Get the frame. If it's not there, it's probably the caller's fault for
   // not waiting for the data to be loaded from the network or not passing
   // FLAG_SYNC_DECODE
-  DrawableFrameRef frameRef = GetFrame(GetRequestedFrameIndex(aWhichFrame));
+  DrawableFrameRef frameRef = LookupFrame(GetRequestedFrameIndex(aWhichFrame),
+                                          aFlags, aShouldSyncNotify);
   if (!frameRef) {
-    // The OS threw this frame away. We'll request a redecode.
-    if (aFlags & FLAG_SYNC_DECODE) {
-      ForceDiscard();
-      return GetFrame(aWhichFrame, aFlags);
-    }
+    // The OS threw this frame away and we couldn't redecode it.
     return nullptr;
   }
 
-
   // If this frame covers the entire image, we can just reuse its existing
   // surface.
   RefPtr<SourceSurface> frameSurf;
   nsIntRect frameRect = frameRef->GetRect();
   if (frameRect.x == 0 && frameRect.y == 0 &&
       frameRect.width == mSize.width &&
       frameRect.height == mSize.height) {
     frameSurf = frameRef->GetSurface();
   }
 
   // The image doesn't have a usable surface because it's been optimized away or
   // because it's a partial update frame from an animation. Create one.
   if (!frameSurf) {
-    frameSurf = CopyFrame(aWhichFrame, aFlags);
+    frameSurf = CopyFrame(aWhichFrame, aFlags, aShouldSyncNotify);
   }
 
   return frameSurf;
 }
 
 already_AddRefed<layers::Image>
 RasterImage::GetCurrentImage()
 {
-  if (!mDecoded) {
-    // We can't call StartDecoding because that can synchronously notify
-    // which can cause DOM modification
-    RequestDecodeCore(ASYNCHRONOUS);
-    return nullptr;
-  }
-
-  RefPtr<SourceSurface> surface = GetFrame(FRAME_CURRENT, FLAG_NONE);
+  RefPtr<SourceSurface> surface =
+    GetFrameInternal(FRAME_CURRENT, FLAG_NONE, /* aShouldSyncNotify = */ false);
   if (!surface) {
-    // The OS threw out some or all of our buffer. Start decoding again.
-    // GetFrame will only return null in the case that the image was
-    // discarded. We already checked that the image is decoded, so other
-    // error paths are not possible.
-    ForceDiscard();
-    RequestDecodeCore(ASYNCHRONOUS);
+    // The OS threw out some or all of our buffer. We'll need to wait for the
+    // redecode (which was automatically triggered by GetFrame) to complete.
     return nullptr;
   }
 
   if (!mImageContainer) {
     mImageContainer = LayerManager::CreateImageContainer();
   }
 
   CairoImage::Data cairoData;
@@ -1400,17 +1388,17 @@ RasterImage::StartAnimation()
 {
   if (mError)
     return NS_ERROR_FAILURE;
 
   NS_ABORT_IF_FALSE(ShouldAnimate(), "Should not animate!");
 
   EnsureAnimExists();
 
-  nsRefPtr<imgFrame> currentFrame = GetFrameNoDecode(GetCurrentFrameIndex());
+  nsRefPtr<imgFrame> currentFrame = LookupFrameNoDecode(GetCurrentFrameIndex());
   // A timeout of -1 means we should display this frame forever.
   if (currentFrame &&
       mFrameBlender.GetTimeoutForFrame(GetCurrentFrameIndex()) < 0) {
     mAnimationFinished = true;
     return NS_ERROR_ABORT;
   }
 
   if (mAnim) {
@@ -2132,35 +2120,44 @@ RasterImage::WriteToDecoder(const char *
   // Keep track of the total number of bytes written over the lifetime of the
   // decoder
   mBytesDecoded += aCount;
 
   return NS_OK;
 }
 
 // This function is called in situations where it's clear that we want the
-// frames in decoded form (Draw, GetFrame, etc).  If we're completely decoded,
+// frames in decoded form (Draw, LookupFrame, etc).  If we're completely decoded,
 // this method resets the discard timer (if we're discardable), since wanting
 // the frames now is a good indicator of wanting them again soon. If we're not
 // decoded, this method kicks off asynchronous decoding to generate the frames.
 nsresult
-RasterImage::WantDecodedFrames()
+RasterImage::WantDecodedFrames(uint32_t aFlags, bool aShouldSyncNotify)
 {
   nsresult rv;
 
   // If we can discard, the clock should be running. Reset it.
   if (CanDiscard()) {
     NS_ABORT_IF_FALSE(DiscardingActive(),
                       "Decoded and discardable but discarding not activated!");
     rv = DiscardTracker::Reset(&mDiscardTrackerNode);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
-  // Request a decode (no-op if we're decoded)
-  return StartDecoding();
+  // Request a decode, which does nothing if we're already decoded.
+  if (aShouldSyncNotify) {
+    // We can sync notify, which means we can also sync decode.
+    if (aFlags & FLAG_SYNC_DECODE) {
+      return SyncDecode();
+    }
+    return StartDecoding();
+  }
+
+  // We can't sync notify, so do an async decode.
+  return RequestDecodeCore(ASYNCHRONOUS);
 }
 
 //******************************************************************************
 /* void requestDecode() */
 NS_IMETHODIMP
 RasterImage::RequestDecode()
 {
   return RequestDecodeCore(SYNCHRONOUS_NOTIFY);
@@ -2670,17 +2667,18 @@ RasterImage::Draw(gfxContext* aContext,
   }
 
   // If a synchronous draw is requested, flush anything that might be sitting around
   if (aFlags & FLAG_SYNC_DECODE) {
     nsresult rv = SyncDecode();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  DrawableFrameRef ref = GetFrame(GetRequestedFrameIndex(aWhichFrame));
+  DrawableFrameRef ref = LookupFrame(GetRequestedFrameIndex(aWhichFrame),
+                                     aFlags);
   if (!ref) {
     return NS_OK; // Getting the frame (above) touches the image and kicks off decoding
   }
 
   DrawWithPreDownscaleIfNeeded(Move(ref), aContext, aSize, aRegion, aFilter, aFlags);
 
   if (mDecoded && !mDrawStartTime.IsNull()) {
       TimeDuration drawLatency = TimeStamp::Now() - mDrawStartTime;
@@ -3591,17 +3589,17 @@ RasterImage::OptimalImageSizeForDest(con
                            RasterSurfaceKey(destSize.ToIntSize(),
                                             DecodeFlags(aFlags)));
 
     if (frameRef && frameRef->ImageComplete()) {
         return destSize;  // We have an existing HQ scale for this size.
     }
     if (!frameRef) {
       // We could HQ scale to this size, but we haven't. Request a scale now.
-      frameRef = GetFrame(GetRequestedFrameIndex(aWhichFrame));
+      frameRef = LookupFrame(GetRequestedFrameIndex(aWhichFrame), aFlags);
       if (frameRef) {
         RequestScale(frameRef.get(), aFlags, destSize);
       }
     }
   }
 
   // We either can't HQ scale to this size or the scaled version isn't ready
   // yet. Use our intrinsic size for now.
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -536,20 +536,24 @@ private:
   void DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef,
                                     gfxContext* aContext,
                                     const nsIntSize& aSize,
                                     const ImageRegion& aRegion,
                                     GraphicsFilter aFilter,
                                     uint32_t aFlags);
 
   TemporaryRef<gfx::SourceSurface> CopyFrame(uint32_t aWhichFrame,
-                                             uint32_t aFlags);
+                                             uint32_t aFlags,
+                                             bool aShouldSyncNotify = true);
+  TemporaryRef<gfx::SourceSurface> GetFrameInternal(uint32_t aWhichFrame,
+                                                    uint32_t aFlags,
+                                                    bool aShouldSyncNotify = true);
 
-  already_AddRefed<imgFrame> GetFrameNoDecode(uint32_t aFrameNum);
-  DrawableFrameRef GetFrame(uint32_t aFrameNum);
+  already_AddRefed<imgFrame> LookupFrameNoDecode(uint32_t aFrameNum);
+  DrawableFrameRef LookupFrame(uint32_t aFrameNum, uint32_t aFlags, bool aShouldSyncNotify = true);
   uint32_t GetCurrentFrameIndex() const;
   uint32_t GetRequestedFrameIndex(uint32_t aWhichFrame) const;
 
   size_t SizeOfDecodedWithComputedFallbackIfHeap(gfxMemoryLocation aLocation,
                                                  MallocSizeOf aMallocSizeOf) const;
 
   void EnsureAnimExists();
 
@@ -691,17 +695,17 @@ private: // data
   // stop decode work immediately.
   bool                       mPendingError:1;
 
   // Decoding
   nsresult RequestDecodeIfNeeded(nsresult aStatus,
                                  eShutdownIntent aIntent,
                                  bool aDone,
                                  bool aWasSize);
-  nsresult WantDecodedFrames();
+  nsresult WantDecodedFrames(uint32_t aFlags, bool aShouldSyncNotify);
   nsresult SyncDecode();
   nsresult InitDecoder(bool aDoSizeDecode);
   nsresult WriteToDecoder(const char *aBuffer, uint32_t aCount, DecodeStrategy aStrategy);
   nsresult DecodeSomeData(size_t aMaxBytes, DecodeStrategy aStrategy);
   bool     IsDecodeFinished();
   TimeStamp mDrawStartTime;
 
   // Initializes imgStatusTracker and resets it on RasterImage destruction.