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 306956 37340346a89e3ff5680559afabbc430ffaf9bb74
parent 306955 196ddcbf522500350d90dfb8af8fbc6fb0f475c5
child 306957 5083d26c9c92a364f954be87a4aea2323950afc4
push id30502
push usercbook@mozilla.com
push dateThu, 28 Jul 2016 15:43:16 +0000
treeherdermozilla-central@9ec789c0ee5b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, edwin
bugs1289628
milestone50.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 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();