Bug 1251091. Fix surface key comparison in ImageSurfaceCache::LookupBestMatch. r=dholbert
authorTimothy Nikkel <tnikkel@gmail.com>
Mon, 29 Feb 2016 12:20:50 -0600
changeset 286133 921b59dcd782e0e9803f9dfb542a9c2820772f61
parent 286132 b7769b2dc2bd9d06f67c5f8c1121d3d29d5acc6a
child 286134 3078ede85d93d149bd4c3d13fe2815b5db812902
push id30039
push usercbook@mozilla.com
push dateTue, 01 Mar 2016 11:02:11 +0000
treeherdermozilla-central@5cafa6f3019b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1251091, 1186796
milestone47.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 1251091. Fix surface key comparison in ImageSurfaceCache::LookupBestMatch. r=dholbert http://hg.mozilla.org/mozilla-central/rev/411f18fdffeb (bug 1186796) had a mistake in it. It changed ImageSurfaceCache::LookupBestMatch to use a for loop instead of using a callback to iterate each entry of the hashtable. The callback was called with the surface key of its entry, and it used the name |aSurfaceKey| for that key. ImageSurfaceCache::LookupBestMatch uses the name |aSurfaceKey| for the key we are looking for. So when the code from the callback was moved into the for loop in ImageSurfaceCache::LookupBestMatch the meaning of |aSurfaceKey| changed, but the code was not updated.
image/SurfaceCache.cpp
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -275,82 +275,82 @@ public:
   already_AddRefed<CachedSurface> Lookup(const SurfaceKey& aSurfaceKey)
   {
     RefPtr<CachedSurface> surface;
     mSurfaces.Get(aSurfaceKey, getter_AddRefs(surface));
     return surface.forget();
   }
 
   Pair<already_AddRefed<CachedSurface>, MatchType>
-  LookupBestMatch(const SurfaceKey& aSurfaceKey)
+  LookupBestMatch(const SurfaceKey& aIdealKey)
   {
     // Try for an exact match first.
     RefPtr<CachedSurface> exactMatch;
-    mSurfaces.Get(aSurfaceKey, getter_AddRefs(exactMatch));
+    mSurfaces.Get(aIdealKey, getter_AddRefs(exactMatch));
     if (exactMatch && exactMatch->IsDecoded()) {
       return MakePair(exactMatch.forget(), MatchType::EXACT);
     }
 
     // There's no perfect match, so find the best match we can.
     RefPtr<CachedSurface> bestMatch;
     for (auto iter = ConstIter(); !iter.Done(); iter.Next()) {
-      CachedSurface* surface = iter.UserData();
-      const SurfaceKey& idealKey = aSurfaceKey;
+      CachedSurface* current = iter.UserData();
+      const SurfaceKey& currentKey = current->GetSurfaceKey();
 
       // We never match a placeholder.
-      if (surface->IsPlaceholder()) {
+      if (current->IsPlaceholder()) {
         continue;
       }
       // Matching the animation time and SVG context is required.
-      if (aSurfaceKey.AnimationTime() != idealKey.AnimationTime() ||
-          aSurfaceKey.SVGContext() != idealKey.SVGContext()) {
+      if (currentKey.AnimationTime() != aIdealKey.AnimationTime() ||
+          currentKey.SVGContext() != aIdealKey.SVGContext()) {
         continue;
       }
       // Matching the flags is required.
-      if (aSurfaceKey.Flags() != idealKey.Flags()) {
+      if (currentKey.Flags() != aIdealKey.Flags()) {
         continue;
       }
       // Anything is better than nothing! (Within the constraints we just
       // checked, of course.)
       if (!bestMatch) {
-        bestMatch = surface;
+        bestMatch = current;
         continue;
       }
 
       MOZ_ASSERT(bestMatch, "Should have a current best match");
 
       // Always prefer completely decoded surfaces.
       bool bestMatchIsDecoded = bestMatch->IsDecoded();
-      if (bestMatchIsDecoded && !surface->IsDecoded()) {
+      if (bestMatchIsDecoded && !current->IsDecoded()) {
         continue;
       }
-      if (!bestMatchIsDecoded && surface->IsDecoded()) {
-        bestMatch = surface;
+      if (!bestMatchIsDecoded && current->IsDecoded()) {
+        bestMatch = current;
         continue;
       }
 
       SurfaceKey bestMatchKey = bestMatch->GetSurfaceKey();
 
       // Compare sizes. We use an area-based heuristic here instead of computing a
       // truly optimal answer, since it seems very unlikely to make a difference
       // for realistic sizes.
-      int64_t idealArea = AreaOfIntSize(idealKey.Size());
-      int64_t surfaceArea = AreaOfIntSize(aSurfaceKey.Size());
+      int64_t idealArea = AreaOfIntSize(aIdealKey.Size());
+      int64_t currentArea = AreaOfIntSize(currentKey.Size());
       int64_t bestMatchArea = AreaOfIntSize(bestMatchKey.Size());
 
       // If the best match is smaller than the ideal size, prefer bigger sizes.
       if (bestMatchArea < idealArea) {
-        if (surfaceArea > bestMatchArea) {
-          bestMatch = surface;
+        if (currentArea > bestMatchArea) {
+          bestMatch = current;
         }
         continue;
       }
       // Other, prefer sizes closer to the ideal size, but still not smaller.
-      if (idealArea <= surfaceArea && surfaceArea < bestMatchArea) {
-        bestMatch = surface;
+      if (idealArea <= currentArea && currentArea < bestMatchArea) {
+        bestMatch = current;
         continue;
       }
       // This surface isn't an improvement over the current best match.
     }
 
     MatchType matchType;
     if (bestMatch) {
       if (!exactMatch) {