Bug 1251091. Fix surface key comparison in ImageSurfaceCache::LookupBestMatch. r=dholbert a=lizzard
authorTimothy Nikkel <tnikkel@gmail.com>
Mon, 29 Feb 2016 12:20:50 -0600
changeset 304425 821f5cfb92c5c1228051846ad72320494e10c176
parent 304424 e1c37c0fc9350ddfe8bc704faae7d59154e70378
child 304426 1de2a92b100497424d44d5a523f4dc57574c1710
push id9198
push userkwierso@gmail.com
push dateSat, 05 Mar 2016 01:10:06 +0000
treeherdermozilla-aurora@20fd8603e83d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, lizzard
bugs1251091, 1186796
milestone46.0a2
Bug 1251091. Fix surface key comparison in ImageSurfaceCache::LookupBestMatch. r=dholbert a=lizzard 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. MozReview-Commit-ID: 3dVs7YkRSjC
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) {