Bug 1176124 (Part 1) - Add a MatchType enum to LookupResult to let Lookup*() return more detailed information. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Sun, 19 Jul 2015 18:39:40 -0700
changeset 279536 6747b7f35dcb1e2ce70251e31080a3abba2115bd
parent 279535 108ef3a1bdbd36510127bb24c04f8fcb4f98e32f
child 279537 f52a8f3b15ed32a3c9084133b0fe44374effeaf7
push id3616
push userjames@hoppipolla.co.uk
push dateMon, 20 Jul 2015 09:40:39 +0000
reviewersdholbert
bugs1176124
milestone42.0a1
Bug 1176124 (Part 1) - Add a MatchType enum to LookupResult to let Lookup*() return more detailed information. r=dholbert
image/FrameAnimator.cpp
image/LookupResult.h
image/RasterImage.cpp
image/SurfaceCache.cpp
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -268,18 +268,17 @@ FrameAnimator::GetFirstFrameRefreshArea(
 
 LookupResult
 FrameAnimator::GetCompositedFrame(uint32_t aFrameNum)
 {
   MOZ_ASSERT(aFrameNum != 0, "First frame is never composited");
 
   // If we have a composited version of this frame, return that.
   if (mLastCompositedFrameIndex == int32_t(aFrameNum)) {
-    return LookupResult(mCompositingFrame->DrawableRef(),
-                        /* aIsExactMatch = */ true);
+    return LookupResult(mCompositingFrame->DrawableRef(), MatchType::EXACT);
   }
 
   // Otherwise return the raw frame. DoBlend is required to ensure that we only
   // hit this case if the frame is not paletted and doesn't require compositing.
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           0,  // Default decode flags.
--- a/image/LookupResult.h
+++ b/image/LookupResult.h
@@ -13,57 +13,78 @@
 
 #include "mozilla/Attributes.h"
 #include "mozilla/Move.h"
 #include "imgFrame.h"
 
 namespace mozilla {
 namespace image {
 
+enum class MatchType : uint8_t
+{
+  NOT_FOUND,  // No matching surface and no placeholder.
+  PENDING,    // Found a matching placeholder, but no surface.
+  EXACT,      // Found a surface that matches exactly.
+  SUBSTITUTE_BECAUSE_NOT_FOUND,  // No exact match, but found a similar one.
+  SUBSTITUTE_BECAUSE_PENDING     // Found a similar surface and a placeholder
+                                 // for an exact match.
+};
+
 /**
  * LookupResult is the return type of SurfaceCache's Lookup*() functions. It
  * combines a surface with relevant metadata tracked by SurfaceCache.
  */
 class MOZ_STACK_CLASS LookupResult
 {
 public:
-  LookupResult()
-    : mIsExactMatch(false)
-  { }
+  explicit LookupResult(MatchType aMatchType)
+    : mMatchType(aMatchType)
+  {
+    MOZ_ASSERT(mMatchType == MatchType::NOT_FOUND ||
+               mMatchType == MatchType::PENDING,
+               "Only NOT_FOUND or PENDING make sense with no surface");
+  }
 
   LookupResult(LookupResult&& aOther)
     : mDrawableRef(Move(aOther.mDrawableRef))
-    , mIsExactMatch(aOther.mIsExactMatch)
+    , mMatchType(aOther.mMatchType)
   { }
 
-  LookupResult(DrawableFrameRef&& aDrawableRef, bool aIsExactMatch)
+  LookupResult(DrawableFrameRef&& aDrawableRef, MatchType aMatchType)
     : mDrawableRef(Move(aDrawableRef))
-    , mIsExactMatch(aIsExactMatch)
-  { }
+    , mMatchType(aMatchType)
+  {
+    MOZ_ASSERT(!mDrawableRef || !(mMatchType == MatchType::NOT_FOUND ||
+                                  mMatchType == MatchType::PENDING),
+               "Only NOT_FOUND or PENDING make sense with no surface");
+    MOZ_ASSERT(mDrawableRef || mMatchType == MatchType::NOT_FOUND ||
+                               mMatchType == MatchType::PENDING,
+               "NOT_FOUND or PENDING do not make sense with a surface");
+  }
 
   LookupResult& operator=(LookupResult&& aOther)
   {
     MOZ_ASSERT(&aOther != this, "Self-move-assignment is not supported");
     mDrawableRef = Move(aOther.mDrawableRef);
-    mIsExactMatch = aOther.mIsExactMatch;
+    mMatchType = aOther.mMatchType;
     return *this;
   }
 
   DrawableFrameRef& DrawableRef() { return mDrawableRef; }
   const DrawableFrameRef& DrawableRef() const { return mDrawableRef; }
 
   /// @return true if this LookupResult contains a surface.
   explicit operator bool() const { return bool(mDrawableRef); }
 
-  /// @return true if the surface is an exact match for the Lookup*() arguments.
-  bool IsExactMatch() const { return mIsExactMatch; }
+  /// @return what kind of match this is (exact, substitute, etc.)
+  MatchType Type() const { return mMatchType; }
 
 private:
   LookupResult(const LookupResult&) = delete;
 
   DrawableFrameRef mDrawableRef;
-  bool mIsExactMatch;
+  MatchType mMatchType;
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_LookupResult_h
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -521,18 +521,19 @@ RasterImage::LookupFrame(uint32_t aFrame
 
   LookupResult result = LookupFrameInternal(aFrameNum, requestedSize, aFlags);
 
   if (!result && !mHasSize) {
     // We can't request a decode without knowing our intrinsic size. Give up.
     return DrawableFrameRef();
   }
 
-  if (!result || !result.IsExactMatch()) {
-    // The OS threw this frame away. We need to redecode if we can.
+  if (!result || result.Type() == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND) {
+    // We don't have a copy of this frame, and there's no decoder working on
+    // one. Trigger decoding so it'll be available next time.
     MOZ_ASSERT(!mAnim, "Animated frames should be locked");
 
     Decode(Some(requestedSize), aFlags);
 
     // If we can sync decode, we should already have the frame.
     if (aFlags & FLAG_SYNC_DECODE) {
       result = LookupFrameInternal(aFrameNum, requestedSize, aFlags);
     }
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -545,60 +545,66 @@ public:
   }
 
   LookupResult Lookup(const ImageKey    aImageKey,
                       const SurfaceKey& aSurfaceKey,
                       bool aMarkUsed = true)
   {
     nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
-      return LookupResult();  // No cached surfaces for this image.
+      // No cached surfaces for this image.
+      return LookupResult(MatchType::NOT_FOUND);
     }
 
     nsRefPtr<CachedSurface> surface = cache->Lookup(aSurfaceKey);
     if (!surface) {
-      return LookupResult();  // Lookup in the per-image cache missed.
+      // Lookup in the per-image cache missed.
+      return LookupResult(MatchType::NOT_FOUND);
     }
 
     DrawableFrameRef ref = surface->DrawableRef();
     if (!ref) {
       // The surface was released by the operating system. Remove the cache
       // entry as well.
       Remove(surface);
-      return LookupResult();
+      return LookupResult(MatchType::NOT_FOUND);
     }
 
     if (aMarkUsed) {
       MarkUsed(surface, cache);
     }
 
-    return LookupResult(Move(ref), /* aIsExactMatch = */ true);
+    MOZ_ASSERT(surface->GetSurfaceKey() == aSurfaceKey,
+               "Lookup() not returning an exact match?");
+    return LookupResult(Move(ref), MatchType::EXACT);
   }
 
   LookupResult LookupBestMatch(const ImageKey         aImageKey,
                                const SurfaceKey&      aSurfaceKey,
                                const Maybe<uint32_t>& aAlternateFlags)
   {
     nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
-      return LookupResult();  // No cached surfaces for this image.
+      // No cached surfaces for this image.
+      return LookupResult(MatchType::NOT_FOUND);
     }
 
     // Repeatedly look up the best match, trying again if the resulting surface
     // has been freed by the operating system, until we can either lock a
     // surface for drawing or there are no matching surfaces left.
     // XXX(seth): This is O(N^2), but N is expected to be very small. If we
     // encounter a performance problem here we can revisit this.
 
     nsRefPtr<CachedSurface> surface;
     DrawableFrameRef ref;
     while (true) {
       surface = cache->LookupBestMatch(aSurfaceKey, aAlternateFlags);
       if (!surface) {
-        return LookupResult();  // Lookup in the per-image cache missed.
+        // Lookup in the per-image cache missed.
+        return LookupResult(MatchType::NOT_FOUND);
       }
 
       ref = surface->DrawableRef();
       if (ref) {
         break;
       }
 
       // The surface was released by the operating system. Remove the cache
@@ -614,17 +620,19 @@ public:
          (aAlternateFlags && key == aSurfaceKey.WithNewFlags(*aAlternateFlags))),
       "Result differs in a way other than size or alternate flags");
 
 
     if (isExactMatch) {
       MarkUsed(surface, cache);
     }
 
-    return LookupResult(Move(ref), isExactMatch);
+    MatchType matchType = isExactMatch ? MatchType::EXACT
+                                       : MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
+    return LookupResult(Move(ref), matchType);
   }
 
   void RemoveSurface(const ImageKey    aImageKey,
                      const SurfaceKey& aSurfaceKey)
   {
     nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       return;  // No cached surfaces for this image.
@@ -987,17 +995,17 @@ SurfaceCache::Shutdown()
 }
 
 /* static */ LookupResult
 SurfaceCache::Lookup(const ImageKey         aImageKey,
                      const SurfaceKey&      aSurfaceKey,
                      const Maybe<uint32_t>& aAlternateFlags /* = Nothing() */)
 {
   if (!sInstance) {
-    return LookupResult();
+    return LookupResult(MatchType::NOT_FOUND);
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
 
   LookupResult result = sInstance->Lookup(aImageKey, aSurfaceKey);
   if (!result && aAlternateFlags) {
     result = sInstance->Lookup(aImageKey,
                                aSurfaceKey.WithNewFlags(*aAlternateFlags));
@@ -1008,17 +1016,17 @@ SurfaceCache::Lookup(const ImageKey     
 
 /* static */ LookupResult
 SurfaceCache::LookupBestMatch(const ImageKey         aImageKey,
                               const SurfaceKey&      aSurfaceKey,
                               const Maybe<uint32_t>& aAlternateFlags
                                 /* = Nothing() */)
 {
   if (!sInstance) {
-    return LookupResult();
+    return LookupResult(MatchType::NOT_FOUND);
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
   return sInstance->LookupBestMatch(aImageKey, aSurfaceKey, aAlternateFlags);
 }
 
 /* static */ InsertOutcome
 SurfaceCache::Insert(imgFrame*         aSurface,