Bug 1146663 (Part 2) - Remove the concept of lifetimes from the SurfaceCache. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Sat, 19 Sep 2015 16:20:59 -0700
changeset 263442 83a6dc9ee30c1c97314ea8a342154cba4a523ed1
parent 263441 2b6a3ec30a149c1142ac390fd5e5bf3e7a34a2e1
child 263443 77357a5a1ad1a0306b1f1f4688baf2d5312b315b
push id29400
push userphilringnalda@gmail.com
push dateSun, 20 Sep 2015 04:08:02 +0000
treeherderautoland@ccd6b5f5e544 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1146663
milestone43.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 1146663 (Part 2) - Remove the concept of lifetimes from the SurfaceCache. r=dholbert
image/Decoder.cpp
image/Image.h
image/SurfaceCache.cpp
image/SurfaceCache.h
image/VectorImage.cpp
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -319,18 +319,17 @@ Decoder::AllocateFrameInternal(uint32_t 
     return RawAccessFrameRef();
   }
 
   if (ShouldUseSurfaceCache()) {
     InsertOutcome outcome =
       SurfaceCache::Insert(frame, ImageKey(mImage.get()),
                            RasterSurfaceKey(aTargetSize,
                                             mSurfaceFlags,
-                                            aFrameNum),
-                           Lifetime::Persistent);
+                                            aFrameNum));
     if (outcome == InsertOutcome::FAILURE) {
       // We couldn't insert the surface, almost certainly due to low memory. We
       // treat this as a permanent error to help the system recover; otherwise,
       // we might just end up attempting to decode this image again immediately.
       ref->Abort();
       return RawAccessFrameRef();
     } else if (outcome == InsertOutcome::FAILURE_ALREADY_PRESENT) {
       // Another decoder beat us to decoding this frame. We abort this decoder
--- a/image/Image.h
+++ b/image/Image.h
@@ -212,18 +212,17 @@ public:
    * @param aLastPart Whether this is the final part of the underlying request.
    */
   virtual nsresult OnImageDataComplete(nsIRequest* aRequest,
                                        nsISupports* aContext,
                                        nsresult aStatus,
                                        bool aLastPart) = 0;
 
   /**
-   * Called when the SurfaceCache discards a persistent surface belonging to
-   * this image.
+   * Called when the SurfaceCache discards a surface belonging to this image.
    */
   virtual void OnSurfaceDiscarded() = 0;
 
   virtual void SetInnerWindowID(uint64_t aInnerWindowId) = 0;
   virtual uint64_t InnerWindowID() const = 0;
 
   virtual bool HasError() = 0;
   virtual void SetHasError() = 0;
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -130,27 +130,24 @@ class CachedSurface
   ~CachedSurface() { }
 public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(CachedSurface)
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CachedSurface)
 
   CachedSurface(imgFrame*          aSurface,
                 const Cost         aCost,
                 const ImageKey     aImageKey,
-                const SurfaceKey&  aSurfaceKey,
-                const Lifetime     aLifetime)
+                const SurfaceKey&  aSurfaceKey)
     : mSurface(aSurface)
     , mCost(aCost)
     , mImageKey(aImageKey)
     , mSurfaceKey(aSurfaceKey)
-    , mLifetime(aLifetime)
   {
-    MOZ_ASSERT(!IsPlaceholder() ||
-               (mCost == sPlaceholderCost && mLifetime == Lifetime::Transient),
-               "Placeholder should have trivial cost and transient lifetime");
+    MOZ_ASSERT(!IsPlaceholder() || mCost == sPlaceholderCost,
+               "Placeholder should have trivial cost");
     MOZ_ASSERT(mImageKey, "Must have a valid image key");
   }
 
   DrawableFrameRef DrawableRef() const
   {
     if (MOZ_UNLIKELY(IsPlaceholder())) {
       MOZ_ASSERT_UNREACHABLE("Shouldn't call DrawableRef() on a placeholder");
       return DrawableFrameRef();
@@ -160,34 +157,33 @@ public:
   }
 
   void SetLocked(bool aLocked)
   {
     if (IsPlaceholder()) {
       return;  // Can't lock a placeholder.
     }
 
-    if (aLocked && mLifetime == Lifetime::Persistent) {
+    if (aLocked) {
       // This may fail, and that's OK. We make no guarantees about whether
       // locking is successful if you call SurfaceCache::LockImage() after
       // SurfaceCache::Insert().
       mDrawableRef = mSurface->DrawableRef();
     } else {
       mDrawableRef.reset();
     }
   }
 
   bool IsPlaceholder() const { return !bool(mSurface); }
   bool IsLocked() const { return bool(mDrawableRef); }
 
   ImageKey GetImageKey() const { return mImageKey; }
   SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
   CostEntry GetCostEntry() { return image::CostEntry(this, mCost); }
   nsExpirationState* GetExpirationState() { return &mExpirationState; }
-  Lifetime GetLifetime() const { return mLifetime; }
 
   bool IsDecoded() const
   {
     return !IsPlaceholder() && mSurface->IsImageComplete();
   }
 
   // A helper type used by SurfaceCacheImpl::CollectSizeOfSurfaces.
   struct MOZ_STACK_CLASS SurfaceMemoryReport
@@ -225,17 +221,16 @@ public:
 
 private:
   nsExpirationState  mExpirationState;
   nsRefPtr<imgFrame> mSurface;
   DrawableFrameRef   mDrawableRef;
   const Cost         mCost;
   const ImageKey     mImageKey;
   const SurfaceKey   mSurfaceKey;
-  const Lifetime     mLifetime;
 };
 
 /**
  * An ImageSurfaceCache is a per-image surface cache. For correctness we must be
  * able to remove all surfaces associated with an image when the image is
  * destroyed or invalidated. Since this will happen frequently, it makes sense
  * to make it cheap by storing the surfaces for each image separately.
  *
@@ -254,19 +249,18 @@ public:
   typedef
     nsRefPtrHashtable<nsGenericHashKey<SurfaceKey>, CachedSurface> SurfaceTable;
 
   bool IsEmpty() const { return mSurfaces.Count() == 0; }
 
   void Insert(const SurfaceKey& aKey, CachedSurface* aSurface)
   {
     MOZ_ASSERT(aSurface, "Should have a surface");
-    MOZ_ASSERT(!mLocked || aSurface->GetLifetime() != Lifetime::Persistent ||
-               aSurface->IsLocked(),
-               "Inserting an unlocked persistent surface for a locked image");
+    MOZ_ASSERT(!mLocked || aSurface->IsPlaceholder() || aSurface->IsLocked(),
+               "Inserting an unlocked surface for a locked image");
     mSurfaces.Put(aKey, aSurface);
   }
 
   void Remove(CachedSurface* aSurface)
   {
     MOZ_ASSERT(aSurface, "Should have a surface");
     MOZ_ASSERT(mSurfaces.GetWeak(aSurface->GetSurfaceKey()),
         "Should not be removing a surface we don't have");
@@ -465,18 +459,17 @@ private:
 public:
   void InitMemoryReporter() { RegisterWeakMemoryReporter(this); }
 
   Mutex& GetMutex() { return mMutex; }
 
   InsertOutcome Insert(imgFrame*         aSurface,
                        const Cost        aCost,
                        const ImageKey    aImageKey,
-                       const SurfaceKey& aSurfaceKey,
-                       Lifetime          aLifetime)
+                       const SurfaceKey& aSurfaceKey)
   {
     // If this is a duplicate surface, refuse to replace the original.
     // XXX(seth): Calling Lookup() and then RemoveSurface() does the lookup
     // twice. We'll make this more efficient in bug 1185137.
     LookupResult result = Lookup(aImageKey, aSurfaceKey, /* aMarkUsed = */ false);
     if (MOZ_UNLIKELY(result)) {
       return InsertOutcome::FAILURE_ALREADY_PRESENT;
     }
@@ -508,22 +501,22 @@ public:
     // for this image, create it.
     nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       cache = new ImageSurfaceCache;
       mImageCaches.Put(aImageKey, cache);
     }
 
     nsRefPtr<CachedSurface> surface =
-      new CachedSurface(aSurface, aCost, aImageKey, aSurfaceKey, aLifetime);
+      new CachedSurface(aSurface, aCost, aImageKey, aSurfaceKey);
 
-    // We require that locking succeed if the image is locked and the surface is
-    // persistent; the caller may need to know this to handle errors correctly.
-    if (cache->IsLocked() && aLifetime == Lifetime::Persistent) {
-      MOZ_ASSERT(!surface->IsPlaceholder(), "Placeholders should be transient");
+    // 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");
@@ -536,18 +529,18 @@ public:
   void Remove(CachedSurface* aSurface)
   {
     MOZ_ASSERT(aSurface, "Should have a surface");
     ImageKey imageKey = aSurface->GetImageKey();
 
     nsRefPtr<ImageSurfaceCache> cache = GetImageCache(imageKey);
     MOZ_ASSERT(cache, "Shouldn't try to remove a surface with no image cache");
 
-    // If the surface was persistent, tell its image that we discarded it.
-    if (aSurface->GetLifetime() == Lifetime::Persistent) {
+    // If the surface was not a placeholder, tell its image that we discarded it.
+    if (!aSurface->IsPlaceholder()) {
       static_cast<Image*>(imageKey)->OnSurfaceDiscarded();
     }
 
     StopTracking(aSurface);
     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.)
@@ -772,19 +765,18 @@ 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);
   }
 
   void DiscardAll()
   {
     // Remove in order of cost because mCosts is an array and the other data
-    // structures are all hash tables. Note that locked surfaces (persistent
-    // surfaces belonging to locked images) are not removed, since they aren't
-    // present in mCosts.
+    // structures are all hash tables. Note that locked surfaces are not
+    // removed, since they aren't present in mCosts.
     while (!mCosts.IsEmpty()) {
       Remove(mCosts.LastElement().GetSurface());
     }
   }
 
   void DiscardForMemoryPressure()
   {
     // Compute our discardable cost. Since locked surfaces aren't discardable,
@@ -808,18 +800,17 @@ public:
     while (mAvailableCost < targetCost) {
       MOZ_ASSERT(!mCosts.IsEmpty(), "Removed everything and still not done");
       Remove(mCosts.LastElement().GetSurface());
     }
   }
 
   void LockSurface(CachedSurface* aSurface)
   {
-    if (aSurface->GetLifetime() == Lifetime::Transient ||
-        aSurface->IsLocked()) {
+    if (aSurface->IsPlaceholder() || aSurface->IsLocked()) {
       return;
     }
 
     StopTracking(aSurface);
 
     // Lock the surface. This can fail.
     aSurface->SetLocked(true);
     StartTracking(aSurface);
@@ -832,18 +823,17 @@ public:
     static_cast<SurfaceCacheImpl*>(aCache)->StopTracking(aSurface);
     return PL_DHASH_NEXT;
   }
 
   static PLDHashOperator DoUnlockSurface(const SurfaceKey&,
                                          CachedSurface*    aSurface,
                                          void*             aCache)
   {
-    if (aSurface->GetLifetime() == Lifetime::Transient ||
-        !aSurface->IsLocked()) {
+    if (aSurface->IsPlaceholder() || !aSurface->IsLocked()) {
       return PL_DHASH_NEXT;
     }
 
     auto cache = static_cast<SurfaceCacheImpl*>(aCache);
     cache->StopTracking(aSurface);
 
     aSurface->SetLocked(false);
     cache->StartTracking(aSurface);
@@ -916,19 +906,19 @@ private:
   {
     nsRefPtr<ImageSurfaceCache> imageCache;
     mImageCaches.Get(aImageKey, getter_AddRefs(imageCache));
     return imageCache.forget();
   }
 
   // This is similar to CanHold() except that it takes into account the costs of
   // locked surfaces. It's used internally in Insert(), but it's not exposed
-  // publicly because if we start permitting multithreaded access to the surface
-  // cache, which seems likely, then the result would be meaningless: another
-  // thread could insert a persistent surface or lock an image at any time.
+  // publicly because we permit multithreaded access to the surface cache, which
+  // means that the result would be meaningless: another thread could insert a
+  // surface or lock an image at any time.
   bool CanHoldAfterDiscarding(const Cost aCost) const
   {
     return aCost <= mMaxCost - mLockedCost;
   }
 
   void MarkUsed(CachedSurface* aSurface, ImageSurfaceCache* aCache)
   {
     if (aCache->IsLocked()) {
@@ -1091,44 +1081,42 @@ SurfaceCache::LookupBestMatch(const Imag
 
   MutexAutoLock lock(sInstance->GetMutex());
   return sInstance->LookupBestMatch(aImageKey, aSurfaceKey, aAlternateFlags);
 }
 
 /* static */ InsertOutcome
 SurfaceCache::Insert(imgFrame*         aSurface,
                      const ImageKey    aImageKey,
-                     const SurfaceKey& aSurfaceKey,
-                     Lifetime          aLifetime)
+                     const SurfaceKey& aSurfaceKey)
 {
   if (!sInstance) {
     return InsertOutcome::FAILURE;
   }
 
   // Refuse null surfaces.
   if (!aSurface) {
     MOZ_CRASH("Don't pass null surfaces to SurfaceCache::Insert");
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
   Cost cost = ComputeCost(aSurface->GetSize(), aSurface->GetBytesPerPixel());
-  return sInstance->Insert(aSurface, cost, aImageKey, aSurfaceKey, aLifetime);
+  return sInstance->Insert(aSurface, cost, aImageKey, aSurfaceKey);
 }
 
 /* static */ InsertOutcome
 SurfaceCache::InsertPlaceholder(const ImageKey    aImageKey,
                                 const SurfaceKey& aSurfaceKey)
 {
   if (!sInstance) {
     return InsertOutcome::FAILURE;
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
-  return sInstance->Insert(nullptr, sPlaceholderCost, aImageKey, aSurfaceKey,
-                           Lifetime::Transient);
+  return sInstance->Insert(nullptr, sPlaceholderCost, aImageKey, aSurfaceKey);
 }
 
 /* static */ bool
 SurfaceCache::CanHold(const IntSize& aSize, uint32_t aBytesPerPixel /* = 4 */)
 {
   if (!sInstance) {
     return false;
   }
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -116,21 +116,16 @@ VectorSurfaceKey(const gfx::IntSize& aSi
                  float aAnimationTime)
 {
   // We don't care about aFlags for VectorImage because none of the flags we
   // have right now influence VectorImage's rendering. If we add a new flag that
   // *does* affect how a VectorImage renders, we'll have to change this.
   return SurfaceKey(aSize, aSVGContext, aAnimationTime, DefaultSurfaceFlags());
 }
 
-enum class Lifetime : uint8_t {
-  Transient,
-  Persistent
-};
-
 enum class InsertOutcome : uint8_t {
   SUCCESS,                 // Success (but see Insert documentation).
   FAILURE,                 // Couldn't insert (e.g., for capacity reasons).
   FAILURE_ALREADY_PRESENT  // A surface with the same key is already present.
 };
 
 /**
  * SurfaceCache is an imagelib-global service that allows caching of temporary
@@ -141,19 +136,18 @@ enum class InsertOutcome : uint8_t {
  * objects, which hold surfaces but also layer on additional features specific
  * to imagelib's needs like animation, padding support, and transparent support
  * for volatile buffers.
  *
  * Sometime it's useful to temporarily prevent surfaces from expiring from the
  * cache. This is most often because losing the data could harm the user
  * experience (for example, we often don't want to allow surfaces that are
  * currently visible to expire) or because it's not possible to rematerialize
- * the surface. SurfaceCache supports this through the use of image locking and
- * surface lifetimes; see the comments for Insert() and LockImage() for more
- * details.
+ * the surface. SurfaceCache supports this through the use of image locking; see
+ * the comments for Insert() and LockImage() for more details.
  *
  * Any image which stores surfaces in the SurfaceCache *must* ensure that it
  * calls RemoveImage() before it is destroyed. See the comments for
  * RemoveImage() for more details.
  */
 struct SurfaceCache
 {
   typedef gfx::IntSize IntSize;
@@ -173,18 +167,18 @@ struct SurfaceCache
    * drawable reference to that imgFrame.
    *
    * If the image associated with the surface is locked, then the surface will
    * be locked before it is returned.
    *
    * If the imgFrame was found in the cache, but had stored its surface in a
    * volatile buffer which was discarded by the OS, then it is automatically
    * removed from the cache and an empty LookupResult is returned. Note that
-   * this will never happen to persistent surfaces associated with a locked
-   * image; the cache keeps a strong reference to such surfaces internally.
+   * this will never happen to surfaces associated with a locked image; the
+   * cache keeps a strong reference to such surfaces internally.
    *
    * @param aImageKey       Key data identifying which image the surface belongs
    *                        to.
    *
    * @param aSurfaceKey     Key data which uniquely identifies the requested
    *                        surface.
    *
    * @param aAlternateFlags If not Nothing(), a different set of flags than the
@@ -231,64 +225,58 @@ struct SurfaceCache
                                       const Maybe<SurfaceFlags>& aAlternateFlags
                                         = Nothing());
 
   /**
    * Insert a surface into the cache. If a surface with the same ImageKey and
    * SurfaceKey is already in the cache, Insert returns FAILURE_ALREADY_PRESENT.
    * If a matching placeholder is already present, the placeholder is removed.
    *
-   * Each surface in the cache has a lifetime, either Transient or Persistent.
-   * Transient surfaces can expire from the cache at any time. Persistent
-   * surfaces, on the other hand, will never expire as long as they remain
-   * locked, but if they become unlocked, can expire just like transient
-   * surfaces. When it is first inserted, a persistent surface is locked if its
-   * associated image is locked. When that image is later unlocked, the surface
-   * becomes unlocked too. To become locked again at that point, two things must
-   * happen: the image must become locked again (via LockImage()), and the
-   * surface must be touched again (via one of the Lookup() functions).
+   * Surfaces will never expire as long as they remain locked, but if they
+   * become unlocked, they can expire either because the SurfaceCache runs out
+   * of capacity or because they've gone too long without being used.  When it
+   * is first inserted, a surface is locked if its associated image is locked.
+   * When that image is later unlocked, the surface becomes unlocked too. To
+   * become locked again at that point, two things must happen: the image must
+   * become locked again (via LockImage()), and the surface must be touched
+   * again (via one of the Lookup() functions).
    *
    * All of this means that a very particular procedure has to be followed for
    * surfaces which cannot be rematerialized. First, they must be inserted
-   * with a persistent lifetime *after* the image is locked with LockImage(); if
-   * you use the other order, the surfaces might expire before LockImage() gets
-   * called or before the surface is touched again by Lookup(). Second, the
-   * image they are associated with must never be unlocked.
+   * *after* the image is locked with LockImage(); if you use the other order,
+   * the surfaces might expire before LockImage() gets called or before the
+   * surface is touched again by Lookup(). Second, the image they are associated
+   * with must never be unlocked.
    *
    * If a surface cannot be rematerialized, it may be important to know whether
    * it was inserted into the cache successfully. Insert() returns FAILURE if it
    * failed to insert the surface, which could happen because of capacity
-   * reasons, or because it was already freed by the OS. If you aren't inserting
-   * a surface with persistent lifetime, or if the surface isn't associated with
-   * a locked image, checking for SUCCESS or FAILURE is useless: the surface
-   * 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.
+   * reasons, or because it was already freed by the OS. If the surface isn't
+   * associated with a locked image, checking for SUCCESS or FAILURE is useless:
+   * the surface 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 aTarget      The new surface (wrapped in an imgFrame) to insert into
    *                     the cache.
    * @param aImageKey    Key data identifying which image the surface belongs
    *                     to.
    * @param aSurfaceKey  Key data which uniquely identifies the requested
    *                     surface.
-   * @param aLifetime    Whether this is a transient surface that can always be
-   *                     allowed to expire, or a persistent surface that
-   *                     shouldn't expire if the image is locked.
    * @return SUCCESS if the surface was inserted successfully. (But see above
    *           for more information about when you should check this.)
    *         FAILURE if the surface 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 a surface with the same ImageKey and
    *           SurfaceKey already exists in the cache.
    */
   static InsertOutcome Insert(imgFrame*         aSurface,
                               const ImageKey    aImageKey,
-                              const SurfaceKey& aSurfaceKey,
-                              Lifetime          aLifetime);
+                              const SurfaceKey& aSurfaceKey);
 
   /**
    * Insert a placeholder for a surface into the cache. If a surface with the
    * same ImageKey and SurfaceKey is already in the cache, InsertPlaceholder()
    * returns FAILURE_ALREADY_PRESENT.
    *
    * Placeholders exist to allow lazy allocation of surfaces. The Lookup*()
    * methods will report whether a placeholder for an exactly matching surface
@@ -328,27 +316,26 @@ struct SurfaceCache
    *                        images.
    *
    * @return false if the surface cache can't hold a surface of that size.
    */
   static bool CanHold(const IntSize& aSize, uint32_t aBytesPerPixel = 4);
   static bool CanHold(size_t aSize);
 
   /**
-   * Locks an image. Any of the image's persistent surfaces which are either
-   * inserted or accessed while the image is locked will not expire.
+   * Locks an image. Any of the image's surfaces which are either inserted or
+   * accessed while the image is locked will not expire.
    *
    * Locking an image does not automatically lock that image's existing
-   * surfaces. A call to LockImage() guarantees that persistent surfaces which
-   * are inserted afterward will not expire before the next call to
-   * UnlockImage() or UnlockSurfaces() for that image. Surfaces that are
-   * accessed via Lookup() or LookupBestMatch() after a LockImage() call will
-   * also not expire until the next UnlockImage() or UnlockSurfaces() call for
-   * that image. Any other surfaces owned by the image may expire at any time,
-   * whether they are persistent or transient.
+   * surfaces. A call to LockImage() guarantees that surfaces which are inserted
+   * afterward will not expire before the next call to UnlockImage() or
+   * UnlockSurfaces() for that image. Surfaces that are accessed via Lookup() or
+   * LookupBestMatch() after a LockImage() call will also not expire until the
+   * next UnlockImage() or UnlockSurfaces() call for that image. Any other
+   * surfaces owned by the image may expire at any time.
    *
    * Regardless of locking, any of an image's surfaces may be removed using
    * RemoveSurface(), and all of an image's surfaces are removed by
    * RemoveImage(), whether the image is locked or not.
    *
    * It's safe to call LockImage() on an image that's already locked; this has
    * no effect.
    *
@@ -376,20 +363,20 @@ struct SurfaceCache
    * Unlocks the existing surfaces of an image, allowing them to expire at any
    * time.
    *
    * This does not unlock the image itself, so accessing the surfaces via
    * Lookup() or LookupBestMatch() will lock them again, and prevent them from
    * expiring.
    *
    * This is intended to be used in situations where it's no longer clear that
-   * all of the persistent surfaces owned by an image are needed. Calling
-   * UnlockSurfaces() and then taking some action that will cause Lookup() to
-   * touch any surfaces that are still useful will permit the remaining surfaces
-   * to expire from the cache.
+   * all of the surfaces owned by an image are needed. Calling UnlockSurfaces()
+   * and then taking some action that will cause Lookup() to touch any surfaces
+   * that are still useful will permit the remaining surfaces to expire from the
+   * cache.
    *
    * If the image is unlocked, this has no effect.
    *
    * @param aImageKey    The image which should have its existing surfaces
    *                     unlocked.
    */
   static void UnlockSurfaces(const ImageKey aImageKey);
 
@@ -420,19 +407,19 @@ struct SurfaceCache
    *
    * @param aImageKey  The image which should be removed from the cache.
    */
   static void RemoveImage(const ImageKey aImageKey);
 
   /**
    * Evicts all evictable surfaces from the cache.
    *
-   * All surfaces are evictable except for persistent surfaces associated with
-   * locked images. Non-evictable surfaces can only be removed by
-   * RemoveSurface() or RemoveImage().
+   * All surfaces are evictable except for surfaces associated with locked
+   * images. Non-evictable surfaces can only be removed by RemoveSurface() or
+   * RemoveImage().
    */
   static void DiscardAll();
 
   /**
    * Collects an accounting of the surfaces contained in the SurfaceCache for
    * the given image, along with their size and various other metadata.
    *
    * This is intended for use with memory reporting.
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -915,18 +915,17 @@ VectorImage::CreateSurfaceAndShow(const 
   if (!surface) {
     return Show(svgDrawable, aParams);
   }
 
   // Attempt to cache the frame.
   SurfaceCache::Insert(frame, ImageKey(this),
                        VectorSurfaceKey(aParams.size,
                                         aParams.svgContext,
-                                        aParams.animationTime),
-                       Lifetime::Persistent);
+                                        aParams.animationTime));
 
   // Draw.
   nsRefPtr<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.