Issue #2073 - m-c 1651587: Make image::Image release efficient on main thread

6 files changed, 75 insertions, 13 deletions

diff --git a/image/AnimationSurfaceProvider.cpp b/image/AnimationSurfaceProvider.cpp
index 1d76332930..16c872ec5e 100644
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -61,14 +61,8 @@ AnimationSurfaceProvider::DropImageReference()
     return;  // Nothing to do.
   }
 
-  // RasterImage objects need to be destroyed on the main thread. We also need
-  // to destroy them asynchronously, because if our surface cache entry is
-  // destroyed and we were the only thing keeping |mImage| alive, RasterImage's
-  // destructor may call into the surface cache while whatever code caused us to
-  // get evicted is holding the surface cache lock, causing deadlock.
-  RefPtr<RasterImage> image = mImage;
-  mImage = nullptr;
-  NS_ReleaseOnMainThread(image.forget(), /* aAlwaysProxy = */ true);
+  // RasterImage objects need to be destroyed on the main thread.
+  SurfaceCache::ReleaseImageOnMainThread(mImage.forget());
 }
 
 void
diff --git a/image/DecodedSurfaceProvider.cpp b/image/DecodedSurfaceProvider.cpp
index 45b4849d29..d24380a375 100644
--- a/image/DecodedSurfaceProvider.cpp
+++ b/image/DecodedSurfaceProvider.cpp
@@ -49,7 +49,8 @@ DecodedSurfaceProvider::DropImageReference()
   // get evicted is holding the surface cache lock, causing deadlock.
   RefPtr<RasterImage> image = mImage;
   mImage = nullptr;
-  NS_ReleaseOnMainThread(image.forget(), /* aAlwaysProxy = */ true);
+  SurfaceCache::ReleaseImageOnMainThread(image.forget(),
+                                         /* aAlwaysProxy = */ true);
 }
 
 DrawableFrameRef
diff --git a/image/Decoder.cpp b/image/Decoder.cpp
index 9d0647a4a3..8aaeb3127a 100644
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -54,7 +54,7 @@ Decoder::~Decoder()
   if (mImage && !NS_IsMainThread()) {
     // Dispatch mImage to main thread to prevent it from being destructed by the
     // decode thread.
-    NS_ReleaseOnMainThread(mImage.forget());
+    SurfaceCache::ReleaseImageOnMainThread(mImage.forget());
   }
 }
 
diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp
index 4c6cce891b..bb591920ef 100644
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -434,8 +434,7 @@ RasterImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey)
     mAnimationState && aSurfaceKey.Playback() == PlaybackType::eAnimated;
 
   RefPtr<RasterImage> image = this;
-  NS_DispatchToMainThread(NS_NewRunnableFunction(
-                            [=]() -> void {
+  NS_DispatchToMainThread(NS_NewRunnableFunction([=]() -> void {
     image->OnSurfaceDiscardedInternal(animatedFramesDiscarded);
   }));
 }
diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp
index f3068fd4c1..856ba162dc 100644
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -835,6 +835,32 @@ public:
     }
   }
 
+  void ReleaseImageOnMainThread(already_AddRefed<image::Image>&& aImage,
+                                const StaticMutexAutoLock& aAutoLock) {
+    RefPtr<image::Image> image = aImage;
+    if (!image) {
+      return;
+    }
+
+    bool needsDispatch = mReleasingImagesOnMainThread.IsEmpty();
+    mReleasingImagesOnMainThread.AppendElement(image);
+
+    if (!needsDispatch) {
+      // There is already a ongoing task for ClearReleasingImages().
+      return;
+    }
+
+    NS_DispatchToMainThread(NS_NewRunnableFunction([]() -> void {
+      SurfaceCache::ClearReleasingImages();
+    }));
+  }
+
+  void TakeReleasingImages(nsTArray<RefPtr<image::Image>>& aImage,
+                           const StaticMutexAutoLock& aAutoLock) {
+    MOZ_ASSERT(NS_IsMainThread());
+    aImage.SwapElements(mReleasingImagesOnMainThread);
+  }
+
 private:
   already_AddRefed<ImageSurfaceCache> GetImageCache(const ImageKey aImageKey)
   {
@@ -942,7 +968,8 @@ private:
   nsRefPtrHashtable<nsPtrHashKey<Image>,
     ImageSurfaceCache> mImageCaches;
   SurfaceTracker                          mExpirationTracker;
-  RefPtr<MemoryPressureObserver>        mMemoryPressureObserver;
+  RefPtr<MemoryPressureObserver>          mMemoryPressureObserver;
+  nsTArray<RefPtr<image::Image>>          mReleasingImagesOnMainThread;
   const uint32_t                          mDiscardFactor;
   const Cost                              mMaxCost;
   Cost                                    mAvailableCost;
@@ -1160,5 +1187,34 @@ SurfaceCache::MaximumCapacity()
   return sInstance->MaximumCapacity();
 }
 
+/* static */
+void SurfaceCache::ReleaseImageOnMainThread(
+    already_AddRefed<image::Image> aImage, bool aAlwaysProxy) {
+  if (NS_IsMainThread() && !aAlwaysProxy) {
+    RefPtr<image::Image> image = std::move(aImage);
+    return;
+  }
+
+  StaticMutexAutoLock lock(sInstanceMutex);
+  if (sInstance) {
+    sInstance->ReleaseImageOnMainThread(std::move(aImage), lock);
+  } else {
+    NS_ReleaseOnMainThread(std::move(aImage), /* aAlwaysProxy */ true);
+  }
+}
+
+/* static */
+void SurfaceCache::ClearReleasingImages() {
+  MOZ_ASSERT(NS_IsMainThread());
+
+  nsTArray<RefPtr<image::Image>> images;
+  {
+    StaticMutexAutoLock lock(sInstanceMutex);
+    if (sInstance) {
+      sInstance->TakeReleasingImages(images, lock);
+    }
+  }
+}
+
 } // namespace image
 } // namespace mozilla
diff --git a/image/SurfaceCache.h b/image/SurfaceCache.h
index e0c22c999c..518b589d34 100644
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -417,6 +417,18 @@ struct SurfaceCache
    */
   static size_t MaximumCapacity();
 
+  /**
+   * Release image on main thread.
+   * The function uses SurfaceCache to release pending releasing images quickly.
+   */
+  static void ReleaseImageOnMainThread(already_AddRefed<image::Image> aImage,
+                                       bool aAlwaysProxy = false);
+
+  /**
+   * Clear all pending releasing images.
+   */
+  static void ClearReleasingImages();
+
 private:
   virtual ~SurfaceCache() = 0;  // Forbid instantiation.
 };