mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2024-11-07 20:31:04 +03:00
LibWeb: Track fetching-related tasks in FetchController for cancellation
The HTMLMediaElement, for example, contains spec text which states any ongoing fetch process must be "stopped". The spec does not indicate how to do this, so our implementation is rather ad-hoc. Our current implementation may cause a crash in places that assume one of the fetch algorithms that we set to null is *not* null. For example: if (fetch_params.process_response) { queue_fetch_task([]() { fetch_params.process_response(); }; } If the fetch process is stopped after queuing the fetch task, but not before the fetch task is run, we will crash when running this fetch algorithm. We now track queued fetch tasks on the fetch controller. When the fetch process is stopped, we cancel any such pending task. It is a little bit awkward maintaining a fetch task ID. Ideally, we could use the underlying task ID throughout. But we do not have access to the underlying task nor its ID when the task is running, at which point we need some ID to remove from the pending task list.
This commit is contained in:
parent
4806cf9527
commit
7b3ddd5e15
Notes:
sideshowbarker
2024-07-17 07:43:44 +09:00
Author: https://github.com/trflynn89 Commit: https://github.com/SerenityOS/serenity/commit/7b3ddd5e15 Pull-request: https://github.com/SerenityOS/serenity/pull/23681
1
Tests/LibWeb/Text/expected/video-canceled-load.txt
Normal file
1
Tests/LibWeb/Text/expected/video-canceled-load.txt
Normal file
@ -0,0 +1 @@
|
||||
wfh
|
13
Tests/LibWeb/Text/input/video-canceled-load.html
Normal file
13
Tests/LibWeb/Text/input/video-canceled-load.html
Normal file
@ -0,0 +1,13 @@
|
||||
<script src="include.js"></script>
|
||||
<script type="text/javascript">
|
||||
function updateSrc() {
|
||||
video.src = "wfh";
|
||||
}
|
||||
|
||||
test(() => {
|
||||
println(video.src);
|
||||
});
|
||||
</script>
|
||||
<video id="video" src="data:">
|
||||
<details open ontoggle="updateSrc()"></details>
|
||||
</video>
|
@ -627,7 +627,7 @@ WebIDL::ExceptionOr<void> fetch_response_handover(JS::Realm& realm, Infrastructu
|
||||
auto task_destination = fetch_params.task_destination().get<JS::NonnullGCPtr<JS::Object>>();
|
||||
|
||||
// 5. Queue a fetch task to run processResponseEndOfBodyTask with fetchParams’s task destination.
|
||||
Infrastructure::queue_fetch_task(task_destination, move(process_response_end_of_body_task));
|
||||
Infrastructure::queue_fetch_task(fetch_params.controller(), task_destination, move(process_response_end_of_body_task));
|
||||
};
|
||||
|
||||
// FIXME: Handle 'parallel queue' task destination
|
||||
@ -636,7 +636,7 @@ WebIDL::ExceptionOr<void> fetch_response_handover(JS::Realm& realm, Infrastructu
|
||||
// 4. If fetchParams’s process response is non-null, then queue a fetch task to run fetchParams’s process response
|
||||
// given response, with fetchParams’s task destination.
|
||||
if (fetch_params.algorithms()->process_response()) {
|
||||
Infrastructure::queue_fetch_task(task_destination, [&fetch_params, &response]() {
|
||||
Infrastructure::queue_fetch_task(fetch_params.controller(), task_destination, [&fetch_params, &response]() {
|
||||
fetch_params.algorithms()->process_response()(response);
|
||||
});
|
||||
}
|
||||
@ -674,7 +674,7 @@ WebIDL::ExceptionOr<void> fetch_response_handover(JS::Realm& realm, Infrastructu
|
||||
// 3. If internalResponse's body is null, then queue a fetch task to run processBody given null, with
|
||||
// fetchParams’s task destination.
|
||||
if (!internal_response->body()) {
|
||||
Infrastructure::queue_fetch_task(task_destination, [process_body = move(process_body)]() {
|
||||
Infrastructure::queue_fetch_task(fetch_params.controller(), task_destination, [process_body = move(process_body)]() {
|
||||
process_body({});
|
||||
});
|
||||
}
|
||||
|
@ -9,6 +9,7 @@
|
||||
#include <LibWeb/Fetch/Infrastructure/FetchAlgorithms.h>
|
||||
#include <LibWeb/Fetch/Infrastructure/FetchController.h>
|
||||
#include <LibWeb/Fetch/Infrastructure/FetchParams.h>
|
||||
#include <LibWeb/HTML/EventLoop/EventLoop.h>
|
||||
#include <LibWeb/WebIDL/DOMException.h>
|
||||
|
||||
namespace Web::Fetch::Infrastructure {
|
||||
@ -105,11 +106,29 @@ void FetchController::stop_fetch()
|
||||
// AD-HOC: Some HTML elements need to stop an ongoing fetching process without causing any network error to be raised
|
||||
// (which abort() and terminate() will both do). This is tricky because the fetch process runs across several
|
||||
// nested Platform::EventLoopPlugin::deferred_invoke() invocations. For now, we "stop" the fetch process by
|
||||
// ignoring any callbacks.
|
||||
// cancelling any queued fetch tasks and then ignoring any callbacks.
|
||||
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 task.id() == task_id;
|
||||
});
|
||||
});
|
||||
|
||||
if (m_fetch_params) {
|
||||
auto fetch_algorithms = FetchAlgorithms::create(vm, {});
|
||||
m_fetch_params->set_algorithms(fetch_algorithms);
|
||||
}
|
||||
}
|
||||
|
||||
void FetchController::fetch_task_queued(u64 fetch_task_id, int event_id)
|
||||
{
|
||||
m_ongoing_fetch_tasks.set(fetch_task_id, event_id);
|
||||
}
|
||||
|
||||
void FetchController::fetch_task_complete(u64 fetch_task_id)
|
||||
{
|
||||
m_ongoing_fetch_tasks.remove(fetch_task_id);
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -7,6 +7,7 @@
|
||||
#pragma once
|
||||
|
||||
#include <AK/Badge.h>
|
||||
#include <AK/HashMap.h>
|
||||
#include <LibJS/Forward.h>
|
||||
#include <LibJS/Heap/Cell.h>
|
||||
#include <LibJS/Heap/GCPtr.h>
|
||||
@ -48,6 +49,10 @@ 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_complete(u64 fetch_task_id);
|
||||
|
||||
private:
|
||||
FetchController();
|
||||
|
||||
@ -78,6 +83,9 @@ private:
|
||||
JS::GCPtr<JS::HeapFunction<void()>> m_next_manual_redirect_steps;
|
||||
|
||||
JS::GCPtr<FetchParams> m_fetch_params;
|
||||
|
||||
HashMap<u64, int> m_ongoing_fetch_tasks;
|
||||
u64 m_next_fetch_task_id { 0 };
|
||||
};
|
||||
|
||||
}
|
||||
|
@ -4,18 +4,34 @@
|
||||
* SPDX-License-Identifier: BSD-2-Clause
|
||||
*/
|
||||
|
||||
#include <LibWeb/Fetch/Infrastructure/FetchController.h>
|
||||
#include <LibWeb/Fetch/Infrastructure/Task.h>
|
||||
#include <LibWeb/HTML/EventLoop/EventLoop.h>
|
||||
|
||||
namespace Web::Fetch::Infrastructure {
|
||||
|
||||
// https://fetch.spec.whatwg.org/#queue-a-fetch-task
|
||||
void queue_fetch_task(JS::Object& task_destination, JS::SafeFunction<void()> algorithm)
|
||||
int queue_fetch_task(JS::Object& task_destination, JS::SafeFunction<void()> algorithm)
|
||||
{
|
||||
// FIXME: 1. If taskDestination is a parallel queue, then enqueue algorithm to taskDestination.
|
||||
|
||||
// 2. Otherwise, queue a global task on the networking task source with taskDestination and algorithm.
|
||||
HTML::queue_global_task(HTML::Task::Source::Networking, task_destination, move(algorithm));
|
||||
return HTML::queue_global_task(HTML::Task::Source::Networking, task_destination, move(algorithm));
|
||||
}
|
||||
|
||||
// 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::SafeFunction<void()> algorithm)
|
||||
{
|
||||
auto fetch_task_id = fetch_controller->next_fetch_task_id();
|
||||
|
||||
int event_id = queue_fetch_task(task_destination, [fetch_controller, fetch_task_id, algorithm = move(algorithm)]() {
|
||||
fetch_controller->fetch_task_complete(fetch_task_id);
|
||||
algorithm();
|
||||
});
|
||||
|
||||
fetch_controller->fetch_task_queued(fetch_task_id, event_id);
|
||||
return event_id;
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -10,12 +10,14 @@
|
||||
#include <LibJS/Forward.h>
|
||||
#include <LibJS/Heap/GCPtr.h>
|
||||
#include <LibJS/SafeFunction.h>
|
||||
#include <LibWeb/Forward.h>
|
||||
|
||||
namespace Web::Fetch::Infrastructure {
|
||||
|
||||
// FIXME: 'or a parallel queue'
|
||||
using TaskDestination = Variant<Empty, JS::NonnullGCPtr<JS::Object>>;
|
||||
|
||||
void queue_fetch_task(JS::Object&, JS::SafeFunction<void()>);
|
||||
int queue_fetch_task(JS::Object&, JS::SafeFunction<void()>);
|
||||
int queue_fetch_task(JS::NonnullGCPtr<FetchController>, JS::Object&, JS::SafeFunction<void()>);
|
||||
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user