Bug 1289628 - Return ISurfaceProvider objects from SurfaceCache lookup functions. r=dholbert,edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 26 Jul 2016 16:31:26 -0700
changeset 306983 37340346a89e3ff5680559afabbc430ffaf9bb74
parent 306982 196ddcbf522500350d90dfb8af8fbc6fb0f475c5
child 306984 5083d26c9c92a364f954be87a4aea2323950afc4
push id20145
push usercbook@mozilla.com
push dateThu, 28 Jul 2016 15:45:29 +0000
treeherderfx-team@2d6fced8b624 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, edwin
bugs1289628
milestone50.0a1
Bug 1289628 - Return ISurfaceProvider objects from SurfaceCache lookup functions. r=dholbert,edwin
image/FrameAnimator.cpp
image/LookupResult.h
image/RasterImage.cpp
image/SurfaceCache.cpp
image/SurfaceCache.h
image/VectorImage.cpp
image/VectorImage.h
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -2,16 +2,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "FrameAnimator.h"
 
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
+#include "mozilla/NotNull.h"
 #include "imgIContainer.h"
 #include "LookupResult.h"
 #include "MainThreadUtils.h"
 #include "RasterImage.h"
 
 #include "pixman.h"
 
 namespace mozilla {
@@ -276,27 +277,31 @@ FrameAnimator::RequestRefresh(AnimationS
 
 LookupResult
 FrameAnimator::GetCompositedFrame(uint32_t aFrameNum)
 {
   MOZ_ASSERT(aFrameNum != 0, "First frame is never composited");
 
   // If we have a composited version of this frame, return that.
   if (mLastCompositedFrameIndex == int32_t(aFrameNum)) {
-    return LookupResult(mCompositingFrame->DrawableRef(), MatchType::EXACT);
+    RefPtr<ISurfaceProvider> provider =
+      new SimpleSurfaceProvider(WrapNotNull(mCompositingFrame.get()));
+    return LookupResult(Move(provider), MatchType::EXACT);
   }
 
   // Otherwise return the raw frame. DoBlend is required to ensure that we only
   // hit this case if the frame is not paletted and doesn't require compositing.
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
                                           aFrameNum));
-  MOZ_ASSERT(!result || !result.DrawableRef()->GetIsPaletted(),
+  MOZ_ASSERT(!result ||
+             !result.Provider()->DrawableRef() ||
+             !result.Provider()->DrawableRef()->GetIsPaletted(),
              "About to return a paletted frame");
   return result;
 }
 
 FrameTimeout
 FrameAnimator::GetTimeoutForFrame(uint32_t aFrameNum) const
 {
   RawAccessFrameRef frame = GetRawFrame(aFrameNum);
@@ -356,18 +361,28 @@ FrameAnimator::CollectSizeOfCompositingS
 RawAccessFrameRef
 FrameAnimator::GetRawFrame(uint32_t aFrameNum) const
 {
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
                                           aFrameNum));
-  return result ? result.DrawableRef()->RawAccessRef()
-                : RawAccessFrameRef();
+  if (!result) {
+    return RawAccessFrameRef();
+  }
+
+  DrawableFrameRef drawableRef = result.Provider()->DrawableRef();
+  if (!drawableRef) {
+    MOZ_ASSERT_UNREACHABLE("Animated image frames should be locked and "
+                           "in-memory; how did we lose one?");
+    return RawAccessFrameRef();
+  }
+
+  return drawableRef->RawAccessRef();
 }
 
 //******************************************************************************
 // DoBlend gets called when the timer for animation get fired and we have to
 // update the composited frame of the animation.
 bool
 FrameAnimator::DoBlend(IntRect* aDirtyRect,
                        uint32_t aPrevFrameIndex,
--- a/image/LookupResult.h
+++ b/image/LookupResult.h
@@ -8,83 +8,83 @@
  * combines a surface with relevant metadata tracked by SurfaceCache.
  */
 
 #ifndef mozilla_image_LookupResult_h
 #define mozilla_image_LookupResult_h
 
 #include "mozilla/Attributes.h"
 #include "mozilla/Move.h"
-#include "imgFrame.h"
+#include "ISurfaceProvider.h"
 
 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.
+ * combines an ISurfaceProvider with relevant metadata tracked by SurfaceCache.
  */
 class MOZ_STACK_CLASS LookupResult
 {
 public:
   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))
+    : mProvider(Move(aOther.mProvider))
     , mMatchType(aOther.mMatchType)
   { }
 
-  LookupResult(DrawableFrameRef&& aDrawableRef, MatchType aMatchType)
-    : mDrawableRef(Move(aDrawableRef))
+  LookupResult(RefPtr<ISurfaceProvider>&& aProvider, MatchType aMatchType)
+    : mProvider(Move(aProvider))
     , mMatchType(aMatchType)
   {
-    MOZ_ASSERT(!mDrawableRef || !(mMatchType == MatchType::NOT_FOUND ||
-                                  mMatchType == MatchType::PENDING),
+    MOZ_ASSERT(!mProvider || !(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,
+    MOZ_ASSERT(mProvider || 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);
+    mProvider = Move(aOther.mProvider);
     mMatchType = aOther.mMatchType;
     return *this;
   }
 
-  DrawableFrameRef& DrawableRef() { return mDrawableRef; }
-  const DrawableFrameRef& DrawableRef() const { return mDrawableRef; }
+  ISurfaceProvider* Provider() { return mProvider.get(); }
+  const ISurfaceProvider* Provider() const { return mProvider.get(); }
 
-  /// @return true if this LookupResult contains a surface.
-  explicit operator bool() const { return bool(mDrawableRef); }
+  /// @return true if this LookupResult contains a surface provider.
+  explicit operator bool() const { return bool(mProvider); }
 
   /// @return what kind of match this is (exact, substitute, etc.)
   MatchType Type() const { return mMatchType; }
 
 private:
   LookupResult(const LookupResult&) = delete;
 
-  DrawableFrameRef mDrawableRef;
+  RefPtr<ISurfaceProvider> mProvider;
   MatchType mMatchType;
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_LookupResult_h
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -350,40 +350,51 @@ RasterImage::LookupFrame(uint32_t aFrame
     }
   }
 
   if (!result) {
     // We still weren't able to get a frame. Give up.
     return DrawableFrameRef();
   }
 
-  if (result.DrawableRef()->GetCompositingFailed()) {
+  // OK, we've got an ISurfaceProvider. Ask it to give us a surface containing
+  // the frame we're looking for. (This could be computationally expensive in
+  // some cases.)
+  DrawableFrameRef drawableRef = result.Provider()->DrawableRef();
+  if (!drawableRef) {
+    // An ISurfaceProvider will only fail to give us a surface in catastrophic
+    // cases such as the operating system freeing our volatile buffers. Our best
+    // bet is to throw everything out and attempt to recover.
+    RecoverFromInvalidFrames(aSize, aFlags);
     return DrawableFrameRef();
   }
 
-  MOZ_ASSERT(!result.DrawableRef()->GetIsPaletted(),
-             "Should not have a paletted frame");
+  if (drawableRef->GetCompositingFailed()) {
+    return DrawableFrameRef();
+  }
+
+  MOZ_ASSERT(!drawableRef->GetIsPaletted(), "Should not have a paletted frame");
 
   // Sync decoding guarantees that we got the frame, but if it's owned by an
   // async decoder that's currently running, the contents of the frame may not
   // be available yet. Make sure we get everything.
   if (mHasSourceData && (aFlags & FLAG_SYNC_DECODE)) {
-    result.DrawableRef()->WaitUntilFinished();
+    drawableRef->WaitUntilFinished();
   }
 
   // If we could have done some decoding in this function we need to check if
   // that decoding encountered an error and hence aborted the surface. We want
   // to avoid calling IsAborted if we weren't passed any sync decode flag because
   // IsAborted acquires the monitor for the imgFrame.
   if (aFlags & (FLAG_SYNC_DECODE | FLAG_SYNC_DECODE_IF_FAST) &&
-    result.DrawableRef()->IsAborted()) {
+      drawableRef->IsAborted()) {
     return DrawableFrameRef();
   }
 
-  return Move(result.DrawableRef());
+  return Move(drawableRef);
 }
 
 uint32_t
 RasterImage::GetCurrentFrameIndex() const
 {
   if (mAnimationState) {
     return mAnimationState->GetCurrentAnimationFrameIndex();
   }
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -51,16 +51,17 @@ class SurfaceCacheImpl;
 
 ///////////////////////////////////////////////////////////////////////////////
 // Static Data
 ///////////////////////////////////////////////////////////////////////////////
 
 // The single surface cache instance.
 static StaticRefPtr<SurfaceCacheImpl> sInstance;
 
+
 ///////////////////////////////////////////////////////////////////////////////
 // SurfaceCache Implementation
 ///////////////////////////////////////////////////////////////////////////////
 
 /**
  * Cost models the cost of storing a surface in the cache. Right now, this is
  * simply an estimate of the size of the surface in bytes, but in the future it
  * may be worth taking into account the cost of rematerializing the surface as
@@ -140,24 +141,27 @@ public:
     , mImageKey(aImageKey)
     , mSurfaceKey(aSurfaceKey)
   {
     MOZ_ASSERT(!IsPlaceholder() || mCost == sPlaceholderCost,
                "Placeholder should have trivial cost");
     MOZ_ASSERT(mImageKey, "Must have a valid image key");
   }
 
-  DrawableFrameRef DrawableRef() const
+  already_AddRefed<ISurfaceProvider> Provider() const
   {
     if (MOZ_UNLIKELY(IsPlaceholder())) {
-      MOZ_ASSERT_UNREACHABLE("Shouldn't call DrawableRef() on a placeholder");
-      return DrawableFrameRef();
+      MOZ_ASSERT_UNREACHABLE("Shouldn't call Provider() on a placeholder");
+      return nullptr;
     }
 
-    return mProvider->DrawableRef();
+    MOZ_ASSERT(mProvider);
+
+    RefPtr<ISurfaceProvider> provider = mProvider;
+    return provider.forget();
   }
 
   void SetLocked(bool aLocked)
   {
     if (IsPlaceholder()) {
       return;  // Can't lock a placeholder.
     }
 
@@ -188,40 +192,51 @@ public:
 
     void Add(CachedSurface* aCachedSurface)
     {
       MOZ_ASSERT(aCachedSurface, "Should have a CachedSurface");
 
       SurfaceMemoryCounter counter(aCachedSurface->GetSurfaceKey(),
                                    aCachedSurface->IsLocked());
 
-      if (!aCachedSurface->IsPlaceholder()) {
-        DrawableFrameRef surfaceRef = aCachedSurface->DrawableRef();
-        if (surfaceRef) {
-          counter.SubframeSize() = Some(surfaceRef->GetSize());
+      if (aCachedSurface->IsPlaceholder()) {
+        mCounters.AppendElement(counter);
+        return;
+      }
+
+      RefPtr<ISurfaceProvider> provider = aCachedSurface->Provider();
+      if (!provider) {
+        MOZ_ASSERT_UNREACHABLE("Not a placeholder, but no ISurfaceProvider?");
+        mCounters.AppendElement(counter);
+        return;
+      }
 
-          size_t heap = 0, nonHeap = 0;
-          surfaceRef->AddSizeOfExcludingThis(mMallocSizeOf, heap, nonHeap);
-          counter.Values().SetDecodedHeap(heap);
-          counter.Values().SetDecodedNonHeap(nonHeap);
-        }
+      DrawableFrameRef surfaceRef = provider->DrawableRef();
+      if (!surfaceRef) {
+        mCounters.AppendElement(counter);
+        return;
       }
 
+      counter.SubframeSize() = Some(surfaceRef->GetSize());
+      size_t heap = 0, nonHeap = 0;
+      surfaceRef->AddSizeOfExcludingThis(mMallocSizeOf, heap, nonHeap);
+      counter.Values().SetDecodedHeap(heap);
+      counter.Values().SetDecodedNonHeap(nonHeap);
+
       mCounters.AppendElement(counter);
     }
 
   private:
     nsTArray<SurfaceMemoryCounter>& mCounters;
     MallocSizeOf                    mMallocSizeOf;
   };
 
 private:
   nsExpirationState  mExpirationState;
   RefPtr<ISurfaceProvider> mProvider;
-  DrawableFrameRef   mDrawableRef;
   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);
@@ -583,81 +598,68 @@ public:
       // Lookup in the per-image cache missed.
       return LookupResult(MatchType::NOT_FOUND);
     }
 
     if (surface->IsPlaceholder()) {
       return LookupResult(MatchType::PENDING);
     }
 
-    DrawableFrameRef ref = surface->DrawableRef();
-    if (!ref) {
-      // The surface was released by the operating system. Remove the cache
-      // entry as well.
+    RefPtr<ISurfaceProvider> provider = surface->Provider();
+    if (!provider) {
+      MOZ_ASSERT_UNREACHABLE("Not a placeholder, but no ISurfaceProvider?");
       Remove(surface);
       return LookupResult(MatchType::NOT_FOUND);
     }
 
     if (aMarkUsed) {
       MarkUsed(surface, cache);
     }
 
     MOZ_ASSERT(surface->GetSurfaceKey() == aSurfaceKey,
                "Lookup() not returning an exact match?");
-    return LookupResult(Move(ref), MatchType::EXACT);
+    return LookupResult(Move(provider), MatchType::EXACT);
   }
 
   LookupResult LookupBestMatch(const ImageKey         aImageKey,
                                const SurfaceKey&      aSurfaceKey)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       // No cached surfaces for this image.
       return LookupResult(MatchType::NOT_FOUND);
     }
 
-    // Repeatedly look up the best match, trying again if the resulting surface
-    // has been freed by the operating system, until we can either lock a
-    // surface for drawing or there are no matching surfaces left.
-    // XXX(seth): This is O(N^2), but N is expected to be very small. If we
-    // encounter a performance problem here we can revisit this.
-
     RefPtr<CachedSurface> surface;
-    DrawableFrameRef ref;
     MatchType matchType = MatchType::NOT_FOUND;
-    while (true) {
-      Tie(surface, matchType) = cache->LookupBestMatch(aSurfaceKey);
+    Tie(surface, matchType) = cache->LookupBestMatch(aSurfaceKey);
+    if (!surface) {
+      return LookupResult(matchType);  // Lookup in the per-image cache missed.
+    }
 
-      if (!surface) {
-        return LookupResult(matchType);  // Lookup in the per-image cache missed.
-      }
-
-      ref = surface->DrawableRef();
-      if (ref) {
-        break;
-      }
-
-      // The surface was released by the operating system. Remove the cache
-      // entry as well.
+    RefPtr<ISurfaceProvider> provider = surface->Provider();
+    if (!provider) {
+      MOZ_ASSERT_UNREACHABLE("Not a placeholder, but no ISurfaceProvider?");
       Remove(surface);
+      return LookupResult(MatchType::NOT_FOUND);
     }
 
     MOZ_ASSERT_IF(matchType == MatchType::EXACT,
                   surface->GetSurfaceKey() == aSurfaceKey);
     MOZ_ASSERT_IF(matchType == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
                   matchType == MatchType::SUBSTITUTE_BECAUSE_PENDING,
       surface->GetSurfaceKey().SVGContext() == aSurfaceKey.SVGContext() &&
       surface->GetSurfaceKey().AnimationTime() == aSurfaceKey.AnimationTime() &&
       surface->GetSurfaceKey().Flags() == aSurfaceKey.Flags());
 
     if (matchType == MatchType::EXACT) {
       MarkUsed(surface, cache);
     }
 
-    return LookupResult(Move(ref), matchType);
+    return LookupResult(Move(provider), matchType);
   }
 
   bool CanHold(const Cost aCost) const
   {
     return aCost <= mMaxCost;
   }
 
   size_t MaximumCapacity() const
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -158,57 +158,56 @@ struct SurfaceCache
   static void Initialize();
 
   /**
    * Release static data. Called during imagelib module shutdown.
    */
   static void Shutdown();
 
   /**
-   * Looks up the requested cache entry and returns a drawable reference to its
-   * associated surface.
+   * Looks up and returns the requested cache entry.
    *
    * If the image associated with the cache entry is locked, then the entry will
    * be locked before it is returned.
    *
-   * If a matching ISurfaceProvider was found in the cache, but SurfaceCache
-   * couldn't obtain a surface from it (e.g. because it 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 ISurfaceProviders associated with a
-   * locked image; SurfaceCache tells such ISurfaceProviders to keep a strong
-   * references to their data internally.
+   * This function returns an ISurfaceProvider, but it does not guarantee that
+   * the ISurfaceProvider can give the caller a drawable surface. Lookup()
+   * callers should check that ISurfaceProvider::DrawableRef() returns a
+   * non-empty value; if not, some sort of serious failure has occurred, and the
+   * best bet is to remove all existing surfaces for the image from the cache
+   * using RemoveImage() and try to recover. The most likely cause for this kind
+   * of failure is the surface's volatile buffer being freed by the operating
+   * system due to extreme memory pressure.
    *
    * @param aImageKey       Key data identifying which image the cache entry
    *                        belongs to.
    * @param aSurfaceKey     Key data which uniquely identifies the requested
    *                        cache entry.
-   * @return                a LookupResult, which will either contain a
-   *                        DrawableFrameRef to a surface, or an empty
-   *                        DrawableFrameRef if the cache entry was not found.
+   * @return                a LookupResult which will contain an ISurfaceProvider
+   *                        for the requested surface if a matching cache entry
+   *                        was found.
    */
   static LookupResult Lookup(const ImageKey    aImageKey,
                              const SurfaceKey& aSurfaceKey);
 
   /**
-   * Looks up the best matching cache entry and returns a drawable reference to
-   * its associated surface.
+   * Looks up and returns the best matching cache entry.
    *
    * The result may vary from the requested cache entry only in terms of size.
    *
    * @param aImageKey       Key data identifying which image the cache entry
    *                        belongs to.
    * @param aSurfaceKey     Key data which uniquely identifies the requested
    *                        cache entry.
-   * @return                a LookupResult, which will either contain a
-   *                        DrawableFrameRef to a surface similar to the
-   *                        the one the caller requested, or an empty
-   *                        DrawableFrameRef if no acceptable match was found.
-   *                        Callers can use LookupResult::IsExactMatch() to check
-   *                        whether the returned surface exactly matches
+   * @return                a LookupResult which will contain either an
+   *                        ISurfaceProvider for a surface similar to the one
+   *                        the caller requested, or no ISurfaceProvider if no
+   *                        acceptable match was found. Callers can use
+   *                        LookupResult::IsExactMatch() to check whether the
+   *                        returned ISurfaceProvider exactly matches
    *                        @aSurfaceKey.
    */
   static LookupResult LookupBestMatch(const ImageKey    aImageKey,
                                       const SurfaceKey& aSurfaceKey);
 
   /**
    * Insert an ISurfaceProvider into the cache. If an entry with the same
    * ImageKey and SurfaceKey is already in the cache, Insert returns
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -854,44 +854,65 @@ VectorImage::Draw(gfxContext* aContext,
                                  : mSVGDocumentWrapper->GetCurrentTime();
   AutoSVGRenderingState autoSVGState(svgContext, animTime,
                                      mSVGDocumentWrapper->GetRootSVGElem());
 
 
   SVGDrawingParameters params(aContext, aSize, aRegion, aSamplingFilter,
                               svgContext, animTime, aFlags);
 
-  if (aFlags & FLAG_BYPASS_SURFACE_CACHE) {
-    CreateSurfaceAndShow(params);
+  // If we have an prerasterized version of this image that matches the
+  // drawing parameters, use that.
+  RefPtr<gfxDrawable> svgDrawable = LookupCachedSurface(params);
+  if (svgDrawable) {
+    Show(svgDrawable, params);
     return DrawResult::SUCCESS;
   }
 
+  // We didn't get a hit in the surface cache, so we'll need to rerasterize.
+  CreateSurfaceAndShow(params);
+  return DrawResult::SUCCESS;
+}
+
+already_AddRefed<gfxDrawable>
+VectorImage::LookupCachedSurface(const SVGDrawingParameters& aParams)
+{
+  // If we're not allowed to use a cached surface, don't attempt a lookup.
+  if (aParams.flags & FLAG_BYPASS_SURFACE_CACHE) {
+    return nullptr;
+  }
+
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(this),
-                         VectorSurfaceKey(params.size,
-                                          params.svgContext,
-                                          params.animationTime));
-
-  // Draw.
-  if (result) {
-    RefPtr<SourceSurface> surface = result.DrawableRef()->GetSurface();
-    if (surface) {
-      RefPtr<gfxDrawable> svgDrawable =
-        new gfxSurfaceDrawable(surface, result.DrawableRef()->GetSize());
-      Show(svgDrawable, params);
-      return DrawResult::SUCCESS;
-    }
-
-    // We lost our surface due to some catastrophic event.
-    RecoverFromLossOfSurfaces();
+                         VectorSurfaceKey(aParams.size,
+                                          aParams.svgContext,
+                                          aParams.animationTime));
+  if (!result) {
+    return nullptr;  // No matching surface.
   }
 
-  CreateSurfaceAndShow(params);
+  DrawableFrameRef drawableRef = result.Provider()->DrawableRef();
+  if (!drawableRef) {
+    // Something went wrong. (Probably the OS freeing our volatile buffer due to
+    // low memory.) Attempt to recover.
+    RecoverFromLossOfSurfaces();
+    return nullptr;
+  }
 
-  return DrawResult::SUCCESS;
+  RefPtr<SourceSurface> surface = drawableRef->GetSurface();
+  if (!surface) {
+    // Something went wrong. (Probably a GPU driver crash or device reset.)
+    // Attempt to recover.
+    RecoverFromLossOfSurfaces();
+    return nullptr;
+  }
+
+  RefPtr<gfxDrawable> svgDrawable =
+    new gfxSurfaceDrawable(surface, drawableRef->GetSize());
+  return svgDrawable.forget();
 }
 
 void
 VectorImage::CreateSurfaceAndShow(const SVGDrawingParameters& aParams)
 {
   mSVGDocumentWrapper->UpdateViewportBounds(aParams.viewportSize);
   mSVGDocumentWrapper->FlushImageTransformInvalidation();
 
--- a/image/VectorImage.h
+++ b/image/VectorImage.h
@@ -73,20 +73,24 @@ public:
 protected:
   explicit VectorImage(ImageURL* aURI = nullptr);
   virtual ~VectorImage();
 
   virtual nsresult StartAnimation() override;
   virtual nsresult StopAnimation() override;
   virtual bool     ShouldAnimate() override;
 
+private:
+  /// Attempt to find a cached surface matching @aParams in the SurfaceCache.
+  already_AddRefed<gfxDrawable>
+    LookupCachedSurface(const SVGDrawingParameters& aParams);
+
   void CreateSurfaceAndShow(const SVGDrawingParameters& aParams);
   void Show(gfxDrawable* aDrawable, const SVGDrawingParameters& aParams);
 
-private:
   nsresult Init(const char* aMimeType, uint32_t aFlags);
 
   /**
    * In catastrophic circumstances like a GPU driver crash, we may lose our
    * surfaces even if they're locked. RecoverFromLossOfSurfaces discards all
    * existing surfaces, allowing us to recover.
    */
   void RecoverFromLossOfSurfaces();