From d9107cf87421a8a20c707606dda4808d5059a721 Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Mon, 29 Feb 2016 12:20:50 -0600 Subject: [PATCH] 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 | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp index 44ed92c6b19..e1446e84e5a 100644 --- a/image/SurfaceCache.cpp +++ b/image/SurfaceCache.cpp @@ -280,11 +280,11 @@ public: } Pair, MatchType> - LookupBestMatch(const SurfaceKey& aSurfaceKey) + LookupBestMatch(const SurfaceKey& aIdealKey) { // Try for an exact match first. RefPtr exactMatch; - mSurfaces.Get(aSurfaceKey, getter_AddRefs(exactMatch)); + mSurfaces.Get(aIdealKey, getter_AddRefs(exactMatch)); if (exactMatch && exactMatch->IsDecoded()) { return MakePair(exactMatch.forget(), MatchType::EXACT); } @@ -292,26 +292,26 @@ public: // There's no perfect match, so find the best match we can. RefPtr 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; } @@ -319,11 +319,11 @@ public: // 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; } @@ -332,20 +332,20 @@ public: // 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.