Bug 1368776 - Part 16. Ensure we more consistently pass the suggested size from SurfaceCache::LookupBestMatch. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 17 Nov 2017 06:45:28 -0500
changeset 446749 8d8cfa078d8601abcb66e1edb786e971f929ad89
parent 446748 b9a29d94ccac646c9336fa75e084bbc8581501ad
child 446750 65629d4e95312091679d52b319ff531d0b3be91e
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1368776
milestone59.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 1368776 - Part 16. Ensure we more consistently pass the suggested size from SurfaceCache::LookupBestMatch. r=tnikkel The suggested size is useful in more situations now that GetImageContainerImpl requires it. It should be passed whenever we have it available.
image/LookupResult.h
image/SurfaceCache.cpp
--- a/image/LookupResult.h
+++ b/image/LookupResult.h
@@ -49,16 +49,17 @@ public:
     MOZ_ASSERT(mMatchType == MatchType::NOT_FOUND ||
                mMatchType == MatchType::PENDING,
                "Only NOT_FOUND or PENDING make sense with no surface");
   }
 
   LookupResult(LookupResult&& aOther)
     : mSurface(Move(aOther.mSurface))
     , mMatchType(aOther.mMatchType)
+    , mSuggestedSize(aOther.mSuggestedSize)
   { }
 
   LookupResult(DrawableSurface&& aSurface, MatchType aMatchType)
     : mSurface(Move(aSurface))
     , mMatchType(aMatchType)
   {
     MOZ_ASSERT(!mSurface || !(mMatchType == MatchType::NOT_FOUND ||
                               mMatchType == MatchType::PENDING),
@@ -69,21 +70,22 @@ public:
   }
 
   LookupResult(DrawableSurface&& aSurface, MatchType aMatchType,
                const gfx::IntSize& aSuggestedSize)
     : mSurface(Move(aSurface))
     , mMatchType(aMatchType)
     , mSuggestedSize(aSuggestedSize)
   {
-    MOZ_ASSERT(!mSuggestedSize.IsEmpty());
-    MOZ_ASSERT(!mSurface || aMatchType == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND,
-               "Only SUBSTITUTE_BECAUSE_NOT_FOUND make sense with no surface");
-    MOZ_ASSERT(mSurface || aMatchType == MatchType::NOT_FOUND,
-               "NOT_FOUND does not make sense with a surface");
+    MOZ_ASSERT(!mSurface || !(mMatchType == MatchType::NOT_FOUND ||
+                              mMatchType == MatchType::PENDING),
+               "Only NOT_FOUND or PENDING make sense with no surface");
+    MOZ_ASSERT(mSurface || 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");
     mSurface = Move(aOther.mSurface);
     mMatchType = aOther.mMatchType;
     mSuggestedSize = aOther.mSuggestedSize;
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -330,16 +330,17 @@ public:
 
     // Try for a best match second, if using compact.
     IntSize suggestedSize = SuggestedSize(aIdealKey.Size());
     if (mFactor2Mode) {
       if (!exactMatch) {
         SurfaceKey compactKey = aIdealKey.CloneWithSize(suggestedSize);
         mSurfaces.Get(compactKey, getter_AddRefs(exactMatch));
         if (exactMatch && exactMatch->IsDecoded()) {
+          MOZ_ASSERT(suggestedSize != aIdealKey.Size());
           return MakeTuple(exactMatch.forget(),
                            MatchType::SUBSTITUTE_BECAUSE_BEST,
                            suggestedSize);
         }
       }
     }
 
     // There's no perfect match, so find the best match we can.
@@ -385,36 +386,31 @@ public:
                       currentKey.Size())) {
         bestMatch = current;
       }
     }
 
     MatchType matchType;
     if (bestMatch) {
       if (!exactMatch) {
-        const IntSize& bestMatchSize = bestMatch->GetSurfaceKey().Size();
-        if (mFactor2Mode && suggestedSize == bestMatchSize) {
-          // No exact match, but this is the best we are willing to decode.
-          matchType = MatchType::SUBSTITUTE_BECAUSE_BEST;
-        } else {
-          // No exact match, but we found a substitute.
-          matchType = MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
-        }
+        // No exact match, neither ideal nor factor of 2.
+        MOZ_ASSERT(suggestedSize != bestMatch->GetSurfaceKey().Size(),
+                   "No exact match despite the fact the sizes match!");
+        matchType = MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
       } else if (exactMatch != bestMatch) {
         // The exact match is still decoding, but we found a substitute.
         matchType = MatchType::SUBSTITUTE_BECAUSE_PENDING;
+      } else if (aIdealKey.Size() != bestMatch->GetSurfaceKey().Size()) {
+        // The best factor of 2 match is still decoding, but the best we've got.
+        MOZ_ASSERT(suggestedSize != aIdealKey.Size());
+        MOZ_ASSERT(mFactor2Mode);
+        matchType = MatchType::SUBSTITUTE_BECAUSE_BEST;
       } else {
-        const IntSize& bestMatchSize = bestMatch->GetSurfaceKey().Size();
-        if (mFactor2Mode && aIdealKey.Size() != bestMatchSize) {
-          // The best factor of 2 match is still decoding.
-          matchType = MatchType::SUBSTITUTE_BECAUSE_BEST;
-        } else {
-          // The exact match is still decoding, but it's the best we've got.
-          matchType = MatchType::EXACT;
-        }
+        // The exact match is still decoding, but it's the best we've got.
+        matchType = MatchType::EXACT;
       }
     } else {
       if (exactMatch) {
         // We found an "exact match"; it must have been a placeholder.
         MOZ_ASSERT(exactMatch->IsPlaceholder());
         matchType = MatchType::PENDING;
       } else {
         // We couldn't find an exact match *or* a substitute.
@@ -979,25 +975,17 @@ public:
 
     if (matchType == MatchType::EXACT ||
         matchType == MatchType::SUBSTITUTE_BECAUSE_BEST) {
       if (!MarkUsed(WrapNotNull(surface), WrapNotNull(cache), aAutoLock)) {
         Remove(WrapNotNull(surface), /* aStopTracking */ false, aAutoLock);
       }
     }
 
-    // When the caller may choose to decode at an uncached size because there is
-    // no pending decode at the requested size, we should give it the alternative
-    // size it should decode at.
-    if (matchType == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
-        matchType == MatchType::NOT_FOUND) {
-      return LookupResult(Move(drawableSurface), matchType, suggestedSize);
-    }
-
-    return LookupResult(Move(drawableSurface), matchType);
+    return LookupResult(Move(drawableSurface), matchType, suggestedSize);
   }
 
   bool CanHold(const Cost aCost) const
   {
     return aCost <= mMaxCost;
   }
 
   size_t MaximumCapacity() const