From a701adee411fcece695625cb3dcedf9e4bb8d39f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Wed, 26 May 2021 11:08:10 +0000
Subject: [PATCH] Bug 1677555 - Always release imgRequestProxy async. r=tnikkel

In this particular case the issue wouldn't end up in any sort of memory
corruption if we didn't safely crash, but these are quite tricky to
reason about, so it's better to avoid the reentrancy altogether if
possible.

I tried to convert the fuzzer test-case in a crashtest but failed (as
in, it didn't crash without the patch under the test harness).

Differential Revision: https://phabricator.services.mozilla.com/D115943
---
 layout/style/ImageLoader.cpp   | 8 ++++++++
 layout/style/nsStyleStruct.cpp | 9 +++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/layout/style/ImageLoader.cpp b/layout/style/ImageLoader.cpp
index 9dc9c32032e9..16feb4b31c78 100644
--- a/layout/style/ImageLoader.cpp
+++ b/layout/style/ImageLoader.cpp
@@ -84,6 +84,10 @@ void ImageLoader::Init() {
 
 /* static */
 void ImageLoader::Shutdown() {
+  for (const auto& entry : *sImages) {
+    entry.GetKey()->CancelAndForgetObserver(NS_BINDING_ABORTED);
+  }
+
   delete sImages;
   sImages = nullptr;
   sImageObserver = nullptr;
@@ -449,6 +453,10 @@ void ImageLoader::UnloadImage(imgRequestProxy* aImage) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aImage);
 
+  if (MOZ_UNLIKELY(!sImages)) {
+    return;  // Shutdown() takes care of it.
+  }
+
   auto lookup = sImages->Lookup(aImage);
   MOZ_DIAGNOSTIC_ASSERT(lookup, "Unregistered image?");
   if (MOZ_UNLIKELY(!lookup)) {
diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp
index 92e4ae34e28a..bf4ed243042a 100644
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -188,13 +188,10 @@ class StyleImageRequestCleanupTask final : public mozilla::Runnable {
 // This is defined here for parallelism with LoadURI.
 void Gecko_LoadData_Drop(StyleLoadData* aData) {
   if (aData->resolved_image) {
+    // We want to dispatch this async to prevent reentrancy issues, as
+    // imgRequestProxy going away can destroy documents, etc, see bug 1677555.
     auto task = MakeRefPtr<StyleImageRequestCleanupTask>(*aData);
-    if (NS_IsMainThread()) {
-      task->Run();
-    } else {
-      // if Resolve was not called at some point, mDocGroup is not set.
-      SchedulerGroup::Dispatch(TaskCategory::Other, task.forget());
-    }
+    SchedulerGroup::Dispatch(TaskCategory::Other, task.forget());
   }
 
   // URIs are safe to refcount from any thread.