From f811aad810a7f75d8324a552139081397d5807e3 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Sun, 19 Jul 2015 18:39:40 -0700 Subject: [PATCH] Bug 1176124 (Part 1) - Add a MatchType enum to LookupResult to let Lookup*() return more detailed information. r=dholbert --- image/FrameAnimator.cpp | 3 +-- image/LookupResult.h | 43 ++++++++++++++++++++++++++++++----------- image/RasterImage.cpp | 5 +++-- image/SurfaceCache.cpp | 26 ++++++++++++++++--------- 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp index 6cfb100013aa6..942c6a142d6fa 100644 --- a/image/FrameAnimator.cpp +++ b/image/FrameAnimator.cpp @@ -273,8 +273,7 @@ FrameAnimator::GetCompositedFrame(uint32_t aFrameNum) // 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 diff --git a/image/LookupResult.h b/image/LookupResult.h index 91fb2b4033ae9..67aebf576fe96 100644 --- a/image/LookupResult.h +++ b/image/LookupResult.h @@ -18,6 +18,16 @@ 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. @@ -25,25 +35,36 @@ namespace image { 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; } @@ -53,14 +74,14 @@ class MOZ_STACK_CLASS LookupResult /// @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 diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 01b238a8a8661..85e541fd992e3 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -526,8 +526,9 @@ RasterImage::LookupFrame(uint32_t aFrameNum, 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); diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp index c6c6c2b9827ca..c2206d62f77c8 100644 --- a/image/SurfaceCache.cpp +++ b/image/SurfaceCache.cpp @@ -550,12 +550,14 @@ class SurfaceCacheImpl final : public nsIMemoryReporter { nsRefPtr 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 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(); @@ -563,14 +565,16 @@ class SurfaceCacheImpl final : public nsIMemoryReporter // 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, @@ -579,7 +583,8 @@ class SurfaceCacheImpl final : public nsIMemoryReporter { nsRefPtr 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 @@ -593,7 +598,8 @@ class SurfaceCacheImpl final : public nsIMemoryReporter 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(); @@ -619,7 +625,9 @@ class SurfaceCacheImpl final : public nsIMemoryReporter 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, @@ -992,7 +1000,7 @@ SurfaceCache::Lookup(const ImageKey aImageKey, const Maybe& aAlternateFlags /* = Nothing() */) { if (!sInstance) { - return LookupResult(); + return LookupResult(MatchType::NOT_FOUND); } MutexAutoLock lock(sInstance->GetMutex()); @@ -1013,7 +1021,7 @@ SurfaceCache::LookupBestMatch(const ImageKey aImageKey, /* = Nothing() */) { if (!sInstance) { - return LookupResult(); + return LookupResult(MatchType::NOT_FOUND); } MutexAutoLock lock(sInstance->GetMutex());