LibWeb: Stop inactive requestAnimationFrame() callbacks from running

Previously requestAnimationFrame() callbacks were registered with a
static global RequestAnimationFrameDriver shared between all windows.
This led to callbacks still running after navigating away from a page
(This could be seen with the WASM GoL demo).

This commit moves the RequestAnimationFrameDriver (now
AnimationFrameCallbackDriver) to be a member of the HTML::Window
object, then uses the 'active document' parameter of
run_animation_frame_callbacks() to run only the active callbacks.

Userland/Libraries/LibWeb/HTML/AnimationFrameCallbackDriver.h

@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2022, the SerenityOS developers.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/Function.h>
+#include <AK/IDAllocator.h>
+#include <LibCore/Timer.h>
+#include <LibWeb/HTML/EventLoop/EventLoop.h>
+
+namespace Web::HTML {
+
+struct AnimationFrameCallbackDriver {
+    using Callback = Function<void(i32)>;
+
+    AnimationFrameCallbackDriver()
+    {
+        m_timer = Core::Timer::create_single_shot(16, [] {
+            HTML::main_thread_event_loop().schedule();
+        });
+    }
+
+    i32 add(Callback handler)
+    {
+        auto id = m_id_allocator.allocate();
+        m_callbacks.set(id, move(handler));
+        if (!m_timer->is_active())
+            m_timer->start();
+        return id;
+    }
+
+    bool remove(i32 id)
+    {
+        auto it = m_callbacks.find(id);
+        if (it == m_callbacks.end())
+            return false;
+        m_callbacks.remove(it);
+        m_id_allocator.deallocate(id);
+        return true;
+    }
+
+    void run()
+    {
+        auto taken_callbacks = move(m_callbacks);
+        for (auto& [id, callback] : taken_callbacks)
+            callback(id);
+    }
+
+    bool has_callbacks() const
+    {
+        return !m_callbacks.is_empty();
+    }
+
+private:
+    HashMap<i32, Callback> m_callbacks;
+    IDAllocator m_id_allocator;
+    RefPtr<Core::Timer> m_timer;
+};
+
+}

Userland/Libraries/LibWeb/HTML/Window.cpp

@@ -32,80 +32,11 @@
 
 namespace Web::HTML {
 
-class RequestAnimationFrameCallback : public RefCounted<RequestAnimationFrameCallback> {
-public:
-    explicit RequestAnimationFrameCallback(i32 id, Function<void(i32)> handler)
-        : m_id(id)
-        , m_handler(move(handler))
-    {
-    }
-    ~RequestAnimationFrameCallback() = default;
-
-    i32 id() const { return m_id; }
-    bool is_cancelled() const { return !m_handler; }
-
-    void cancel() { m_handler = nullptr; }
-    void invoke() { m_handler(m_id); }
-
-private:
-    i32 m_id { 0 };
-    Function<void(i32)> m_handler;
-};
-
-struct RequestAnimationFrameDriver {
-    RequestAnimationFrameDriver()
-    {
-        m_timer = Core::Timer::create_single_shot(16, [] {
-            HTML::main_thread_event_loop().schedule();
-        });
-    }
-
-    NonnullRefPtr<RequestAnimationFrameCallback> add(Function<void(i32)> handler)
-    {
-        auto id = m_id_allocator.allocate();
-        auto callback = adopt_ref(*new RequestAnimationFrameCallback { id, move(handler) });
-        m_callbacks.set(id, callback);
-        if (!m_timer->is_active())
-            m_timer->start();
-        return callback;
-    }
-
-    bool remove(i32 id)
-    {
-        auto it = m_callbacks.find(id);
-        if (it == m_callbacks.end())
-            return false;
-        m_callbacks.remove(it);
-        m_id_allocator.deallocate(id);
-        return true;
-    }
-
-    void run()
-    {
-        auto taken_callbacks = move(m_callbacks);
-        for (auto& it : taken_callbacks) {
-            if (!it.value->is_cancelled())
-                it.value->invoke();
-        }
-    }
-
-private:
-    HashMap<i32, NonnullRefPtr<RequestAnimationFrameCallback>> m_callbacks;
-    IDAllocator m_id_allocator;
-    RefPtr<Core::Timer> m_timer;
-};
-
-static RequestAnimationFrameDriver& request_animation_frame_driver()
-{
-    static RequestAnimationFrameDriver driver;
-    return driver;
-}
-
 // https://html.spec.whatwg.org/#run-the-animation-frame-callbacks
-void run_animation_frame_callbacks(DOM::Document&, double)
+void run_animation_frame_callbacks(DOM::Document& document, double)
 {
     // FIXME: Bring this closer to the spec.
-    request_animation_frame_driver().run();
+    document.window().animation_frame_callback_driver().run();
 }
 
 class IdleCallback : public RefCounted<IdleCallback> {
@@ -295,26 +226,19 @@ i32 Window::run_timer_initialization_steps(Bindings::TimerHandler handler, i32 t
 // https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html#run-the-animation-frame-callbacks
 i32 Window::request_animation_frame(NonnullOwnPtr<Bindings::CallbackType> js_callback)
 {
-    auto callback = request_animation_frame_driver().add([this, js_callback = move(js_callback)](i32 id) mutable {
+    return m_animation_frame_callback_driver.add([this, js_callback = move(js_callback)](auto) mutable {
         // 3. Invoke callback, passing now as the only argument,
         auto result = Bindings::IDL::invoke_callback(*js_callback, {}, JS::Value(performance().now()));
 
         // and if an exception is thrown, report the exception.
         if (result.is_error())
             HTML::report_exception(result);
-        m_request_animation_frame_callbacks.remove(id);
     });
-    m_request_animation_frame_callbacks.set(callback->id(), callback);
-    return callback->id();
 }
 
 void Window::cancel_animation_frame(i32 id)
 {
-    auto it = m_request_animation_frame_callbacks.find(id);
+    m_animation_frame_callback_driver.remove(id);
-    if (it == m_request_animation_frame_callbacks.end())
-        return;
-    it->value->cancel();
-    m_request_animation_frame_callbacks.remove(it);
 }
 
 void Window::did_set_location_href(Badge<Bindings::LocationObject>, AK::URL const& new_href)

Userland/Libraries/LibWeb/HTML/Window.h

@@ -18,12 +18,12 @@
 #include <LibWeb/DOM/Document.h>
 #include <LibWeb/DOM/Event.h>
 #include <LibWeb/DOM/EventTarget.h>
+#include <LibWeb/HTML/AnimationFrameCallbackDriver.h>
 #include <LibWeb/HTML/BrowsingContext.h>
 #include <LibWeb/HTML/GlobalEventHandlers.h>
 
 namespace Web::HTML {
 
-class RequestAnimationFrameCallback;
 class IdleCallback;
 
 class Window final
@@ -59,7 +59,7 @@ public:
     String prompt(String const&, String const&);
     i32 request_animation_frame(NonnullOwnPtr<Bindings::CallbackType> js_callback);
     void cancel_animation_frame(i32);
-    bool has_animation_frame_callbacks() const { return !m_request_animation_frame_callbacks.is_empty(); }
+    bool has_animation_frame_callbacks() const { return m_animation_frame_callback_driver.has_callbacks(); }
 
     i32 set_timeout(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector<JS::Value> arguments);
     i32 set_interval(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector<JS::Value> arguments);
@@ -122,6 +122,8 @@ public:
     u32 request_idle_callback(NonnullOwnPtr<Bindings::CallbackType> callback);
     void cancel_idle_callback(u32);
 
+    AnimationFrameCallbackDriver& animation_frame_callback_driver() { return m_animation_frame_callback_driver; }
+
 private:
     explicit Window(DOM::Document&);
 
@@ -149,7 +151,7 @@ private:
     NonnullOwnPtr<CSS::Screen> m_screen;
     RefPtr<DOM::Event> m_current_event;
 
-    HashMap<i32, NonnullRefPtr<RequestAnimationFrameCallback>> m_request_animation_frame_callbacks;
+    AnimationFrameCallbackDriver m_animation_frame_callback_driver;
 
     // https://w3c.github.io/requestidlecallback/#dfn-list-of-idle-request-callbacks
     NonnullRefPtrVector<IdleCallback> m_idle_request_callbacks;