Bug 1296828 (Part 3) - Update SurfaceCache API to rely on ImageKeys and SurfaceKeys stored on ISurfaceProviders. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 19 Aug 2016 20:26:59 -0700
changeset 311563 d28b09df98ff4a1fa02b37542b99f7a6b5191071
parent 311562 8f8d3c106410aed2252be6180c542aef3b8ad738
child 311564 90c8439299ea297d780b0a5ec2d81468520437d0
push idunknown
push userunknown
push dateunknown
reviewersdholbert
bugs1296828
milestone51.0a1
Bug 1296828 (Part 3) - Update SurfaceCache API to rely on ImageKeys and SurfaceKeys stored on ISurfaceProviders. r=dholbert
image/AnimationSurfaceProvider.cpp
image/DecodedSurfaceProvider.cpp
image/DecoderFactory.cpp
image/ISurfaceProvider.h
image/SurfaceCache.cpp
image/SurfaceCache.h
image/VectorImage.cpp
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -248,19 +248,17 @@ AnimationSurfaceProvider::AnnounceSurfac
   mFramesMutex.AssertNotCurrentThreadOwns();
   MOZ_ASSERT(mImage);
 
   // We just got the first frame; let the surface cache know. We deliberately do
   // this outside of mFramesMutex to avoid a potential deadlock with
   // AddSizeOfExcludingThis(), since otherwise we'd be acquiring mFramesMutex
   // and then the surface cache lock, while the memory reporting code would
   // acquire the surface cache lock and then mFramesMutex.
-  SurfaceCache::SurfaceAvailable(WrapNotNull(this),
-                                 GetImageKey(),
-                                 GetSurfaceKey());
+  SurfaceCache::SurfaceAvailable(WrapNotNull(this));
 }
 
 void
 AnimationSurfaceProvider::FinishDecoding()
 {
   mDecodingMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mImage);
   MOZ_ASSERT(mDecoder);
--- a/image/DecodedSurfaceProvider.cpp
+++ b/image/DecodedSurfaceProvider.cpp
@@ -182,19 +182,17 @@ DecodedSurfaceProvider::CheckForNewSurfa
   // We don't have a surface yet; try to get one from the decoder.
   mSurface = mDecoder->GetCurrentFrameRef().get();
   if (!mSurface) {
     return;  // No surface yet.
   }
 
   // We just got a surface for the first time; let the surface cache know.
   MOZ_ASSERT(mImage);
-  SurfaceCache::SurfaceAvailable(WrapNotNull(this),
-                                 GetImageKey(),
-                                 GetSurfaceKey());
+  SurfaceCache::SurfaceAvailable(WrapNotNull(this));
 }
 
 void
 DecodedSurfaceProvider::FinishDecoding()
 {
   mMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mImage);
   MOZ_ASSERT(mDecoder);
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -145,19 +145,17 @@ DecoderFactory::CreateDecoder(DecoderTyp
     RasterSurfaceKey(aOutputSize, aSurfaceFlags, PlaybackType::eStatic);
   NotNull<RefPtr<DecodedSurfaceProvider>> provider =
     WrapNotNull(new DecodedSurfaceProvider(aImage,
                                            surfaceKey,
                                            WrapNotNull(decoder)));
 
   // Attempt to insert the surface provider into the surface cache right away so
   // we won't trigger any more decoders with the same parameters.
-  InsertOutcome outcome =
-    SurfaceCache::Insert(provider, ImageKey(aImage.get()), surfaceKey);
-  if (outcome != InsertOutcome::SUCCESS) {
+  if (SurfaceCache::Insert(provider) != InsertOutcome::SUCCESS) {
     return nullptr;
   }
 
   // Return the surface provider in its IDecodingTask guise.
   RefPtr<IDecodingTask> task = provider.get();
   return task.forget();
 }
 
@@ -197,19 +195,17 @@ DecoderFactory::CreateAnimationDecoder(D
     RasterSurfaceKey(aIntrinsicSize, aSurfaceFlags, PlaybackType::eAnimated);
   NotNull<RefPtr<AnimationSurfaceProvider>> provider =
     WrapNotNull(new AnimationSurfaceProvider(aImage,
                                              surfaceKey,
                                              WrapNotNull(decoder)));
 
   // Attempt to insert the surface provider into the surface cache right away so
   // we won't trigger any more decoders with the same parameters.
-  InsertOutcome outcome =
-    SurfaceCache::Insert(provider, ImageKey(aImage.get()), surfaceKey);
-  if (outcome != InsertOutcome::SUCCESS) {
+  if (SurfaceCache::Insert(provider) != InsertOutcome::SUCCESS) {
     return nullptr;
   }
 
   // Return the surface provider in its IDecodingTask guise.
   RefPtr<IDecodingTask> task = provider.get();
   return task.forget();
 }
 
--- a/image/ISurfaceProvider.h
+++ b/image/ISurfaceProvider.h
@@ -51,17 +51,18 @@ public:
   /// @return a (potentially lazily computed) drawable reference to a surface.
   virtual DrawableSurface Surface();
 
   /// @return true if DrawableRef() will return a completely decoded surface.
   virtual bool IsFinished() const = 0;
 
   /// @return the number of bytes of memory this ISurfaceProvider is expected to
   /// require. Optimizations may result in lower real memory usage. Trivial
-  /// overhead is ignored.
+  /// overhead is ignored. Because this value is used in bookkeeping, it's
+  /// important that it be constant over the lifetime of this object.
   virtual size_t LogicalSizeInBytes() const = 0;
 
   /// @return the actual number of bytes of memory this ISurfaceProvider is
   /// using. May vary over the lifetime of the ISurfaceProvider. The default
   /// implementation is appropriate for static ISurfaceProviders.
   virtual void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                       size_t& aHeapSizeOut,
                                       size_t& aNonHeapSizeOut)
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -119,27 +119,19 @@ private:
  */
 class CachedSurface
 {
   ~CachedSurface() { }
 public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(CachedSurface)
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CachedSurface)
 
-  CachedSurface(NotNull<ISurfaceProvider*> aProvider,
-                const Cost                 aCost,
-                const ImageKey             aImageKey,
-                const SurfaceKey&          aSurfaceKey)
+  explicit CachedSurface(NotNull<ISurfaceProvider*> aProvider)
     : mProvider(aProvider)
-    , mCost(aCost)
-    , mImageKey(aImageKey)
-    , mSurfaceKey(aSurfaceKey)
-  {
-    MOZ_ASSERT(mImageKey, "Must have a valid image key");
-  }
+  { }
 
   DrawableSurface GetDrawableSurface() const
   {
     if (MOZ_UNLIKELY(IsPlaceholder())) {
       MOZ_ASSERT_UNREACHABLE("Called GetDrawableSurface() on a placeholder");
       return DrawableSurface();
     }
 
@@ -154,21 +146,25 @@ public:
 
     mProvider->SetLocked(aLocked);
   }
 
   bool IsLocked() const { return !IsPlaceholder() && mProvider->IsLocked(); }
   bool IsPlaceholder() const { return mProvider->Availability().IsPlaceholder(); }
   bool IsDecoded() const { return !IsPlaceholder() && mProvider->IsFinished(); }
 
-  ImageKey GetImageKey() const { return mImageKey; }
-  SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
-  CostEntry GetCostEntry() { return image::CostEntry(WrapNotNull(this), mCost); }
+  ImageKey GetImageKey() const { return mProvider->GetImageKey(); }
+  SurfaceKey GetSurfaceKey() const { return mProvider->GetSurfaceKey(); }
   nsExpirationState* GetExpirationState() { return &mExpirationState; }
 
+  CostEntry GetCostEntry()
+  {
+    return image::CostEntry(WrapNotNull(this), mProvider->LogicalSizeInBytes());
+  }
+
   // A helper type used by SurfaceCacheImpl::CollectSizeOfSurfaces.
   struct MOZ_STACK_CLASS SurfaceMemoryReport
   {
     SurfaceMemoryReport(nsTArray<SurfaceMemoryCounter>& aCounters,
                         MallocSizeOf                    aMallocSizeOf)
       : mCounters(aCounters)
       , mMallocSizeOf(aMallocSizeOf)
     { }
@@ -199,19 +195,16 @@ public:
   private:
     nsTArray<SurfaceMemoryCounter>& mCounters;
     MallocSizeOf                    mMallocSizeOf;
   };
 
 private:
   nsExpirationState                 mExpirationState;
   NotNull<RefPtr<ISurfaceProvider>> mProvider;
-  const Cost                        mCost;
-  const ImageKey                    mImageKey;
-  const SurfaceKey                  mSurfaceKey;
 };
 
 static int64_t
 AreaOfIntSize(const IntSize& aSize) {
   return static_cast<int64_t>(aSize.width) * static_cast<int64_t>(aSize.height);
 }
 
 /**
@@ -232,21 +225,21 @@ public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(ImageSurfaceCache)
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ImageSurfaceCache)
 
   typedef
     nsRefPtrHashtable<nsGenericHashKey<SurfaceKey>, CachedSurface> SurfaceTable;
 
   bool IsEmpty() const { return mSurfaces.Count() == 0; }
 
-  void Insert(const SurfaceKey& aKey, NotNull<CachedSurface*> aSurface)
+  void Insert(NotNull<CachedSurface*> aSurface)
   {
     MOZ_ASSERT(!mLocked || aSurface->IsPlaceholder() || aSurface->IsLocked(),
                "Inserting an unlocked surface for a locked image");
-    mSurfaces.Put(aKey, aSurface);
+    mSurfaces.Put(aSurface->GetSurfaceKey(), aSurface);
   }
 
   void Remove(NotNull<CachedSurface*> aSurface)
   {
     MOZ_ASSERT(mSurfaces.GetWeak(aSurface->GetSurfaceKey()),
         "Should not be removing a surface we don't have");
 
     mSurfaces.Remove(aSurface->GetSurfaceKey());
@@ -412,81 +405,81 @@ private:
   }
 
 public:
   void InitMemoryReporter() { RegisterWeakMemoryReporter(this); }
 
   Mutex& GetMutex() { return mMutex; }
 
   InsertOutcome Insert(NotNull<ISurfaceProvider*> aProvider,
-                       const Cost                 aCost,
-                       const ImageKey             aImageKey,
-                       const SurfaceKey&          aSurfaceKey,
                        bool                       aSetAvailable)
   {
     // If this is a duplicate surface, refuse to replace the original.
     // XXX(seth): Calling Lookup() and then RemoveEntry() does the lookup
     // twice. We'll make this more efficient in bug 1185137.
-    LookupResult result = Lookup(aImageKey, aSurfaceKey, /* aMarkUsed = */ false);
+    LookupResult result = Lookup(aProvider->GetImageKey(),
+                                 aProvider->GetSurfaceKey(),
+                                 /* aMarkUsed = */ false);
     if (MOZ_UNLIKELY(result)) {
       return InsertOutcome::FAILURE_ALREADY_PRESENT;
     }
 
     if (result.Type() == MatchType::PENDING) {
-      RemoveEntry(aImageKey, aSurfaceKey);
+      RemoveEntry(aProvider->GetImageKey(), aProvider->GetSurfaceKey());
     }
 
     MOZ_ASSERT(result.Type() == MatchType::NOT_FOUND ||
                result.Type() == MatchType::PENDING,
                "A LookupResult with no surface should be NOT_FOUND or PENDING");
 
     // If this is bigger than we can hold after discarding everything we can,
     // refuse to cache it.
-    if (MOZ_UNLIKELY(!CanHoldAfterDiscarding(aCost))) {
+    Cost cost = aProvider->LogicalSizeInBytes();
+    if (MOZ_UNLIKELY(!CanHoldAfterDiscarding(cost))) {
       mOverflowCount++;
       return InsertOutcome::FAILURE;
     }
 
     // Remove elements in order of cost until we can fit this in the cache. Note
     // that locked surfaces aren't in mCosts, so we never remove them here.
-    while (aCost > mAvailableCost) {
+    while (cost > mAvailableCost) {
       MOZ_ASSERT(!mCosts.IsEmpty(),
                  "Removed everything and it still won't fit");
       Remove(mCosts.LastElement().Surface());
     }
 
     // Locate the appropriate per-image cache. If there's not an existing cache
     // for this image, create it.
-    RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
+    RefPtr<ImageSurfaceCache> cache = GetImageCache(aProvider->GetImageKey());
     if (!cache) {
       cache = new ImageSurfaceCache;
-      mImageCaches.Put(aImageKey, cache);
+      mImageCaches.Put(aProvider->GetImageKey(), cache);
     }
 
     // If we were asked to mark the cache entry available, do so.
     if (aSetAvailable) {
       aProvider->Availability().SetAvailable();
     }
 
     NotNull<RefPtr<CachedSurface>> surface =
-      WrapNotNull(new CachedSurface(aProvider, aCost, aImageKey, aSurfaceKey));
+      WrapNotNull(new CachedSurface(aProvider));
 
     // We require that locking succeed if the image is locked and we're not
     // inserting a placeholder; the caller may need to know this to handle
     // errors correctly.
     if (cache->IsLocked() && !surface->IsPlaceholder()) {
       surface->SetLocked(true);
       if (!surface->IsLocked()) {
         return InsertOutcome::FAILURE;
       }
     }
 
     // Insert.
-    MOZ_ASSERT(aCost <= mAvailableCost, "Inserting despite too large a cost");
-    cache->Insert(aSurfaceKey, surface);
+    MOZ_ASSERT(cost <= mAvailableCost, "Inserting despite too large a cost");
+    cache->Insert(surface);
     StartTracking(surface);
 
     return InsertOutcome::SUCCESS;
   }
 
   void Remove(NotNull<CachedSurface*> aSurface)
   {
     ImageKey imageKey = aSurface->GetImageKey();
@@ -649,33 +642,30 @@ public:
     return aCost <= mMaxCost;
   }
 
   size_t MaximumCapacity() const
   {
     return size_t(mMaxCost);
   }
 
-  void SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider,
-                        const ImageKey             aImageKey,
-                        const SurfaceKey&          aSurfaceKey)
+  void SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider)
   {
     if (!aProvider->Availability().IsPlaceholder()) {
       MOZ_ASSERT_UNREACHABLE("Calling SurfaceAvailable on non-placeholder");
       return;
     }
 
     // Reinsert the provider, requesting that Insert() mark it available. This
     // may or may not succeed, depending on whether some other decoder has
     // beaten us to the punch and inserted a non-placeholder version of this
     // surface first, but it's fine either way.
     // XXX(seth): This could be implemented more efficiently; we should be able
     // to just update our data structures without reinserting.
-    Cost cost = aProvider->LogicalSizeInBytes();
-    Insert(aProvider, cost, aImageKey, aSurfaceKey, /* aSetAvailable = */ true);
+    Insert(aProvider, /* aSetAvailable = */ true);
   }
 
   void LockImage(const ImageKey aImageKey)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       cache = new ImageSurfaceCache;
       mImageCaches.Put(aImageKey, cache);
@@ -1023,28 +1013,24 @@ SurfaceCache::LookupBestMatch(const Imag
     return LookupResult(MatchType::NOT_FOUND);
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
   return sInstance->LookupBestMatch(aImageKey, aSurfaceKey);
 }
 
 /* static */ InsertOutcome
-SurfaceCache::Insert(NotNull<ISurfaceProvider*> aProvider,
-                     const ImageKey             aImageKey,
-                     const SurfaceKey&          aSurfaceKey)
+SurfaceCache::Insert(NotNull<ISurfaceProvider*> aProvider)
 {
   if (!sInstance) {
     return InsertOutcome::FAILURE;
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
-  Cost cost = aProvider->LogicalSizeInBytes();
-  return sInstance->Insert(aProvider, cost, aImageKey, aSurfaceKey,
-                           /* aSetAvailable = */ false);
+  return sInstance->Insert(aProvider, /* aSetAvailable = */ false);
 }
 
 /* static */ bool
 SurfaceCache::CanHold(const IntSize& aSize, uint32_t aBytesPerPixel /* = 4 */)
 {
   if (!sInstance) {
     return false;
   }
@@ -1059,26 +1045,24 @@ SurfaceCache::CanHold(size_t aSize)
   if (!sInstance) {
     return false;
   }
 
   return sInstance->CanHold(aSize);
 }
 
 /* static */ void
-SurfaceCache::SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider,
-                               const ImageKey             aImageKey,
-                               const SurfaceKey&          aSurfaceKey)
+SurfaceCache::SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider)
 {
   if (!sInstance) {
     return;
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
-  sInstance->SurfaceAvailable(aProvider, aImageKey, aSurfaceKey);
+  sInstance->SurfaceAvailable(aProvider);
 }
 
 /* static */ void
 SurfaceCache::LockImage(const ImageKey aImageKey)
 {
   if (sInstance) {
     MutexAutoLock lock(sInstance->GetMutex());
     return sInstance->LockImage(aImageKey);
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -264,54 +264,42 @@ struct SurfaceCache
    * FAILURE if it failed to insert the cache entry, which could happen because
    * of capacity reasons, or because it was already freed by the OS. If the
    * cache entry isn't associated with a locked image, checking for SUCCESS or
    * FAILURE is useless: the entry might expire immediately after being
    * inserted, even though Insert() returned SUCCESS. Thus, many callers do not
    * need to check the result of Insert() at all.
    *
    * @param aProvider    The new cache entry to insert into the cache.
-   * @param aImageKey       Key data identifying which image the cache entry
-   *                        belongs to.
-   * @param aSurfaceKey     Key data which uniquely identifies the requested
-   *                        cache entry.
    * @return SUCCESS if the cache entry was inserted successfully. (But see above
    *           for more information about when you should check this.)
    *         FAILURE if the cache entry could not be inserted, e.g. for capacity
    *           reasons. (But see above for more information about when you
    *           should check this.)
    *         FAILURE_ALREADY_PRESENT if an entry with the same ImageKey and
    *           SurfaceKey already exists in the cache.
    */
-  static InsertOutcome Insert(NotNull<ISurfaceProvider*> aProvider,
-                              const ImageKey    aImageKey,
-                              const SurfaceKey& aSurfaceKey);
+  static InsertOutcome Insert(NotNull<ISurfaceProvider*> aProvider);
 
   /**
    * Mark the cache entry @aProvider as having an available surface. This turns
    * a placeholder cache entry into a normal cache entry. The cache entry
    * becomes locked if the associated image is locked; otherwise, it starts in
    * the unlocked state.
    *
    * If the cache entry containing @aProvider has already been evicted from the
    * surface cache, this function has no effect.
    *
    * It's illegal to call this function if @aProvider is not a placeholder; by
    * definition, non-placeholder ISurfaceProviders should have a surface
    * available already.
    *
    * @param aProvider       The cache entry that now has a surface available.
-   * @param aImageKey       Key data identifying which image the cache entry
-   *                        belongs to.
-   * @param aSurfaceKey     Key data which uniquely identifies the requested
-   *                        cache entry.
    */
-  static void SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider,
-                               const ImageKey    aImageKey,
-                               const SurfaceKey& aSurfaceKey);
+  static void SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider);
 
   /**
    * Checks if a surface of a given size could possibly be stored in the cache.
    * If CanHold() returns false, Insert() will always fail to insert the
    * surface, but the inverse is not true: Insert() may take more information
    * into account than just image size when deciding whether to cache the
    * surface, so Insert() may still fail even if CanHold() returns true.
    *
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -960,17 +960,17 @@ VectorImage::CreateSurfaceAndShow(const 
   if (!surface) {
     return Show(svgDrawable, aParams);
   }
 
   // Attempt to cache the frame.
   SurfaceKey surfaceKey = VectorSurfaceKey(aParams.size, aParams.svgContext);
   NotNull<RefPtr<ISurfaceProvider>> provider =
     WrapNotNull(new SimpleSurfaceProvider(ImageKey(this), surfaceKey, frame));
-  SurfaceCache::Insert(provider, ImageKey(this), surfaceKey);
+  SurfaceCache::Insert(provider);
 
   // Draw.
   RefPtr<gfxDrawable> drawable =
     new gfxSurfaceDrawable(surface, aParams.size);
   Show(drawable, aParams);
 
   // Send out an invalidation so that surfaces that are still in use get
   // re-locked. See the discussion of the UnlockSurfaces call above.