LibWeb: Make SessionHistoryTraversalQueue GC-allocated

- Add missing visit of a navigable in SHTQ entry
- Use HeapFunction instead of SafeFunction for entry callback

Userland/Libraries/LibWeb/CMakeLists.txt

@@ -407,6 +407,7 @@ set(SOURCES
     HTML/SelectedFile.cpp
     HTML/SelectItem.cpp
     HTML/SessionHistoryEntry.cpp
+    HTML/SessionHistoryTraversalQueue.cpp
     HTML/SharedImageRequest.cpp
     HTML/SourceSet.cpp
     HTML/Storage.cpp

Userland/Libraries/LibWeb/HTML/SessionHistoryTraversalQueue.cpp

@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibWeb/HTML/Navigable.h>
+#include <LibWeb/HTML/SessionHistoryTraversalQueue.h>
+
+namespace Web::HTML {
+
+JS_DEFINE_ALLOCATOR(SessionHistoryTraversalQueue);
+JS_DEFINE_ALLOCATOR(SessionHistoryTraversalQueueEntry);
+
+JS::NonnullGCPtr<SessionHistoryTraversalQueueEntry> SessionHistoryTraversalQueueEntry::create(JS::VM& vm, Function<void()> steps, JS::GCPtr<HTML::Navigable> target_navigable)
+{
+    return vm.heap().allocate_without_realm<SessionHistoryTraversalQueueEntry>(JS::create_heap_function(vm.heap(), move(steps)), target_navigable);
+}
+
+void SessionHistoryTraversalQueueEntry::visit_edges(JS::Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_steps);
+    visitor.visit(m_target_navigable);
+}
+
+SessionHistoryTraversalQueue::SessionHistoryTraversalQueue()
+{
+    m_timer = Core::Timer::create_single_shot(0, [this] {
+        if (m_is_task_running && m_queue.size() > 0) {
+            m_timer->start();
+            return;
+        }
+        while (m_queue.size() > 0) {
+            m_is_task_running = true;
+            auto entry = m_queue.take_first();
+            entry->execute_steps();
+            m_is_task_running = false;
+        }
+    }).release_value_but_fixme_should_propagate_errors();
+}
+
+void SessionHistoryTraversalQueue::visit_edges(JS::Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    for (auto const& entry : m_queue) {
+        visitor.visit(entry);
+    }
+}
+
+void SessionHistoryTraversalQueue::append(Function<void()> steps)
+{
+    m_queue.append(SessionHistoryTraversalQueueEntry::create(vm(), move(steps), nullptr));
+    if (!m_timer->is_active()) {
+        m_timer->start();
+    }
+}
+
+void SessionHistoryTraversalQueue::append_sync(Function<void()> steps, JS::GCPtr<Navigable> target_navigable)
+{
+    m_queue.append(SessionHistoryTraversalQueueEntry::create(vm(), move(steps), target_navigable));
+    if (!m_timer->is_active()) {
+        m_timer->start();
+    }
+}
+
+// https://html.spec.whatwg.org/multipage/browsing-the-web.html#sync-navigations-jump-queue
+JS::GCPtr<SessionHistoryTraversalQueueEntry> SessionHistoryTraversalQueue::first_synchronous_navigation_steps_with_target_navigable_not_contained_in(Vector<JS::GCPtr<Navigable>> const& list)
+{
+    auto index = m_queue.find_first_index_if([&list](auto const& entry) -> bool {
+        return (entry->target_navigable() != nullptr) && !list.contains_slow(entry->target_navigable());
+    });
+    if (index.has_value())
+        return m_queue.take(*index);
+    return {};
+}
+
+}

Userland/Libraries/LibWeb/HTML/SessionHistoryTraversalQueue.h

@@ -8,63 +8,57 @@
 
 #include <AK/Vector.h>
 #include <LibCore/Timer.h>
+#include <LibJS/Forward.h>
+#include <LibJS/Heap/Cell.h>
+#include <LibJS/Heap/CellAllocator.h>
 #include <LibJS/Heap/GCPtr.h>
+#include <LibJS/Heap/HeapFunction.h>
 #include <LibJS/SafeFunction.h>
 #include <LibWeb/Forward.h>
 
 namespace Web::HTML {
 
-struct SessionHistoryTraversalQueueEntry {
-    JS::SafeFunction<void()> steps;
-    JS::GCPtr<HTML::Navigable> target_navigable;
-};
+struct SessionHistoryTraversalQueueEntry : public JS::Cell {
+    JS_CELL(SessionHistoryTraversalQueueEntry, JS::Cell);
+    JS_DECLARE_ALLOCATOR(SessionHistoryTraversalQueueEntry);
 
-// https://html.spec.whatwg.org/multipage/document-sequences.html#tn-session-history-traversal-queue
-class SessionHistoryTraversalQueue {
 public:
-    SessionHistoryTraversalQueue()
-    {
-        m_timer = Core::Timer::create_single_shot(0, [this] {
-            if (m_is_task_running && m_queue.size() > 0) {
-                m_timer->start();
-                return;
-            }
-            while (m_queue.size() > 0) {
-                m_is_task_running = true;
-                auto entry = m_queue.take_first();
-                entry.steps();
-                m_is_task_running = false;
-            }
-        }).release_value_but_fixme_should_propagate_errors();
-    }
+    static JS::NonnullGCPtr<SessionHistoryTraversalQueueEntry> create(JS::VM& vm, Function<void()> steps, JS::GCPtr<HTML::Navigable> target_navigable);
 
-    void append(JS::SafeFunction<void()> steps)
-    {
-        m_queue.append({ move(steps), nullptr });
-        if (!m_timer->is_active()) {
-            m_timer->start();
-        }
-    }
+    JS::GCPtr<HTML::Navigable> target_navigable() const { return m_target_navigable; }
+    void execute_steps() const { m_steps->function()(); }
 
-    void append_sync(JS::SafeFunction<void()> steps, JS::GCPtr<Navigable> target_navigable)
+private:
+    SessionHistoryTraversalQueueEntry(JS::NonnullGCPtr<JS::HeapFunction<void()>> steps, JS::GCPtr<HTML::Navigable> target_navigable)
+        : m_steps(steps)
+        , m_target_navigable(target_navigable)
     {
-        m_queue.append({ move(steps), target_navigable });
-        if (!m_timer->is_active()) {
-            m_timer->start();
-        }
     }
 
+    virtual void visit_edges(Cell::Visitor&) override;
+
+    JS::NonnullGCPtr<JS::HeapFunction<void()>> m_steps;
+    JS::GCPtr<HTML::Navigable> m_target_navigable;
+};
+
+// https://html.spec.whatwg.org/multipage/document-sequences.html#tn-session-history-traversal-queue
+class SessionHistoryTraversalQueue : public JS::Cell {
+    JS_CELL(SessionHistoryTraversalQueue, Cell);
+    JS_DECLARE_ALLOCATOR(SessionHistoryTraversalQueue);
+
+public:
+    SessionHistoryTraversalQueue();
+
+    void append(Function<void()> steps);
+    void append_sync(Function<void()> steps, JS::GCPtr<Navigable> target_navigable);
+
     // https://html.spec.whatwg.org/multipage/browsing-the-web.html#sync-navigations-jump-queue
-    SessionHistoryTraversalQueueEntry first_synchronous_navigation_steps_with_target_navigable_not_contained_in(Vector<JS::GCPtr<Navigable>> const& list)
-    {
-        auto index = m_queue.find_first_index_if([&list](auto const& entry) -> bool {
-            return (entry.target_navigable != nullptr) && !list.contains_slow(entry.target_navigable);
-        });
-        return index.has_value() ? m_queue.take(*index) : SessionHistoryTraversalQueueEntry {};
-    }
+    JS::GCPtr<SessionHistoryTraversalQueueEntry> first_synchronous_navigation_steps_with_target_navigable_not_contained_in(Vector<JS::GCPtr<Navigable>> const& list);
 
 private:
-    Vector<SessionHistoryTraversalQueueEntry> m_queue;
+    virtual void visit_edges(Cell::Visitor&) override;
+
+    Vector<JS::NonnullGCPtr<SessionHistoryTraversalQueueEntry>> m_queue;
     RefPtr<Core::Timer> m_timer;
     bool m_is_task_running { false };
 };

Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp

@@ -23,7 +23,8 @@ namespace Web::HTML {
 JS_DEFINE_ALLOCATOR(TraversableNavigable);
 
 TraversableNavigable::TraversableNavigable(JS::NonnullGCPtr<Page> page)
-    : m_page(page)
+    : m_session_history_traversal_queue(vm().heap().allocate_without_realm<SessionHistoryTraversalQueue>())
+    , m_page(page)
 {
 }
 
@@ -35,6 +36,7 @@ void TraversableNavigable::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_page);
     for (auto& entry : m_session_history_entries)
         visitor.visit(entry);
+    visitor.visit(m_session_history_traversal_queue);
 }
 
 static OrderedHashTable<TraversableNavigable*>& user_agent_top_level_traversable_set()
@@ -600,15 +602,15 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
             //   1. Let steps be the first item in traversable's session history traversal queue's algorithm set
             //    that is synchronous navigation steps with a target navigable not contained in navigablesThatMustWaitBeforeHandlingSyncNavigation.
             //   2. Remove steps from traversable's session history traversal queue's algorithm set.
-            for (auto steps = m_session_history_traversal_queue.first_synchronous_navigation_steps_with_target_navigable_not_contained_in(navigables_that_must_wait_before_handling_sync_navigation);
-                 steps.target_navigable != nullptr;
-                 steps = m_session_history_traversal_queue.first_synchronous_navigation_steps_with_target_navigable_not_contained_in(navigables_that_must_wait_before_handling_sync_navigation)) {
+            for (auto entry = m_session_history_traversal_queue->first_synchronous_navigation_steps_with_target_navigable_not_contained_in(navigables_that_must_wait_before_handling_sync_navigation);
+                 entry;
+                 entry = m_session_history_traversal_queue->first_synchronous_navigation_steps_with_target_navigable_not_contained_in(navigables_that_must_wait_before_handling_sync_navigation)) {
 
                 // 3. Set traversable's running nested apply history step to true.
                 m_running_nested_apply_history_step = true;
 
                 // 4. Run steps.
-                steps.steps();
+                entry->execute_steps();
 
                 // 5. Set traversable's running nested apply history step to false.
                 m_running_nested_apply_history_step = false;

Userland/Libraries/LibWeb/HTML/TraversableNavigable.h

@@ -69,14 +69,14 @@ public:
     void close_top_level_traversable();
     void destroy_top_level_traversable();
 
-    void append_session_history_traversal_steps(JS::SafeFunction<void()> steps)
+    void append_session_history_traversal_steps(Function<void()> steps)
     {
-        m_session_history_traversal_queue.append(move(steps));
+        m_session_history_traversal_queue->append(move(steps));
     }
 
-    void append_session_history_synchronous_navigation_steps(JS::NonnullGCPtr<Navigable> target_navigable, JS::SafeFunction<void()> steps)
+    void append_session_history_synchronous_navigation_steps(JS::NonnullGCPtr<Navigable> target_navigable, Function<void()> steps)
     {
-        m_session_history_traversal_queue.append_sync(move(steps), target_navigable);
+        m_session_history_traversal_queue->append_sync(move(steps), target_navigable);
     }
 
     Page& page() { return m_page; }
@@ -116,7 +116,7 @@ private:
     // https://html.spec.whatwg.org/multipage/document-sequences.html#system-visibility-state
     VisibilityState m_system_visibility_state { VisibilityState::Visible };
 
-    SessionHistoryTraversalQueue m_session_history_traversal_queue;
+    JS::NonnullGCPtr<SessionHistoryTraversalQueue> m_session_history_traversal_queue;
 
     JS::NonnullGCPtr<Page> m_page;