From d188aaf288960ef00d1753b5ba77077a5c10c56b Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Thu, 31 Oct 2024 10:05:44 -0400
Subject: [PATCH] LibWeb: Protect animation frame callbacks from GC while they
 execute

Stealing the callbacks from the AnimationFrameCallbackDriver made them
no longer safe from GC. Continue to store them on the class until we
have finished their execution.
---
 .../expected/HTML/requestAnimationFrame-gc.txt     |  2 ++
 .../Text/input/HTML/requestAnimationFrame-gc.html  | 14 ++++++++++++++
 .../LibWeb/HTML/AnimationFrameCallbackDriver.cpp   |  8 ++++++--
 .../LibWeb/HTML/AnimationFrameCallbackDriver.h     |  1 +
 4 files changed, 23 insertions(+), 2 deletions(-)
 create mode 100644 Tests/LibWeb/Text/expected/HTML/requestAnimationFrame-gc.txt
 create mode 100644 Tests/LibWeb/Text/input/HTML/requestAnimationFrame-gc.html

diff --git a/Tests/LibWeb/Text/expected/HTML/requestAnimationFrame-gc.txt b/Tests/LibWeb/Text/expected/HTML/requestAnimationFrame-gc.txt
new file mode 100644
index 00000000000..c6eef0ccec7
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/HTML/requestAnimationFrame-gc.txt
@@ -0,0 +1,2 @@
+Collect garbage
+PASS! (Didn't crash)
diff --git a/Tests/LibWeb/Text/input/HTML/requestAnimationFrame-gc.html b/Tests/LibWeb/Text/input/HTML/requestAnimationFrame-gc.html
new file mode 100644
index 00000000000..0a1b63423bf
--- /dev/null
+++ b/Tests/LibWeb/Text/input/HTML/requestAnimationFrame-gc.html
@@ -0,0 +1,14 @@
+<script src="../include.js"></script>
+<script>
+    asyncTest((done) => {
+        requestAnimationFrame(() => {
+            println("Collect garbage");
+            internals.gc();
+        });
+
+        requestAnimationFrame(() => {
+            println("PASS! (Didn't crash)");
+            done();
+        });
+    });
+</script>
diff --git a/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.cpp b/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.cpp
index f0765c9164a..c21f866ca9a 100644
--- a/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.cpp
+++ b/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.cpp
@@ -6,6 +6,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/ScopeGuard.h>
 #include <LibWeb/HTML/AnimationFrameCallbackDriver.h>
 
 namespace Web::HTML {
@@ -16,6 +17,7 @@ void AnimationFrameCallbackDriver::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
     visitor.visit(m_callbacks);
+    visitor.visit(m_executing_callbacks);
 }
 
 WebIDL::UnsignedLong AnimationFrameCallbackDriver::add(Callback handler)
@@ -37,8 +39,10 @@ bool AnimationFrameCallbackDriver::has_callbacks() const
 
 void AnimationFrameCallbackDriver::run(double now)
 {
-    auto taken_callbacks = move(m_callbacks);
-    for (auto& [id, callback] : taken_callbacks)
+    AK::ScopeGuard guard { [&]() { m_executing_callbacks.clear(); } };
+    m_executing_callbacks = move(m_callbacks);
+
+    for (auto& [id, callback] : m_executing_callbacks)
         callback->function()(now);
 }
 
diff --git a/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.h b/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.h
index 24161a5d324..85f2e4e144e 100644
--- a/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.h
+++ b/Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.h
@@ -34,6 +34,7 @@ private:
     WebIDL::UnsignedLong m_animation_frame_callback_identifier { 0 };
 
     OrderedHashMap<WebIDL::UnsignedLong, Callback> m_callbacks;
+    OrderedHashMap<WebIDL::UnsignedLong, Callback> m_executing_callbacks;
 };
 
 }