LibWeb: Prevent running permanently unrunnable tasks in EventLoop

In `::spin_processing_tasks_with_source_until()`, we would first take a
set of tasks based on a filter, and then run them one by one. If there
was more than one task matched and put in that vector, they could
interfere with each other's runnability by making later tasks
permanently unrunnable.

The `::take_tasks_matching()` API is a footgun - remove it in favor of
an API that takes tasks one by one, performing the runnability check
just in time.

Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp

@@ -131,16 +131,15 @@ void EventLoop::spin_processing_tasks_with_source_until(Task::Source source, GC:
     Platform::EventLoopPlugin::the().spin_until(GC::create_function(heap(), [this, source, goal_condition] {
         if (goal_condition->function()())
             return true;
-        if (m_task_queue->has_runnable_tasks()) {
-            auto tasks = m_task_queue->take_tasks_matching([&](auto& task) {
-                return task.source() == source && task.is_runnable();
-            });
+        while (auto task = m_task_queue->take_first_runnable_matching([&](auto& candidate_task) {
+            return candidate_task.source() == source;
+        })) {
+            m_currently_running_task = task.ptr();
+            task->execute();
+            m_currently_running_task = nullptr;
 
-            for (auto& task : tasks) {
-                m_currently_running_task = task.ptr();
-                task->execute();
-                m_currently_running_task = nullptr;
-            }
+            if (goal_condition->function()())
+                break;
         }
 
         // FIXME: Remove the platform event loop plugin so that this doesn't look out of place

Libraries/LibWeb/HTML/EventLoop/TaskQueue.cpp

@@ -81,27 +81,23 @@ void TaskQueue::remove_tasks_matching(Function<bool(HTML::Task const&)> filter)
     m_tasks.remove_all_matching(filter);
 }
 
-GC::RootVector<GC::Ref<Task>> TaskQueue::take_tasks_matching(Function<bool(HTML::Task const&)> filter)
+GC::Ptr<Task> TaskQueue::take_first_runnable_matching(Function<bool(HTML::Task const&)> filter)
 {
-    GC::RootVector<GC::Ref<Task>> matching_tasks(heap());
-
     for (size_t i = 0; i < m_tasks.size();) {
         auto& task = m_tasks.at(i);
 
+        if (task->is_runnable() && filter(*task))
+            return m_tasks.take(i);
+
         if (task->is_permanently_unrunnable()) {
             m_tasks.remove(i);
             continue;
         }
 
-        if (filter(*task)) {
-            matching_tasks.append(task);
-            m_tasks.remove(i);
-        } else {
-            ++i;
-        }
+        ++i;
     }
 
-    return matching_tasks;
+    return nullptr;
 }
 
 Task const* TaskQueue::last_added_task() const

Libraries/LibWeb/HTML/EventLoop/TaskQueue.h

@@ -6,7 +6,7 @@
 
 #pragma once
 
-#include <AK/Queue.h>
+#include <AK/Vector.h>
 #include <LibJS/Heap/Cell.h>
 #include <LibWeb/HTML/EventLoop/Task.h>
 
@@ -37,7 +37,7 @@ public:
     }
 
     void remove_tasks_matching(Function<bool(HTML::Task const&)>);
-    GC::RootVector<GC::Ref<Task>> take_tasks_matching(Function<bool(HTML::Task const&)>);
+    GC::Ptr<Task> take_first_runnable_matching(Function<bool(HTML::Task const&)>);
 
     Task const* last_added_task() const;
 

Libraries/LibWeb/HTML/HTMLMediaElement.cpp

@@ -617,17 +617,15 @@ WebIDL::ExceptionOr<void> HTMLMediaElement::load_element()
     //          any ongoing fetch operation. Therefore, all resource selection algorithms will be cancelled before a new
     //          one begins.
 
-    // 2. Let pending tasks be a list of all tasks from the media element's media element event task source in one of the task queues.
-    [[maybe_unused]] auto pending_tasks = HTML::main_thread_event_loop().task_queue().take_tasks_matching([&](auto& task) {
+    // 2. Let pending tasks be a list of all tasks from the media element's media element event task source in one of
+    //    the task queues.
+    // FIXME: 3. For each task in pending tasks that would resolve pending play promises or reject pending play promises,
+    //    immediately resolve or reject those promises in the order the corresponding tasks were queued.
+    // 4. Remove each task in pending tasks from its task queue
+    main_thread_event_loop().task_queue().remove_tasks_matching([&](auto& task) {
         return task.source() == media_element_event_task_source();
     });
 
-    // FIXME: 3. For each task in pending tasks that would resolve pending play promises or reject pending play promises, immediately resolve or
-    //           reject those promises in the order the corresponding tasks were queued.
-
-    // 4. Remove each task in pending tasks from its task queue
-    //    NOTE: We performed this step along with step 2.
-
     // 5. If the media element's networkState is set to NETWORK_LOADING or NETWORK_IDLE, queue a media element task given the media element to
     //    fire an event named abort at the media element.
     if (m_network_state == NetworkState::Loading || m_network_state == NetworkState::Idle) {