Bug 1256603. Only mark images as used in the surface cache if we actually use them. r=aosmond

We were marking them used even if only a decode was requested.

This can cause us to hold extra decoded copies of the image around because we have a tendency to request decode at the intrinsic size.

image/FrameAnimator.cpp

@@ -37,7 +37,8 @@ AnimationState::UpdateState(bool aAnimationFinished,
     SurfaceCache::Lookup(ImageKey(aImage),
                          RasterSurfaceKey(aSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         /* aMarkUsed = */ false);
 
   return UpdateStateInternal(result, aAnimationFinished, aSize, aAllowInvalidation);
 }
@@ -398,7 +399,8 @@ FrameAnimator::ResetAnimation(AnimationState& aState)
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         /* aMarkUsed = */ false);
   if (!result) {
     return;
   }
@@ -427,7 +429,8 @@ FrameAnimator::RequestRefresh(AnimationState& aState,
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         /* aMarkUsed = */ true);
 
   ret.mDirtyRect = aState.UpdateStateInternal(result, aAnimationFinished, mSize);
   if (aState.IsDiscarded() || !result) {
@@ -502,7 +505,7 @@ FrameAnimator::RequestRefresh(AnimationState& aState,
 }
 
 LookupResult
-FrameAnimator::GetCompositedFrame(AnimationState& aState)
+FrameAnimator::GetCompositedFrame(AnimationState& aState, bool aMarkUsed)
 {
   aState.mCompositedFrameRequested = true;
 
@@ -517,7 +520,8 @@ FrameAnimator::GetCompositedFrame(AnimationState& aState)
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         aMarkUsed);
 
   if (aState.mCompositedFrameInvalid) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());

image/FrameAnimator.h

@@ -323,7 +323,8 @@ public:
    * not have required compositing). It may not be available because it hasn't
    * been decoded yet, in which case we return an empty LookupResult.
    */
-  LookupResult GetCompositedFrame(AnimationState& aState);
+  LookupResult GetCompositedFrame(AnimationState& aState,
+                                  bool aMarkUsed);
 
   /**
    * Collect an accounting of the memory occupied by the compositing surfaces we

image/RasterImage.cpp

@@ -298,13 +298,14 @@ RasterImage::GetType(uint16_t* aType)
 LookupResult
 RasterImage::LookupFrameInternal(const IntSize& aSize,
                                  uint32_t aFlags,
-                                 PlaybackType aPlaybackType)
+                                 PlaybackType aPlaybackType,
+                                 bool aMarkUsed)
 {
   if (mAnimationState && aPlaybackType == PlaybackType::eAnimated) {
     MOZ_ASSERT(mFrameAnimator);
     MOZ_ASSERT(ToSurfaceFlags(aFlags) == DefaultSurfaceFlags(),
                "Can't composite frames with non-default surface flags");
-    return mFrameAnimator->GetCompositedFrame(*mAnimationState);
+    return mFrameAnimator->GetCompositedFrame(*mAnimationState, aMarkUsed);
   }
 
   SurfaceFlags surfaceFlags = ToSurfaceFlags(aFlags);
@@ -316,20 +317,23 @@ RasterImage::LookupFrameInternal(const IntSize& aSize,
     return SurfaceCache::Lookup(ImageKey(this),
                                 RasterSurfaceKey(aSize,
                                                  surfaceFlags,
-                                                 PlaybackType::eStatic));
+                                                 PlaybackType::eStatic),
+                                aMarkUsed);
   }
 
   // We'll return the best match we can find to the requested frame.
   return SurfaceCache::LookupBestMatch(ImageKey(this),
                                        RasterSurfaceKey(aSize,
                                                         surfaceFlags,
-                                                        PlaybackType::eStatic));
+                                                        PlaybackType::eStatic),
+                                       aMarkUsed);
 }
 
 LookupResult
 RasterImage::LookupFrame(const IntSize& aSize,
                          uint32_t aFlags,
-                         PlaybackType aPlaybackType)
+                         PlaybackType aPlaybackType,
+                         bool aMarkUsed /* = true */)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -347,7 +351,7 @@ RasterImage::LookupFrame(const IntSize& aSize,
   }
 
   LookupResult result =
-    LookupFrameInternal(requestedSize, aFlags, aPlaybackType);
+    LookupFrameInternal(requestedSize, aFlags, aPlaybackType, aMarkUsed);
 
   if (!result && !mHasSize) {
     // We can't request a decode without knowing our intrinsic size. Give up.
@@ -377,7 +381,7 @@ RasterImage::LookupFrame(const IntSize& aSize,
 
     // If we can or did sync decode, we should already have the frame.
     if (ranSync || (aFlags & FLAG_SYNC_DECODE)) {
-      result = LookupFrameInternal(requestedSize, aFlags, aPlaybackType);
+      result = LookupFrameInternal(requestedSize, aFlags, aPlaybackType, aMarkUsed);
     }
   }
 
@@ -462,7 +466,8 @@ RasterImage::WillDrawOpaqueNow()
     SurfaceCache::LookupBestMatch(ImageKey(this),
                                   RasterSurfaceKey(mSize,
                                                    DefaultSurfaceFlags(),
-                                                   PlaybackType::eStatic));
+                                                   PlaybackType::eStatic),
+                                  /* aMarkUsed = */ false);
   MatchType matchType = result.Type();
   if (matchType == MatchType::NOT_FOUND || matchType == MatchType::PENDING ||
       !result.Surface()->IsFinished()) {
@@ -1200,7 +1205,7 @@ RasterImage::RequestDecodeForSizeInternal(const IntSize& aSize, uint32_t aFlags)
   // Perform a frame lookup, which will implicitly start decoding if needed.
   PlaybackType playbackType = mAnimationState ? PlaybackType::eAnimated
                                               : PlaybackType::eStatic;
-  LookupResult result = LookupFrame(aSize, flags, playbackType);
+  LookupResult result = LookupFrame(aSize, flags, playbackType, /* aMarkUsed = */ false);
   return std::move(result.Surface());
 }
 

image/RasterImage.h

@@ -284,19 +284,22 @@ private:
    * data, we'll attempt a sync decode if no matching surface is found. If
    * FLAG_SYNC_DECODE was not specified and no matching surface was found, we'll
    * kick off an async decode so that the surface is (hopefully) available next
-   * time it's requested.
+   * time it's requested. aMarkUsed determines if we mark the surface used in
+   * the surface cache or not.
    *
    * @return a drawable surface, which may be empty if the requested surface
    *         could not be found.
    */
   LookupResult LookupFrame(const gfx::IntSize& aSize,
                            uint32_t aFlags,
-                           PlaybackType aPlaybackType);
+                           PlaybackType aPlaybackType,
+                           bool aMarkUsed = true);
 
   /// Helper method for LookupFrame().
   LookupResult LookupFrameInternal(const gfx::IntSize& aSize,
                                    uint32_t aFlags,
-                                   PlaybackType aPlaybackType);
+                                   PlaybackType aPlaybackType,
+                                   bool aMarkUsed);
 
   ImgDrawResult DrawInternal(DrawableSurface&& aFrameRef,
                           gfxContext* aContext,

image/SurfaceCache.cpp

@@ -999,7 +999,8 @@ public:
 
   LookupResult LookupBestMatch(const ImageKey         aImageKey,
                                const SurfaceKey&      aSurfaceKey,
-                               const StaticMutexAutoLock& aAutoLock)
+                               const StaticMutexAutoLock& aAutoLock,
+                               bool aMarkUsed /* = true */)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
@@ -1045,7 +1046,8 @@ public:
 
     if (matchType == MatchType::EXACT ||
         matchType == MatchType::SUBSTITUTE_BECAUSE_BEST) {
-      if (!MarkUsed(WrapNotNull(surface), WrapNotNull(cache), aAutoLock)) {
+      if (aMarkUsed &&
+          !MarkUsed(WrapNotNull(surface), WrapNotNull(cache), aAutoLock)) {
         Remove(WrapNotNull(surface), /* aStopTracking */ false, aAutoLock);
       }
     }
@@ -1516,7 +1518,8 @@ SurfaceCache::Shutdown()
 
 /* static */ LookupResult
 SurfaceCache::Lookup(const ImageKey         aImageKey,
-                     const SurfaceKey&      aSurfaceKey)
+                     const SurfaceKey&      aSurfaceKey,
+                     bool aMarkUsed /* = true */)
 {
   nsTArray<RefPtr<CachedSurface>> discard;
   LookupResult rv(MatchType::NOT_FOUND);
@@ -1527,7 +1530,7 @@ SurfaceCache::Lookup(const ImageKey         aImageKey,
       return rv;
     }
 
-    rv = sInstance->Lookup(aImageKey, aSurfaceKey, lock);
+    rv = sInstance->Lookup(aImageKey, aSurfaceKey, lock, aMarkUsed);
     sInstance->TakeDiscard(discard, lock);
   }
 
@@ -1536,7 +1539,8 @@ SurfaceCache::Lookup(const ImageKey         aImageKey,
 
 /* static */ LookupResult
 SurfaceCache::LookupBestMatch(const ImageKey         aImageKey,
-                              const SurfaceKey&      aSurfaceKey)
+                              const SurfaceKey&      aSurfaceKey,
+                              bool aMarkUsed /* = true */)
 {
   nsTArray<RefPtr<CachedSurface>> discard;
   LookupResult rv(MatchType::NOT_FOUND);
@@ -1547,7 +1551,7 @@ SurfaceCache::LookupBestMatch(const ImageKey         aImageKey,
       return rv;
     }
 
-    rv = sInstance->LookupBestMatch(aImageKey, aSurfaceKey, lock);
+    rv = sInstance->LookupBestMatch(aImageKey, aSurfaceKey, lock, aMarkUsed);
     sInstance->TakeDiscard(discard, lock);
   }
 

image/SurfaceCache.h

@@ -232,7 +232,8 @@ struct SurfaceCache
    *                        if the cache entry was found.
    */
   static LookupResult Lookup(const ImageKey    aImageKey,
-                             const SurfaceKey& aSurfaceKey);
+                             const SurfaceKey& aSurfaceKey,
+                             bool aMarkUsed = true);
 
   /**
    * Looks up the best matching cache entry and returns a drawable reference to
@@ -251,7 +252,8 @@ struct SurfaceCache
    *                        returned surface exactly matches @aSurfaceKey.
    */
   static LookupResult LookupBestMatch(const ImageKey    aImageKey,
-                                      const SurfaceKey& aSurfaceKey);
+                                      const SurfaceKey& aSurfaceKey,
+                                      bool aMarkUsed = true);
 
   /**
    * Insert an ISurfaceProvider into the cache. If an entry with the same