Bug 1641594 - Remove the wrapper around recycled surfaces. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Thu, 11 Jun 2020 17:49:13 +0000
changeset 535436 7024de129b91a791c78b8d535f84705c54580433
parent 535435 7a8c4bb304b870919276746e18296a601779f3fa
child 535437 f1f9e78e6a1290ab7427776bb59f2a87311e6dab
push id37501
push usernbeleuzu@mozilla.com
push dateSat, 13 Jun 2020 03:21:52 +0000
treeherdermozilla-central@80b6f21783a3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1641594
milestone79.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 1641594 - Remove the wrapper around recycled surfaces. r=tnikkel We can perform the same function as RecyclingSourceSurface by checking the ref count of the underlying surface directly. We need to ensure WebRender is explicitly aware that it is a recycled surface, but that is easily achieved by changing the type of the surface. This avoids unnecessary heap allocations, particularly in the case where many elements on the same page refer to the same animation (and thus duplicating RecyclingSourceSurface objects). Differential Revision: https://phabricator.services.mozilla.com/D77513
gfx/layers/SourceSurfaceSharedData.h
gfx/layers/ipc/SharedSurfacesChild.cpp
gfx/layers/ipc/SharedSurfacesChild.h
image/RecyclingSourceSurface.h
image/imgFrame.cpp
image/imgFrame.h
image/moz.build
--- a/gfx/layers/SourceSurfaceSharedData.h
+++ b/gfx/layers/SourceSurfaceSharedData.h
@@ -101,17 +101,17 @@ class SourceSurfaceSharedDataWrapper fin
   base::ProcessId mCreatorPid;
   bool mCreatorRef;
 };
 
 /**
  * This class is used to wrap shared (as in process) data buffers used by a
  * source surface.
  */
-class SourceSurfaceSharedData final : public DataSourceSurface {
+class SourceSurfaceSharedData : public DataSourceSurface {
   typedef mozilla::ipc::SharedMemoryBasic SharedMemoryBasic;
 
  public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(SourceSurfaceSharedData, override)
 
   SourceSurfaceSharedData()
       : mMutex("SourceSurfaceSharedData"),
         mStride(0),
@@ -125,56 +125,56 @@ class SourceSurfaceSharedData final : pu
    * Initialize the surface by creating a shared memory buffer with a size
    * determined by aSize, aStride and aFormat. If aShare is true, it will also
    * immediately attempt to share the surface with the GPU process via
    * SharedSurfacesChild.
    */
   bool Init(const IntSize& aSize, int32_t aStride, SurfaceFormat aFormat,
             bool aShare = true);
 
-  uint8_t* GetData() override {
+  uint8_t* GetData() final {
     MutexAutoLock lock(mMutex);
     return GetDataInternal();
   }
 
-  int32_t Stride() override { return mStride; }
+  int32_t Stride() final { return mStride; }
 
   SurfaceType GetType() const override { return SurfaceType::DATA_SHARED; }
-  IntSize GetSize() const override { return mSize; }
-  SurfaceFormat GetFormat() const override { return mFormat; }
+  IntSize GetSize() const final { return mSize; }
+  SurfaceFormat GetFormat() const final { return mFormat; }
 
-  void GuaranteePersistance() override;
+  void GuaranteePersistance() final;
 
   void SizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
-                           SizeOfInfo& aInfo) const override;
+                           SizeOfInfo& aInfo) const final;
 
-  bool OnHeap() const override { return false; }
+  bool OnHeap() const final { return false; }
 
   /**
    * Although Map (and Moz2D in general) isn't normally threadsafe,
    * we want to allow it for SourceSurfaceSharedData since it should
    * always be fine (for reading at least).
    *
    * This is the same as the base class implementation except using
    * mMapCount instead of mIsMapped since that breaks for multithread.
    *
    * Additionally if a reallocation happened while there were active
    * mappings, then we guarantee that GetData will continue to return
    * the same data pointer by retaining the old shared buffer until
    * the last mapping is freed via Unmap.
    */
-  bool Map(MapType, MappedSurface* aMappedSurface) override {
+  bool Map(MapType, MappedSurface* aMappedSurface) final {
     MutexAutoLock lock(mMutex);
     ++mMapCount;
     aMappedSurface->mData = GetDataInternal();
     aMappedSurface->mStride = mStride;
     return true;
   }
 
-  void Unmap() override {
+  void Unmap() final {
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(mMapCount > 0);
     if (--mMapCount == 0) {
       mOldBuf = nullptr;
     }
   }
 
   /**
@@ -227,29 +227,29 @@ class SourceSurfaceSharedData final : pu
   bool IsFinalized() const {
     MutexAutoLock lock(mMutex);
     return mFinalized;
   }
 
   /**
    * Yields a dirty rect of what has changed since it was last called.
    */
-  Maybe<IntRect> TakeDirtyRect() override {
+  Maybe<IntRect> TakeDirtyRect() final {
     MutexAutoLock lock(mMutex);
     if (mDirtyRect) {
       Maybe<IntRect> ret = std::move(mDirtyRect);
       return ret;
     }
     return Nothing();
   }
 
   /**
    * Increment the invalidation counter.
    */
-  void Invalidate(const IntRect& aDirtyRect) override {
+  void Invalidate(const IntRect& aDirtyRect) final {
     MutexAutoLock lock(mMutex);
     if (!aDirtyRect.IsEmpty()) {
       if (mDirtyRect) {
         mDirtyRect->UnionRect(mDirtyRect.ref(), aDirtyRect);
       } else {
         mDirtyRect = Some(aDirtyRect);
       }
     } else {
@@ -270,21 +270,22 @@ class SourceSurfaceSharedData final : pu
     }
 
     ~HandleLock() { mSurface->UnlockHandle(); }
 
    private:
     RefPtr<SourceSurfaceSharedData> mSurface;
   };
 
+ protected:
+  virtual ~SourceSurfaceSharedData() = default;
+
  private:
   friend class SourceSurfaceSharedDataWrapper;
 
-  virtual ~SourceSurfaceSharedData() = default;
-
   void LockHandle() {
     MutexAutoLock lock(mMutex);
     ++mHandleCount;
   }
 
   void UnlockHandle() {
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(mHandleCount > 0);
--- a/gfx/layers/ipc/SharedSurfacesChild.cpp
+++ b/gfx/layers/ipc/SharedSurfacesChild.cpp
@@ -3,17 +3,16 @@
 /* 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 "SharedSurfacesChild.h"
 #include "SharedSurfacesParent.h"
 #include "CompositorManagerChild.h"
 #include "mozilla/gfx/gfxVars.h"
-#include "mozilla/image/RecyclingSourceSurface.h"
 #include "mozilla/layers/IpcResourceUpdateQueue.h"
 #include "mozilla/layers/SourceSurfaceSharedData.h"
 #include "mozilla/layers/WebRenderBridgeChild.h"
 #include "mozilla/layers/RenderRootStateManager.h"
 #include "mozilla/SchedulerGroup.h"
 #include "mozilla/StaticPrefs_image.h"
 
 namespace mozilla {
@@ -152,23 +151,18 @@ wr::ImageKey SharedSurfacesChild::Shared
 }
 
 /* static */
 SourceSurfaceSharedData* SharedSurfacesChild::AsSourceSurfaceSharedData(
     SourceSurface* aSurface) {
   MOZ_ASSERT(aSurface);
   switch (aSurface->GetType()) {
     case SurfaceType::DATA_SHARED:
+    case SurfaceType::DATA_RECYCLING_SHARED:
       return static_cast<SourceSurfaceSharedData*>(aSurface);
-    case SurfaceType::DATA_RECYCLING_SHARED: {
-      auto recycleSurface =
-          static_cast<image::RecyclingSourceSurface*>(aSurface);
-      auto childSurface = recycleSurface->GetChildSurface();
-      return static_cast<SourceSurfaceSharedData*>(childSurface);
-    }
     default:
       return nullptr;
   }
 }
 
 /* static */
 nsresult SharedSurfacesChild::ShareInternal(SourceSurfaceSharedData* aSurface,
                                             SharedUserData** aUserData) {
@@ -365,17 +359,17 @@ nsresult SharedSurfacesChild::Share(Imag
 
   auto sharedSurface = AsSourceSurfaceSharedData(surface);
   if (!sharedSurface) {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   SharedSurfacesAnimation* anim = aContainer->GetSharedSurfacesAnimation();
   if (anim) {
-    return anim->UpdateKey(surface, sharedSurface, aManager, aResources, aKey);
+    return anim->UpdateKey(sharedSurface, aManager, aResources, aKey);
   }
 
   return Share(sharedSurface, aManager, aResources, aKey);
 }
 
 /* static */
 nsresult SharedSurfacesChild::Share(SourceSurface* aSurface,
                                     wr::ExternalImageId& aId) {
@@ -468,17 +462,17 @@ nsresult SharedSurfacesChild::UpdateAnim
   if (!sharedSurface) {
     MOZ_ASSERT(!aContainer->GetSharedSurfacesAnimation());
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   SharedSurfacesAnimation* anim = aContainer->EnsureSharedSurfacesAnimation();
   MOZ_ASSERT(anim);
 
-  return anim->SetCurrentFrame(aSurface, sharedSurface, aDirtyRect);
+  return anim->SetCurrentFrame(sharedSurface, aDirtyRect);
 }
 
 AnimationImageKeyData::AnimationImageKeyData(RenderRootStateManager* aManager,
                                              const wr::ImageKey& aImageKey)
     : SharedSurfacesChild::ImageKeyData(aManager, aImageKey) {}
 
 AnimationImageKeyData::AnimationImageKeyData(AnimationImageKeyData&& aOther)
     : SharedSurfacesChild::ImageKeyData(std::move(aOther)),
@@ -517,29 +511,27 @@ void SharedSurfacesAnimation::Destroy() 
     }
     entry.mManager->AddImageKeyForDiscard(entry.mImageKey);
   }
 
   mKeys.Clear();
 }
 
 void SharedSurfacesAnimation::HoldSurfaceForRecycling(
-    AnimationImageKeyData& aEntry, SourceSurface* aParentSurface,
-    SourceSurfaceSharedData* aSurface) {
-  if (aParentSurface == static_cast<SourceSurface*>(aSurface)) {
+    AnimationImageKeyData& aEntry, SourceSurfaceSharedData* aSurface) {
+  if (aSurface->GetType() != SurfaceType::DATA_RECYCLING_SHARED) {
     return;
   }
 
   MOZ_ASSERT(StaticPrefs::image_animated_decode_on_demand_recycle_AtStartup());
-  aEntry.mPendingRelease.AppendElement(aParentSurface);
+  aEntry.mPendingRelease.AppendElement(aSurface);
 }
 
 nsresult SharedSurfacesAnimation::SetCurrentFrame(
-    SourceSurface* aParentSurface, SourceSurfaceSharedData* aSurface,
-    const gfx::IntRect& aDirtyRect) {
+    SourceSurfaceSharedData* aSurface, const gfx::IntRect& aDirtyRect) {
   MOZ_ASSERT(aSurface);
 
   SharedSurfacesChild::SharedUserData* data = nullptr;
   nsresult rv = SharedSurfacesChild::ShareInternal(aSurface, &data);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
@@ -550,30 +542,29 @@ nsresult SharedSurfacesAnimation::SetCur
   while (i > 0) {
     --i;
     AnimationImageKeyData& entry = mKeys[i];
     MOZ_ASSERT(!entry.mManager->IsDestroyed());
 
     entry.MergeDirtyRect(Some(aDirtyRect));
     Maybe<IntRect> dirtyRect = entry.TakeDirtyRect();
     if (dirtyRect) {
-      HoldSurfaceForRecycling(entry, aParentSurface, aSurface);
+      HoldSurfaceForRecycling(entry, aSurface);
       auto& resourceUpdates = entry.mManager->AsyncResourceUpdates();
       resourceUpdates.UpdateSharedExternalImage(
           mId, entry.mImageKey, ViewAs<ImagePixel>(dirtyRect.ref()));
     }
   }
 
   return NS_OK;
 }
 
 nsresult SharedSurfacesAnimation::UpdateKey(
-    SourceSurface* aParentSurface, SourceSurfaceSharedData* aSurface,
-    RenderRootStateManager* aManager, wr::IpcResourceUpdateQueue& aResources,
-    wr::ImageKey& aKey) {
+    SourceSurfaceSharedData* aSurface, RenderRootStateManager* aManager,
+    wr::IpcResourceUpdateQueue& aResources, wr::ImageKey& aKey) {
   SharedSurfacesChild::SharedUserData* data = nullptr;
   nsresult rv = SharedSurfacesChild::ShareInternal(aSurface, &data);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   MOZ_ASSERT(data);
   if (wr::AsUint64(mId) != wr::AsUint64(data->Id())) {
@@ -598,17 +589,17 @@ nsresult SharedSurfacesAnimation::Update
       MOZ_ASSERT(wrBridge);
 
       // Even if the manager is the same, its underlying WebRenderBridgeChild
       // can change state. If our namespace differs, then our old key has
       // already been discarded.
       bool ownsKey = wrBridge->GetNamespace() == entry.mImageKey.mNamespace;
       if (!ownsKey) {
         entry.mImageKey = wrBridge->GetNextImageKey();
-        HoldSurfaceForRecycling(entry, aParentSurface, aSurface);
+        HoldSurfaceForRecycling(entry, aSurface);
         aResources.AddSharedExternalImage(mId, entry.mImageKey);
       } else {
         MOZ_ASSERT(entry.mDirtyRect.isNothing());
       }
 
       aKey = entry.mImageKey;
       found = true;
       break;
@@ -617,17 +608,17 @@ nsresult SharedSurfacesAnimation::Update
 
   if (!found) {
     aKey = aManager->WrBridge()->GetNextImageKey();
     if (StaticPrefs::image_animated_decode_on_demand_recycle_AtStartup()) {
       aManager->RegisterAsyncAnimation(aKey, this);
     }
 
     AnimationImageKeyData data(aManager, aKey);
-    HoldSurfaceForRecycling(data, aParentSurface, aSurface);
+    HoldSurfaceForRecycling(data, aSurface);
     mKeys.AppendElement(std::move(data));
     aResources.AddSharedExternalImage(mId, aKey);
   }
 
   return NS_OK;
 }
 
 void SharedSurfacesAnimation::ReleasePreviousFrame(
@@ -637,22 +628,18 @@ void SharedSurfacesAnimation::ReleasePre
   auto i = mKeys.Length();
   while (i > 0) {
     --i;
     AnimationImageKeyData& entry = mKeys[i];
     MOZ_ASSERT(!entry.mManager->IsDestroyed());
     if (entry.mManager == aManager) {
       size_t k;
       for (k = 0; k < entry.mPendingRelease.Length(); ++k) {
-        auto sharedSurface = SharedSurfacesChild::AsSourceSurfaceSharedData(
-            entry.mPendingRelease[k]);
-        MOZ_ASSERT(sharedSurface);
-
         Maybe<wr::ExternalImageId> extId =
-            SharedSurfacesChild::GetExternalId(sharedSurface);
+            SharedSurfacesChild::GetExternalId(entry.mPendingRelease[k]);
         if (extId && extId.ref() == aId) {
           break;
         }
       }
 
       if (k == entry.mPendingRelease.Length()) {
         continue;
       }
--- a/gfx/layers/ipc/SharedSurfacesChild.h
+++ b/gfx/layers/ipc/SharedSurfacesChild.h
@@ -196,56 +196,48 @@ class AnimationImageKeyData final : publ
   AnimationImageKeyData(RenderRootStateManager* aManager,
                         const wr::ImageKey& aImageKey);
 
   virtual ~AnimationImageKeyData();
 
   AnimationImageKeyData(AnimationImageKeyData&& aOther);
   AnimationImageKeyData& operator=(AnimationImageKeyData&& aOther);
 
-  AutoTArray<RefPtr<gfx::SourceSurface>, 2> mPendingRelease;
+  AutoTArray<RefPtr<gfx::SourceSurfaceSharedData>, 2> mPendingRelease;
 };
 
 /**
  * This helper class owns a single ImageKey which will map to different external
  * image IDs representing different frames in an animation.
  */
 class SharedSurfacesAnimation final {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SharedSurfacesAnimation)
 
   SharedSurfacesAnimation() = default;
 
   void Destroy();
 
   /**
    * Set the animation to display the given frame.
-   * @param aParentSurface The owning surface of aSurface. This may be the same
-   *                       or it may be a wrapper surface such as
-   *                       RecyclingSourceSurface.
    * @param aSurface    The current frame.
    * @param aDirtyRect  Dirty rect representing the change between the new frame
    *                    and the previous frame. We will request only the delta
    *                    be reuploaded by WebRender.
    */
-  nsresult SetCurrentFrame(gfx::SourceSurface* aParentSurface,
-                           gfx::SourceSurfaceSharedData* aSurface,
+  nsresult SetCurrentFrame(gfx::SourceSurfaceSharedData* aSurface,
                            const gfx::IntRect& aDirtyRect);
 
   /**
    * Generate an ImageKey for the given frame.
-   * @param aParentSurface The owning surface of aSurface. This may be the same
-   *                       or it may be a wrapper surface such as
-   *                       RecyclingSourceSurface.
    * @param aSurface  The current frame. This should match what was cached via
    *                  SetCurrentFrame, but if it does not, it will need to
    *                  regenerate the cached ImageKey.
    */
-  nsresult UpdateKey(gfx::SourceSurface* aParentSurface,
-                     gfx::SourceSurfaceSharedData* aSurface,
+  nsresult UpdateKey(gfx::SourceSurfaceSharedData* aSurface,
                      RenderRootStateManager* aManager,
                      wr::IpcResourceUpdateQueue& aResources,
                      wr::ImageKey& aKey);
 
   /**
    * Release our reference to all frames up to and including the frame which
    * has an external image ID which matches aId.
    */
@@ -257,17 +249,16 @@ class SharedSurfacesAnimation final {
    * image keys are already invalid.
    */
   void Invalidate(RenderRootStateManager* aManager);
 
  private:
   ~SharedSurfacesAnimation();
 
   void HoldSurfaceForRecycling(AnimationImageKeyData& aEntry,
-                               gfx::SourceSurface* aParentSurface,
                                gfx::SourceSurfaceSharedData* aSurface);
 
   AutoTArray<AnimationImageKeyData, 1> mKeys;
   wr::ExternalImageId mId;
 };
 
 }  // namespace layers
 }  // namespace mozilla
deleted file mode 100644
--- a/image/RecyclingSourceSurface.h
+++ /dev/null
@@ -1,66 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=2 et sw=2 tw=80: */
-/* 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/. */
-
-#ifndef mozilla_image_RecyclingSourceSurface_h
-#define mozilla_image_RecyclingSourceSurface_h
-
-#include "mozilla/gfx/2D.h"
-#include "mozilla/NotNull.h"
-
-namespace mozilla {
-namespace image {
-
-class imgFrame;
-
-/**
- * This surface subclass will prevent the underlying surface from being recycled
- * as long as it is still alive. We will create this surface to wrap imgFrame's
- * mLockedSurface, if we are accessing it on a path that will keep the surface
- * alive for an indeterminate period of time (e.g. imgFrame::GetSourceSurface,
- * imgFrame::Draw with a recording or capture DrawTarget).
- */
-class RecyclingSourceSurface final : public gfx::DataSourceSurface {
- public:
-  RecyclingSourceSurface(imgFrame* aParent, gfx::DataSourceSurface* aSurface);
-
-  MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(RecyclingSourceSurface, override);
-
-  uint8_t* GetData() override { return mSurface->GetData(); }
-  int32_t Stride() override { return mSurface->Stride(); }
-  gfx::SurfaceType GetType() const override { return mType; }
-  gfx::IntSize GetSize() const override { return mSurface->GetSize(); }
-  gfx::SurfaceFormat GetFormat() const override {
-    return mSurface->GetFormat();
-  }
-
-  bool OnHeap() const override { return mSurface->OnHeap(); }
-  bool Map(MapType aType, MappedSurface* aMappedSurface) override {
-    return mSurface->Map(aType, aMappedSurface);
-  }
-  void Unmap() override { mSurface->Unmap(); }
-
-  void SizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
-                           SizeOfInfo& aInfo) const override {
-    aInfo.AddType(mType);
-    mSurface->SizeOfExcludingThis(aMallocSizeOf, aInfo);
-  }
-
-  gfx::DataSourceSurface* GetChildSurface() const { return mSurface.get(); }
-
- protected:
-  void GuaranteePersistance() override {}
-
-  ~RecyclingSourceSurface() override;
-
-  NotNull<RefPtr<imgFrame>> mParent;
-  NotNull<RefPtr<DataSourceSurface>> mSurface;
-  gfx::SurfaceType mType;
-};
-
-}  // namespace image
-}  // namespace mozilla
-
-#endif  // mozilla_image_RecyclingSourceSurface_h
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -17,42 +17,59 @@
 #include "gfxUtils.h"
 
 #include "GeckoProfiler.h"
 #include "MainThreadUtils.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/gfx/gfxVars.h"
 #include "mozilla/gfx/Tools.h"
 #include "mozilla/gfx/SourceSurfaceRawData.h"
-#include "mozilla/image/RecyclingSourceSurface.h"
 #include "mozilla/layers/SourceSurfaceSharedData.h"
 #include "mozilla/layers/SourceSurfaceVolatileData.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/StaticPrefs_browser.h"
 #include "nsMargin.h"
 #include "nsRefreshDriver.h"
 #include "nsThreadUtils.h"
 
+#include <algorithm>  // for min, max
+
 namespace mozilla {
 
 using namespace gfx;
 
 namespace image {
 
+/**
+ * This class is identical to SourceSurfaceSharedData but returns a different
+ * type so that SharedSurfacesChild is aware imagelib wants to recycle this
+ * surface for future animation frames.
+ */
+class RecyclingSourceSurfaceSharedData final : public SourceSurfaceSharedData {
+ public:
+  MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(RecyclingSourceSurfaceSharedData,
+                                          override)
+
+  SurfaceType GetType() const override {
+    return SurfaceType::DATA_RECYCLING_SHARED;
+  }
+};
+
 static int32_t VolatileSurfaceStride(const IntSize& size,
                                      SurfaceFormat format) {
   // Stride must be a multiple of four or cairo will complain.
   return (size.width * BytesPerPixel(format) + 0x3) & ~0x3;
 }
 
 static already_AddRefed<DataSourceSurface> CreateLockedSurface(
     DataSourceSurface* aSurface, const IntSize& size, SurfaceFormat format) {
   switch (aSurface->GetType()) {
     case SurfaceType::DATA_SHARED:
+    case SurfaceType::DATA_RECYCLING_SHARED:
     case SurfaceType::DATA_ALIGNED: {
       // Shared memory is never released until the surface itself is released.
       // Similar for aligned/heap surfaces.
       RefPtr<DataSourceSurface> surf(aSurface);
       return surf.forget();
     }
     default: {
       // Volatile memory requires us to map it first, and it is fallible.
@@ -81,29 +98,31 @@ static bool ShouldUseHeap(const IntSize&
   // released to let the OS purge volatile buffers.
   if (aIsAnimated && StaticPrefs::image_mem_animated_use_heap()) {
     return true;
   }
 
   // Lets us avoid too many small images consuming all of the handles. The
   // actual allocation checks for overflow.
   int32_t bufferSize = (aStride * aSize.height) / 1024;
-  if (bufferSize < StaticPrefs::image_mem_volatile_min_threshold_kb()) {
-    return true;
-  }
-
-  return false;
+  return bufferSize < StaticPrefs::image_mem_volatile_min_threshold_kb();
 }
 
 static already_AddRefed<DataSourceSurface> AllocateBufferForImage(
-    const IntSize& size, SurfaceFormat format, bool aIsAnimated = false) {
+    const IntSize& size, SurfaceFormat format, bool aShouldRecycle = false,
+    bool aIsAnimated = false) {
   int32_t stride = VolatileSurfaceStride(size, format);
 
   if (gfxVars::GetUseWebRenderOrDefault() && StaticPrefs::image_mem_shared()) {
-    RefPtr<SourceSurfaceSharedData> newSurf = new SourceSurfaceSharedData();
+    RefPtr<SourceSurfaceSharedData> newSurf;
+    if (aShouldRecycle) {
+      newSurf = new RecyclingSourceSurfaceSharedData();
+    } else {
+      newSurf = new SourceSurfaceSharedData();
+    }
     if (newSurf->Init(size, stride, format)) {
       return newSurf.forget();
     }
   } else if (ShouldUseHeap(size, stride, aIsAnimated)) {
     RefPtr<SourceSurfaceAlignedRawData> newSurf =
         new SourceSurfaceAlignedRawData();
     if (newSurf->Init(size, format, false, 0, stride)) {
       return newSurf.forget();
@@ -171,17 +190,16 @@ static bool ClearSurface(DataSourceSurfa
 
   return true;
 }
 
 imgFrame::imgFrame()
     : mMonitor("imgFrame"),
       mDecoded(0, 0, 0, 0),
       mLockCount(0),
-      mRecycleLockCount(0),
       mAborted(false),
       mFinished(false),
       mOptimizable(false),
       mShouldRecycle(false),
       mTimeout(FrameTimeout::FromRawMilliseconds(100)),
       mDisposalMethod(DisposalMethod::NOT_SPECIFIED),
       mBlendMethod(BlendMethod::OVER),
       mFormat(SurfaceFormat::UNKNOWN),
@@ -234,17 +252,18 @@ nsresult imgFrame::InitForDecoder(const 
   }
 
   mNonPremult = aNonPremult;
   mShouldRecycle = aShouldRecycle;
 
   MOZ_ASSERT(!mLockedSurface, "Called imgFrame::InitForDecoder() twice?");
 
   bool postFirstFrame = aAnimParams && aAnimParams->mFrameNum > 0;
-  mRawSurface = AllocateBufferForImage(mImageSize, mFormat, postFirstFrame);
+  mRawSurface = AllocateBufferForImage(mImageSize, mFormat, mShouldRecycle,
+                                       postFirstFrame);
   if (!mRawSurface) {
     mAborted = true;
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   if (StaticPrefs::browser_measurement_render_anims_and_video_solid() &&
       aAnimParams) {
     mBlankRawSurface = AllocateBufferForImage(mImageSize, mFormat);
@@ -297,17 +316,26 @@ nsresult imgFrame::InitForDecoderRecycle
   MOZ_ASSERT(mLockedSurface);
 
   if (!mShouldRecycle) {
     // This frame either was never marked as recyclable, or the flag was cleared
     // for a caller which does not support recycling.
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  if (mRecycleLockCount > 0) {
+  // Ensure we account for all internal references to the surface.
+  MozRefCountType internalRefs = 1;
+  if (mRawSurface == mLockedSurface) {
+    ++internalRefs;
+  }
+  if (mOptSurface == mLockedSurface) {
+    ++internalRefs;
+  }
+
+  if (mLockedSurface->refCount() > internalRefs) {
     if (NS_IsMainThread()) {
       // We should never be both decoding and recycling on the main thread. Sync
       // decoding can only be used to produce the first set of frames. Those
       // either never use recycling because advancing was blocked (main thread
       // is busy) or we were auto-advancing (to seek to a frame) and the frames
       // were never accessed (and thus cannot have recycle locks).
       MOZ_ASSERT_UNREACHABLE("Recycling/decoding on the main thread?");
       return NS_ERROR_NOT_AVAILABLE;
@@ -319,33 +347,33 @@ nsresult imgFrame::InitForDecoderRecycle
     // animation, we can afford to wait a short period of time to hopefully
     // complete the transaction and reclaim the buffer.
     //
     // We choose to wait for, at most, the refresh driver interval, so that we
     // won't skip more than one frame. If the frame is still in use due to
     // outstanding transactions, we are already skipping frames. If the frame
     // is still in use for some other purpose, it won't be returned to the pool
     // and its owner can hold onto it forever without additional impact here.
-    TimeDuration timeout =
-        TimeDuration::FromMilliseconds(nsRefreshDriver::DefaultInterval());
+    int32_t refreshInterval =
+        std::max(std::min(nsRefreshDriver::DefaultInterval(), 20), 4);
+    TimeDuration waitInterval =
+        TimeDuration::FromMilliseconds(refreshInterval >> 2);
+    TimeStamp timeout =
+        TimeStamp::Now() + TimeDuration::FromMilliseconds(refreshInterval);
     while (true) {
-      TimeStamp start = TimeStamp::Now();
-      mMonitor.Wait(timeout);
-      if (mRecycleLockCount == 0) {
+      mMonitor.Wait(waitInterval);
+      if (mLockedSurface->refCount() <= internalRefs) {
         break;
       }
 
-      TimeDuration delta = TimeStamp::Now() - start;
-      if (delta >= timeout) {
+      if (timeout <= TimeStamp::Now()) {
         // We couldn't secure the frame for recycling. It will allocate a new
         // frame instead.
         return NS_ERROR_NOT_AVAILABLE;
       }
-
-      timeout -= delta;
     }
   }
 
   mBlendRect = aAnimParams.mBlendRect;
   mTimeout = aAnimParams.mTimeout;
   mBlendMethod = aAnimParams.mBlendMethod;
   mDisposalMethod = aAnimParams.mDisposalMethod;
   mDirtyRect = GetRect();
@@ -607,18 +635,17 @@ bool imgFrame::Draw(gfxContext* aContext
     // Most draw targets will just use the surface only during DrawPixelSnapped
     // but captures/recordings will retain a reference outside this stack
     // context. While in theory a decoder thread could be trying to recycle this
     // frame at this very moment, in practice the only way we can get here is if
     // this frame is the current frame of the animation. Since we can only
     // advance on the main thread, we know nothing else will try to use it.
     DrawTarget* drawTarget = aContext->GetDrawTarget();
     bool recording = drawTarget->GetBackendType() == BackendType::RECORDING;
-    bool temporary = !drawTarget->IsCaptureDT() && !recording;
-    RefPtr<SourceSurface> surf = GetSourceSurfaceInternal(temporary);
+    RefPtr<SourceSurface> surf = GetSourceSurfaceInternal();
     if (!surf) {
       return false;
     }
 
     bool doTile = !imageRect.Contains(aRegion.Rect()) &&
                   !(aImageFlags & imgIContainer::FLAG_CLAMP);
 
     surfaceResult = SurfaceForDrawing(doPartialDecode, doTile, region, surf);
@@ -814,64 +841,45 @@ void imgFrame::FinalizeSurfaceInternal()
   mMonitor.AssertCurrentThreadOwns();
 
   // Not all images will have mRawSurface to finalize (i.e. paletted images).
   if (mShouldRecycle || !mRawSurface ||
       mRawSurface->GetType() != SurfaceType::DATA_SHARED) {
     return;
   }
 
-  auto sharedSurf = static_cast<SourceSurfaceSharedData*>(mRawSurface.get());
+  auto* sharedSurf = static_cast<SourceSurfaceSharedData*>(mRawSurface.get());
   sharedSurf->Finalize();
 }
 
 already_AddRefed<SourceSurface> imgFrame::GetSourceSurface() {
   MonitorAutoLock lock(mMonitor);
-  return GetSourceSurfaceInternal(/* aTemporary */ false);
+  return GetSourceSurfaceInternal();
 }
 
-already_AddRefed<SourceSurface> imgFrame::GetSourceSurfaceInternal(
-    bool aTemporary) {
+already_AddRefed<SourceSurface> imgFrame::GetSourceSurfaceInternal() {
   mMonitor.AssertCurrentThreadOwns();
 
   if (mOptSurface) {
     if (mOptSurface->IsValid()) {
       RefPtr<SourceSurface> surf(mOptSurface);
       return surf.forget();
-    } else {
-      mOptSurface = nullptr;
     }
+    mOptSurface = nullptr;
   }
 
   if (mBlankLockedSurface) {
     // We are going to return the blank surface because of the flags.
     // We are including comments here that are copied from below
     // just so that we are on the same page!
-
-    // We don't need to create recycling wrapper for some callers because they
-    // promise to release the surface immediately after.
-    if (!aTemporary && mShouldRecycle) {
-      RefPtr<SourceSurface> surf =
-          new RecyclingSourceSurface(this, mBlankLockedSurface);
-      return surf.forget();
-    }
-
     RefPtr<SourceSurface> surf(mBlankLockedSurface);
     return surf.forget();
   }
 
   if (mLockedSurface) {
-    // We don't need to create recycling wrapper for some callers because they
-    // promise to release the surface immediately after.
-    if (!aTemporary && mShouldRecycle) {
-      RefPtr<SourceSurface> surf =
-          new RecyclingSourceSurface(this, mLockedSurface);
-      return surf.forget();
-    }
-
     RefPtr<SourceSurface> surf(mLockedSurface);
     return surf.forget();
   }
 
   MOZ_ASSERT(!mShouldRecycle, "Should recycle but no locked surface!");
 
   if (!mRawSurface) {
     return nullptr;
@@ -944,31 +952,10 @@ void imgFrame::AddSizeOfExcludingThis(Ma
     SourceSurface::SizeOfInfo info;
     mRawSurface->SizeOfExcludingThis(aMallocSizeOf, info);
     metadata.Accumulate(info);
   }
 
   aCallback(metadata);
 }
 
-RecyclingSourceSurface::RecyclingSourceSurface(imgFrame* aParent,
-                                               DataSourceSurface* aSurface)
-    : mParent(WrapNotNull(aParent)),
-      mSurface(WrapNotNull(aSurface)),
-      mType(SurfaceType::DATA) {
-  mParent->mMonitor.AssertCurrentThreadOwns();
-  ++mParent->mRecycleLockCount;
-
-  if (aSurface->GetType() == SurfaceType::DATA_SHARED) {
-    mType = SurfaceType::DATA_RECYCLING_SHARED;
-  }
-}
-
-RecyclingSourceSurface::~RecyclingSourceSurface() {
-  MonitorAutoLock lock(mParent->mMonitor);
-  MOZ_ASSERT(mParent->mRecycleLockCount > 0);
-  if (--mParent->mRecycleLockCount == 0) {
-    mParent->mMonitor.NotifyAll();
-  }
-}
-
 }  // namespace image
 }  // namespace mozilla
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -214,24 +214,17 @@ class imgFrame {
   void AssertImageDataLocked() const;
 
   bool AreAllPixelsWritten() const;
   nsresult ImageUpdatedInternal(const nsIntRect& aUpdateRect);
   void GetImageDataInternal(uint8_t** aData, uint32_t* length) const;
   uint32_t GetImageBytesPerRow() const;
   uint32_t GetImageDataLength() const;
   void FinalizeSurfaceInternal();
-
-  /**
-   * @param aTemporary  If true, it will assume the caller does not require a
-   *                    wrapping RecycleSourceSurface to protect the underlying
-   *                    surface from recycling. The reference to the surface
-   *                    must be freed before releasing the main thread context.
-   */
-  already_AddRefed<SourceSurface> GetSourceSurfaceInternal(bool aTemporary);
+  already_AddRefed<SourceSurface> GetSourceSurfaceInternal();
 
   struct SurfaceWithFormat {
     RefPtr<gfxDrawable> mDrawable;
     SurfaceFormat mFormat;
     SurfaceWithFormat() : mFormat(SurfaceFormat::UNKNOWN) {}
     SurfaceWithFormat(gfxDrawable* aDrawable, SurfaceFormat aFormat)
         : mDrawable(aDrawable), mFormat(aFormat) {}
     SurfaceWithFormat(SurfaceWithFormat&& aOther)
@@ -248,17 +241,16 @@ class imgFrame {
 
   SurfaceWithFormat SurfaceForDrawing(bool aDoPartialDecode, bool aDoTile,
                                       ImageRegion& aRegion,
                                       SourceSurface* aSurface);
 
  private:  // data
   friend class DrawableFrameRef;
   friend class RawAccessFrameRef;
-  friend class RecyclingSourceSurface;
   friend class UnlockImageDataRunnable;
 
   //////////////////////////////////////////////////////////////////////////////
   // Thread-safe mutable data, protected by mMonitor.
   //////////////////////////////////////////////////////////////////////////////
 
   mutable Monitor mMonitor;
 
@@ -284,19 +276,16 @@ class imgFrame {
    */
   RefPtr<SourceSurface> mOptSurface;
 
   nsIntRect mDecoded;
 
   //! Number of RawAccessFrameRefs currently alive for this imgFrame.
   int16_t mLockCount;
 
-  //! Number of RecyclingSourceSurface's currently alive for this imgFrame.
-  int16_t mRecycleLockCount;
-
   bool mAborted;
   bool mFinished;
   bool mOptimizable;
   bool mShouldRecycle;
 
   //////////////////////////////////////////////////////////////////////////////
   // Effectively const data, only mutated in the Init methods.
   //////////////////////////////////////////////////////////////////////////////
--- a/image/moz.build
+++ b/image/moz.build
@@ -56,17 +56,16 @@ EXPORTS += [
 
 EXPORTS.mozilla.image += [
     'encoders/bmp/nsBMPEncoder.h',
     'encoders/ico/nsICOEncoder.h',
     'encoders/jpeg/nsJPEGEncoder.h',
     'encoders/png/nsPNGEncoder.h',
     'ICOFileHeaders.h',
     'ImageMemoryReporter.h',
-    'RecyclingSourceSurface.h',
 ]
 
 UNIFIED_SOURCES += [
     'AnimationFrameBuffer.cpp',
     'AnimationSurfaceProvider.cpp',
     'ClippedImage.cpp',
     'DecodedSurfaceProvider.cpp',
     'Decoder.cpp',