diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp index 856ba162dc..d9bf4614d3 100644 --- a/image/SurfaceCache.cpp +++ b/image/SurfaceCache.cpp @@ -251,12 +251,14 @@ public: mSurfaces.Put(aSurface->GetSurfaceKey(), aSurface); } - void Remove(NotNull aSurface) + already_AddRefed Remove(NotNull aSurface) { MOZ_ASSERT(mSurfaces.GetWeak(aSurface->GetSurfaceKey()), "Should not be removing a surface we don't have"); - mSurfaces.Remove(aSurface->GetSurfaceKey()); + RefPtr surface; + mSurfaces.Remove(aSurface->GetSurfaceKey(), getter_AddRefs(surface)); + return surface.forget(); } already_AddRefed Lookup(const SurfaceKey& aSurfaceKey) @@ -507,10 +509,14 @@ public: } StopTracking(aSurface, aAutoLock); - cache->Remove(aSurface); - // Remove the per-image cache if it's unneeded now. (Keep it if the image is - // locked, since the per-image cache is where we store that state.) + // Individual surfaces must be freed outside the lock. + mCachedSurfacesDiscard.AppendElement(cache->Remove(aSurface)); + + // Remove the per-image cache if it's unneeded now. Keep it if the image is + // locked, since the per-image cache is where we store that state. Note that + // we don't push it into mImageCachesDiscard because all of its surfaces + // have been removed, so it is safe to free while holding the lock. if (cache->IsEmpty() && !cache->IsLocked()) { mImageCaches.Remove(imageKey); } @@ -719,11 +725,12 @@ public: DoUnlockSurfaces(WrapNotNull(cache), aAutoLock); } - void RemoveImage(const ImageKey aImageKey, const StaticMutexAutoLock& aAutoLock) + already_AddRefed + RemoveImage(const ImageKey aImageKey, const StaticMutexAutoLock& aAutoLock) { RefPtr cache = GetImageCache(aImageKey); if (!cache) { - return; // No cached surfaces for this image, so nothing to do. + return nullptr; // No cached surfaces for this image, so nothing to do. } // Discard all of the cached surfaces for this image. @@ -738,6 +745,10 @@ public: // The per-image cache isn't needed anymore, so remove it as well. // This implicitly unlocks the image if it was locked. mImageCaches.Remove(aImageKey); + + // Since we did not actually remove any of the surfaces from the cache + // itself, only stopped tracking them, we should free it outside the lock. + return cache.forget(); } void DiscardAll(const StaticMutexAutoLock& aAutoLock) @@ -776,6 +787,13 @@ public: } } + void TakeDiscard(nsTArray>& aDiscard, + const StaticMutexAutoLock& aAutoLock) + { + MOZ_ASSERT(aDiscard.IsEmpty()); + aDiscard = Move(mCachedSurfacesDiscard); + } + void LockSurface(NotNull aSurface, const StaticMutexAutoLock& aAutoLock) { @@ -922,10 +940,12 @@ private: Remove(WrapNotNull(surface), aAutoLock); } - struct SurfaceTracker : public ExpirationTrackerImpl + class SurfaceTracker final : + public ExpirationTrackerImpl { + public: explicit SurfaceTracker(uint32_t aSurfaceCacheExpirationTimeMS) : ExpirationTrackerImpl( @@ -939,23 +959,40 @@ private: sInstance->Remove(WrapNotNull(aSurface), aAutoLock); } + void NotifyHandlerEndLocked(const StaticMutexAutoLock& aAutoLock) override + { + sInstance->TakeDiscard(mDiscard, aAutoLock); + } + + void NotifyHandlerEnd() override + { + nsTArray> discard(Move(mDiscard)); + } + StaticMutex& GetMutex() override { return sInstanceMutex; } + + nsTArray> mDiscard; }; - struct MemoryPressureObserver : public nsIObserver + class MemoryPressureObserver final : public nsIObserver { + public: NS_DECL_ISUPPORTS NS_IMETHOD Observe(nsISupports*, const char* aTopic, const char16_t*) override { - StaticMutexAutoLock lock(sInstanceMutex); - if (sInstance && strcmp(aTopic, "memory-pressure") == 0) { - sInstance->DiscardForMemoryPressure(lock); + nsTArray> discard; + { + StaticMutexAutoLock lock(sInstanceMutex); + if (sInstance && strcmp(aTopic, "memory-pressure") == 0) { + sInstance->DiscardForMemoryPressure(lock); + sInstance->TakeDiscard(discard, lock); + } } return NS_OK; } @@ -967,6 +1004,7 @@ private: nsTArray mCosts; nsRefPtrHashtable, ImageSurfaceCache> mImageCaches; + nsTArray> mCachedSurfacesDiscard; SurfaceTracker mExpirationTracker; RefPtr mMemoryPressureObserver; nsTArray> mReleasingImagesOnMainThread; @@ -1043,45 +1081,72 @@ SurfaceCache::Initialize() /* static */ void SurfaceCache::Shutdown() { - StaticMutexAutoLock lock(sInstanceMutex); - MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(sInstance, "No singleton - was Shutdown() called twice?"); - sInstance = nullptr; + RefPtr cache; + { + StaticMutexAutoLock lock(sInstanceMutex); + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(sInstance, "No singleton - was Shutdown() called twice?"); + cache = sInstance.forget(); + } } /* static */ LookupResult SurfaceCache::Lookup(const ImageKey aImageKey, const SurfaceKey& aSurfaceKey) { - StaticMutexAutoLock lock(sInstanceMutex); - if (!sInstance) { - return LookupResult(MatchType::NOT_FOUND); + nsTArray> discard; + LookupResult rv(MatchType::NOT_FOUND); + + { + StaticMutexAutoLock lock(sInstanceMutex); + if (!sInstance) { + return rv; + } + + rv = sInstance->Lookup(aImageKey, aSurfaceKey, lock); + sInstance->TakeDiscard(discard, lock); } - return sInstance->Lookup(aImageKey, aSurfaceKey, lock); + return rv; } /* static */ LookupResult SurfaceCache::LookupBestMatch(const ImageKey aImageKey, const SurfaceKey& aSurfaceKey) { - StaticMutexAutoLock lock(sInstanceMutex); - if (!sInstance) { - return LookupResult(MatchType::NOT_FOUND); + nsTArray> discard; + LookupResult rv(MatchType::NOT_FOUND); + + { + StaticMutexAutoLock lock(sInstanceMutex); + if (!sInstance) { + return rv; + } + + rv = sInstance->LookupBestMatch(aImageKey, aSurfaceKey, lock); + sInstance->TakeDiscard(discard, lock); } - return sInstance->LookupBestMatch(aImageKey, aSurfaceKey, lock); + return rv; } /* static */ InsertOutcome SurfaceCache::Insert(NotNull aProvider) { - StaticMutexAutoLock lock(sInstanceMutex); - if (!sInstance) { - return InsertOutcome::FAILURE; + nsTArray> discard; + InsertOutcome rv(InsertOutcome::FAILURE); + + { + StaticMutexAutoLock lock(sInstanceMutex); + if (!sInstance) { + return rv; + } + + rv = sInstance->Insert(aProvider, /* aSetAvailable = */ false, lock); + sInstance->TakeDiscard(discard, lock); } - return sInstance->Insert(aProvider, /* aSetAvailable = */ false, lock); + return rv; } /* static */ bool @@ -1148,18 +1213,25 @@ SurfaceCache::UnlockEntries(const ImageKey aImageKey) /* static */ void SurfaceCache::RemoveImage(const ImageKey aImageKey) { - StaticMutexAutoLock lock(sInstanceMutex); - if (sInstance) { - sInstance->RemoveImage(aImageKey, lock); + RefPtr discard; + { + StaticMutexAutoLock lock(sInstanceMutex); + if (sInstance) { + discard = sInstance->RemoveImage(aImageKey, lock); + } } } /* static */ void SurfaceCache::DiscardAll() { - StaticMutexAutoLock lock(sInstanceMutex); - if (sInstance) { - sInstance->DiscardAll(lock); + nsTArray> discard; + { + StaticMutexAutoLock lock(sInstanceMutex); + if (sInstance) { + sInstance->DiscardAll(lock); + sInstance->TakeDiscard(discard, lock); + } } }