From d03680a9e7afe00d900b04a87a48819aafdfe4a2 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sun, 20 Mar 2022 18:19:55 +0100
Subject: [PATCH] LibWeb: Always defer callbacks in
 ResourceClient::set_resource()

Previously, we'd invoke the load/fail callbacks synchronously for
resources that were already loaded and cached.

This patch uses deferred_invoke() in the already-loaded case to ensure
that we always invoke these callbacks in a consistent manner.

This isn't to fix a specific issue, but rather because I kept seeing
these callbacks being invoked synchronously on top of an already-tall
call stack, and it was hard to reason about what was going on.
---
 Userland/Libraries/LibWeb/Loader/Resource.cpp | 23 ++++++++++++++-----
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/Userland/Libraries/LibWeb/Loader/Resource.cpp b/Userland/Libraries/LibWeb/Loader/Resource.cpp
index dbef11c1cb..123909380c 100644
--- a/Userland/Libraries/LibWeb/Loader/Resource.cpp
+++ b/Userland/Libraries/LibWeb/Loader/Resource.cpp
@@ -6,6 +6,7 @@
 
 #include <AK/Debug.h>
 #include <AK/Function.h>
+#include <LibCore/EventLoop.h>
 #include <LibCore/MimeData.h>
 #include <LibWeb/HTML/HTMLImageElement.h>
 #include <LibWeb/Loader/Resource.h>
@@ -141,13 +142,23 @@ void ResourceClient::set_resource(Resource* resource)
 
         m_resource->register_client({}, *this);
 
-        // Make sure that reused resources also have their load callback fired.
-        if (resource->is_loaded())
-            resource_did_load();
+        // For resources that are already loaded, we fire their load/fail callbacks via the event loop.
+        // This ensures that these callbacks always happen in a consistent way, instead of being invoked
+        // synchronously in some cases, and asynchronously in others.
+        if (resource->is_loaded() || resource->is_failed()) {
+            Core::deferred_invoke([this, strong_resource = NonnullRefPtr { *m_resource }] {
+                if (m_resource != strong_resource.ptr())
+                    return;
 
-        // Make sure that reused resources also have their fail callback fired.
-        if (resource->is_failed())
-            resource_did_fail();
+                // Make sure that reused resources also have their load callback fired.
+                if (m_resource->is_loaded())
+                    resource_did_load();
+
+                // Make sure that reused resources also have their fail callback fired.
+                if (m_resource->is_failed())
+                    resource_did_fail();
+            });
+        }
     }
 }