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 253634 6747b7f35dcb1e2ce70251e31080a3abba2115bd
parent 253633 108ef3a1bdbd36510127bb24c04f8fcb4f98e32f
child 253635 f52a8f3b15ed32a3c9084133b0fe44374effeaf7
push id29072
push usercbook@mozilla.com
push dateMon, 20 Jul 2015 09:36:58 +0000
treeherdermozilla-central@5df788c56ae7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1176124
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 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,