LibWeb: Make HTML::Task IDs a sequential, distinct numeric type

This also fixes a bug where task IDs were being deallocated from the
wrong IDAllocator. I don't know if it was actually possible to cause any
real trouble with that mistake, nor do I know how to write a test for
it, but this makes the bug go away.

Userland/Libraries/LibWeb/Animations/Animation.h

@@ -192,7 +192,7 @@ private:
     // https://www.w3.org/TR/web-animations-1/#pending-pause-task
     TaskState m_pending_pause_task { TaskState::None };
 
-    Optional<int> m_pending_finish_microtask_id;
+    Optional<HTML::TaskID> m_pending_finish_microtask_id;
 
     Optional<double> m_saved_play_time;
     Optional<double> m_saved_pause_time;

Userland/Libraries/LibWeb/DOM/Element.cpp

@@ -894,7 +894,7 @@ void Element::make_html_uppercased_qualified_name()
 }
 
 // https://html.spec.whatwg.org/multipage/webappapis.html#queue-an-element-task
-int Element::queue_an_element_task(HTML::Task::Source source, Function<void()> steps)
+HTML::TaskID Element::queue_an_element_task(HTML::Task::Source source, Function<void()> steps)
 {
     return queue_a_task(source, HTML::main_thread_event_loop(), document(), JS::create_heap_function(heap(), move(steps)));
 }

Userland/Libraries/LibWeb/DOM/Element.h

@@ -227,7 +227,7 @@ public:
     [[nodiscard]] HashMap<FlyString, CSS::StyleProperty> const& custom_properties(Optional<CSS::Selector::PseudoElement::Type>) const;
 
     // NOTE: The function is wrapped in a JS::HeapFunction immediately.
-    int queue_an_element_task(HTML::Task::Source, Function<void()>);
+    HTML::TaskID queue_an_element_task(HTML::Task::Source, Function<void()>);
 
     bool is_void_element() const;
     bool serializes_as_void() const;

Userland/Libraries/LibWeb/Fetch/Infrastructure/FetchController.cpp

@@ -110,7 +110,7 @@ void FetchController::stop_fetch()
     auto ongoing_fetch_tasks = move(m_ongoing_fetch_tasks);
 
     HTML::main_thread_event_loop().task_queue().remove_tasks_matching([&](auto const& task) {
-        return ongoing_fetch_tasks.remove_all_matching([&](u64, int task_id) {
+        return ongoing_fetch_tasks.remove_all_matching([&](u64, HTML::TaskID task_id) {
             return task.id() == task_id;
         });
     });
@@ -121,7 +121,7 @@ void FetchController::stop_fetch()
     }
 }
 
-void FetchController::fetch_task_queued(u64 fetch_task_id, int event_id)
+void FetchController::fetch_task_queued(u64 fetch_task_id, HTML::TaskID event_id)
 {
     m_ongoing_fetch_tasks.set(fetch_task_id, event_id);
 }

Userland/Libraries/LibWeb/Fetch/Infrastructure/FetchController.h

@@ -16,6 +16,7 @@
 #include <LibJS/SafeFunction.h>
 #include <LibWeb/Fetch/Infrastructure/FetchTimingInfo.h>
 #include <LibWeb/Forward.h>
+#include <LibWeb/HTML/EventLoop/Task.h>
 
 namespace Web::Fetch::Infrastructure {
 
@@ -50,7 +51,7 @@ public:
     void stop_fetch();
 
     u64 next_fetch_task_id() { return m_next_fetch_task_id++; }
-    void fetch_task_queued(u64 fetch_task_id, int event_id);
+    void fetch_task_queued(u64 fetch_task_id, HTML::TaskID event_id);
     void fetch_task_complete(u64 fetch_task_id);
 
 private:
@@ -84,7 +85,7 @@ private:
 
     JS::GCPtr<FetchParams> m_fetch_params;
 
-    HashMap<u64, int> m_ongoing_fetch_tasks;
+    HashMap<u64, HTML::TaskID> m_ongoing_fetch_tasks;
     u64 m_next_fetch_task_id { 0 };
 };
 

Userland/Libraries/LibWeb/Fetch/Infrastructure/Task.cpp

@@ -11,7 +11,7 @@
 namespace Web::Fetch::Infrastructure {
 
 // https://fetch.spec.whatwg.org/#queue-a-fetch-task
-int queue_fetch_task(JS::Object& task_destination, JS::NonnullGCPtr<JS::HeapFunction<void()>> algorithm)
+HTML::TaskID queue_fetch_task(JS::Object& task_destination, JS::NonnullGCPtr<JS::HeapFunction<void()>> algorithm)
 {
     // FIXME: 1. If taskDestination is a parallel queue, then enqueue algorithm to taskDestination.
 
@@ -21,18 +21,18 @@ int queue_fetch_task(JS::Object& task_destination, JS::NonnullGCPtr<JS::HeapFunc
 
 // AD-HOC: This overload allows tracking the queued task within the fetch controller so that we may cancel queued tasks
 //         when the spec indicates that we must stop an ongoing fetch.
-int queue_fetch_task(JS::NonnullGCPtr<FetchController> fetch_controller, JS::Object& task_destination, JS::NonnullGCPtr<JS::HeapFunction<void()>> algorithm)
+HTML::TaskID queue_fetch_task(JS::NonnullGCPtr<FetchController> fetch_controller, JS::Object& task_destination, JS::NonnullGCPtr<JS::HeapFunction<void()>> algorithm)
 {
     auto fetch_task_id = fetch_controller->next_fetch_task_id();
 
     auto& heap = task_destination.heap();
-    int event_id = queue_fetch_task(task_destination, JS::create_heap_function(heap, [fetch_controller, fetch_task_id, algorithm]() {
+    auto html_task_id = queue_fetch_task(task_destination, JS::create_heap_function(heap, [fetch_controller, fetch_task_id, algorithm]() {
         fetch_controller->fetch_task_complete(fetch_task_id);
         algorithm->function()();
     }));
 
-    fetch_controller->fetch_task_queued(fetch_task_id, event_id);
+    fetch_controller->fetch_task_queued(fetch_task_id, html_task_id);
-    return event_id;
+    return html_task_id;
 }
 
 }

Userland/Libraries/LibWeb/Fetch/Infrastructure/Task.h

@@ -11,13 +11,14 @@
 #include <LibJS/Heap/GCPtr.h>
 #include <LibJS/SafeFunction.h>
 #include <LibWeb/Forward.h>
+#include <LibWeb/HTML/EventLoop/Task.h>
 
 namespace Web::Fetch::Infrastructure {
 
 // FIXME: 'or a parallel queue'
 using TaskDestination = Variant<Empty, JS::NonnullGCPtr<JS::Object>>;
 
-int queue_fetch_task(JS::Object&, JS::NonnullGCPtr<JS::HeapFunction<void()>>);
+HTML::TaskID queue_fetch_task(JS::Object&, JS::NonnullGCPtr<JS::HeapFunction<void()>>);
-int queue_fetch_task(JS::NonnullGCPtr<FetchController>, JS::Object&, JS::NonnullGCPtr<JS::HeapFunction<void()>>);
+HTML::TaskID queue_fetch_task(JS::NonnullGCPtr<FetchController>, JS::Object&, JS::NonnullGCPtr<JS::HeapFunction<void()>>);
 
 }

Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp

@@ -384,7 +384,7 @@ void EventLoop::process()
 }
 
 // https://html.spec.whatwg.org/multipage/webappapis.html#queue-a-task
-int queue_a_task(HTML::Task::Source source, JS::GCPtr<EventLoop> event_loop, JS::GCPtr<DOM::Document> document, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps)
+TaskID queue_a_task(HTML::Task::Source source, JS::GCPtr<EventLoop> event_loop, JS::GCPtr<DOM::Document> document, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps)
 {
     // 1. If event loop was not given, set event loop to the implied event loop.
     if (!event_loop)
@@ -409,7 +409,7 @@ int queue_a_task(HTML::Task::Source source, JS::GCPtr<EventLoop> event_loop, JS:
 }
 
 // https://html.spec.whatwg.org/multipage/webappapis.html#queue-a-global-task
-int queue_global_task(HTML::Task::Source source, JS::Object& global_object, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps)
+TaskID queue_global_task(HTML::Task::Source source, JS::Object& global_object, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps)
 {
     // 1. Let event loop be global's relevant agent's event loop.
     auto& global_custom_data = verify_cast<Bindings::WebEngineCustomData>(*global_object.vm().custom_data());

Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.h

@@ -116,8 +116,8 @@ private:
 };
 
 EventLoop& main_thread_event_loop();
-int queue_a_task(HTML::Task::Source, JS::GCPtr<EventLoop>, JS::GCPtr<DOM::Document>, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps);
+TaskID queue_a_task(HTML::Task::Source, JS::GCPtr<EventLoop>, JS::GCPtr<DOM::Document>, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps);
-int queue_global_task(HTML::Task::Source, JS::Object&, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps);
+TaskID queue_global_task(HTML::Task::Source, JS::Object&, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps);
 void queue_a_microtask(DOM::Document const*, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps);
 void perform_a_microtask_checkpoint();
 

Userland/Libraries/LibWeb/HTML/EventLoop/Task.cpp

@@ -13,7 +13,12 @@ namespace Web::HTML {
 JS_DEFINE_ALLOCATOR(Task);
 
 static IDAllocator s_unique_task_source_allocator { static_cast<int>(Task::Source::UniqueTaskSourceStart) };
-static IDAllocator s_task_id_allocator;
+
+[[nodiscard]] static TaskID allocate_task_id()
+{
+    static u64 next_task_id = 1;
+    return next_task_id++;
+}
 
 JS::NonnullGCPtr<Task> Task::create(JS::VM& vm, Source source, JS::GCPtr<DOM::Document const> document, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps)
 {
@@ -21,7 +26,7 @@ JS::NonnullGCPtr<Task> Task::create(JS::VM& vm, Source source, JS::GCPtr<DOM::Do
 }
 
 Task::Task(Source source, JS::GCPtr<DOM::Document const> document, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps)
-    : m_id(s_task_id_allocator.allocate())
+    : m_id(allocate_task_id())
     , m_source(source)
     , m_steps(steps)
     , m_document(document)
@@ -30,11 +35,6 @@ Task::Task(Source source, JS::GCPtr<DOM::Document const> document, JS::NonnullGC
 
 Task::~Task() = default;
 
-void Task::finalize()
-{
-    s_unique_task_source_allocator.deallocate(m_id);
-}
-
 void Task::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);

Userland/Libraries/LibWeb/HTML/EventLoop/Task.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/DistinctNumeric.h>
 #include <LibJS/Heap/Cell.h>
 #include <LibJS/Heap/CellAllocator.h>
 #include <LibJS/SafeFunction.h>
@@ -15,6 +16,8 @@ namespace Web::HTML {
 
 struct UniqueTaskSource;
 
+AK_TYPEDEF_DISTINCT_NUMERIC_GENERAL(u64, TaskID, Comparison);
+
 class Task final : public JS::Cell {
     JS_CELL(Task, JS::Cell);
     JS_DECLARE_ALLOCATOR(Task);
@@ -69,9 +72,8 @@ public:
     static JS::NonnullGCPtr<Task> create(JS::VM&, Source, JS::GCPtr<DOM::Document const>, JS::NonnullGCPtr<JS::HeapFunction<void()>> steps);
 
     virtual ~Task() override;
-    virtual void finalize() override;
 
-    int id() const { return m_id; }
+    [[nodiscard]] TaskID id() const { return m_id; }
     Source source() const { return m_source; }
     void execute();
 
@@ -84,7 +86,7 @@ private:
 
     virtual void visit_edges(Visitor&) override;
 
-    int m_id { 0 };
+    TaskID m_id {};
     Source m_source { Source::Unspecified };
     JS::NonnullGCPtr<JS::HeapFunction<void()>> m_steps;
     JS::GCPtr<DOM::Document const> m_document;

Userland/Libraries/LibWeb/HTML/ToggleTaskTracker.h

@@ -16,7 +16,7 @@ namespace Web::HTML {
 struct ToggleTaskTracker {
     // https://html.spec.whatwg.org/multipage/interaction.html#toggle-task-task
     // NOTE: We store the task's ID rather than the task itself to avoid ownership issues.
-    Optional<int> task_id;
+    Optional<HTML::TaskID> task_id;
 
     // https://html.spec.whatwg.org/multipage/interaction.html#toggle-task-old-state
     String old_state;