From fbb9b28edc0924d0d1da5b2919c28312e04ca213 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sun, 5 Jan 2025 11:05:53 +1300 Subject: [PATCH] LibWeb/FileAPI: Implement aborting a FileReader read This fixes a timeout for the included WPT test. --- Libraries/LibWeb/FileAPI/FileReader.cpp | 45 +++++++++-- Libraries/LibWeb/FileAPI/FileReader.h | 9 ++- .../FileReader-multiple-reads.any.txt | 11 +++ .../FileReader-multiple-reads.any.html | 15 ++++ .../FileReader-multiple-reads.any.js | 81 +++++++++++++++++++ 5 files changed, 153 insertions(+), 8 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.html create mode 100644 Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.js diff --git a/Libraries/LibWeb/FileAPI/FileReader.cpp b/Libraries/LibWeb/FileAPI/FileReader.cpp index 58c56ce42b46..4cdae766427a 100644 --- a/Libraries/LibWeb/FileAPI/FileReader.cpp +++ b/Libraries/LibWeb/FileAPI/FileReader.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,26 @@ WebIDL::ExceptionOr FileReader::blob_package_data(JS::Realm& VERIFY_NOT_REACHED(); } +void FileReader::queue_a_task(GC::Ref> task) +{ + // To implement the requirement of removing queued tasks on an abort we keep track of a list of + // task IDs which are pending evaluation. This allows an abort to go through the task queue to + // remove those pending tasks. + + auto wrapper_task = GC::create_function(heap(), [this, task] { + auto& event_loop = *HTML::relevant_agent(*this).event_loop; + VERIFY(event_loop.currently_running_task()); + auto& current_task = *event_loop.currently_running_task(); + + task->function()(); + + m_pending_tasks.remove(current_task.id()); + }); + + auto id = HTML::queue_global_task(HTML::Task::Source::FileReading, realm().global_object(), wrapper_task); + m_pending_tasks.set(id); +} + // https://w3c.github.io/FileAPI/#readOperation WebIDL::ExceptionOr FileReader::read_operation(Blob& blob, Type type, Optional const& encoding_name) { @@ -123,6 +144,7 @@ WebIDL::ExceptionOr FileReader::read_operation(Blob& blob, Type type, Opti // 2. Set fr’s state to "loading". m_state = State::Loading; + m_is_aborted = false; // 3. Set fr’s result to null. m_result = {}; @@ -150,7 +172,7 @@ WebIDL::ExceptionOr FileReader::read_operation(Blob& blob, Type type, Opti HTML::TemporaryExecutionContext execution_context { realm, HTML::TemporaryExecutionContext::CallbacksEnabled::Yes }; Optional progress_timer; - while (true) { + while (!m_is_aborted) { auto& vm = realm.vm(); // FIXME: Try harder to not reach into the [[Promise]] slot of chunkPromise auto promise = GC::Ref { verify_cast(*chunk_promise->promise()) }; @@ -161,10 +183,13 @@ WebIDL::ExceptionOr FileReader::read_operation(Blob& blob, Type type, Opti return promise->state() == JS::Promise::State::Fulfilled || promise->state() == JS::Promise::State::Rejected; })); + if (m_is_aborted) + return; + // 2. If chunkPromise is fulfilled, and isFirstChunk is true, queue a task to fire a progress event called loadstart at fr. // NOTE: ISSUE 2 We might change loadstart to be dispatched synchronously, to align with XMLHttpRequest behavior. [Issue #119] if (promise->state() == JS::Promise::State::Fulfilled && is_first_chunk) { - HTML::queue_global_task(HTML::Task::Source::FileReading, realm.global_object(), GC::create_function(heap(), [this, &realm]() { + queue_a_task(GC::create_function(heap(), [this, &realm]() { dispatch_event(DOM::Event::create(realm, HTML::EventNames::loadstart)); })); } @@ -193,7 +218,7 @@ WebIDL::ExceptionOr FileReader::read_operation(Blob& blob, Type type, Opti // See http://wpt.live/FileAPI/reading-data-section/filereader_events.any.html bool contained_data = byte_sequence.array_length().length() > 0; if (enough_time_passed && contained_data) { - HTML::queue_global_task(HTML::Task::Source::FileReading, realm.global_object(), GC::create_function(heap(), [this, &realm]() { + queue_a_task(GC::create_function(heap(), [this, &realm]() { dispatch_event(DOM::Event::create(realm, HTML::EventNames::progress)); })); progress_timer = now; @@ -204,7 +229,7 @@ WebIDL::ExceptionOr FileReader::read_operation(Blob& blob, Type type, Opti } // 5. Otherwise, if chunkPromise is fulfilled with an object whose done property is true, queue a task to run the following steps and abort this algorithm: else if (promise->state() == JS::Promise::State::Fulfilled && done.as_bool()) { - HTML::queue_global_task(HTML::Task::Source::FileReading, realm.global_object(), GC::create_function(heap(), [this, bytes, type, &realm, encoding_name, blobs_type]() { + queue_a_task(GC::create_function(heap(), [this, bytes, type, &realm, encoding_name, blobs_type]() { // 1. Set fr’s state to "done". m_state = State::Done; @@ -238,7 +263,7 @@ WebIDL::ExceptionOr FileReader::read_operation(Blob& blob, Type type, Opti } // 6. Otherwise, if chunkPromise is rejected with an error error, queue a task to run the following steps and abort this algorithm: else if (promise->state() == JS::Promise::State::Rejected) { - HTML::queue_global_task(HTML::Task::Source::FileReading, realm.global_object(), GC::create_function(heap(), [this, &realm]() { + queue_a_task(GC::create_function(heap(), [this, &realm]() { // 1. Set fr’s state to "done". m_state = State::Done; @@ -308,9 +333,15 @@ void FileReader::abort() m_result = {}; } - // FIXME: 3. If there are any tasks from this on the file reading task source in an affiliated task queue, then remove those tasks from that task queue. + // 3. If there are any tasks from this on the file reading task source in an affiliated task queue, then remove those tasks from that task queue. + auto& event_loop = *HTML::relevant_agent(*this).event_loop; + event_loop.task_queue().remove_tasks_matching([&](auto const& task) { + return m_pending_tasks.contains(task.id()); + }); + m_pending_tasks.clear(); - // FIXME: 4. Terminate the algorithm for the read method being processed. + // 4. Terminate the algorithm for the read method being processed. + m_is_aborted = true; // 5. Fire a progress event called abort at this. dispatch_event(DOM::Event::create(realm, HTML::EventNames::abort)); diff --git a/Libraries/LibWeb/FileAPI/FileReader.h b/Libraries/LibWeb/FileAPI/FileReader.h index 8523e116199c..39e7b742c807 100644 --- a/Libraries/LibWeb/FileAPI/FileReader.h +++ b/Libraries/LibWeb/FileAPI/FileReader.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Shannon Booth + * Copyright (c) 2023-2025, Shannon Booth * * SPDX-License-Identifier: BSD-2-Clause */ @@ -10,6 +10,7 @@ #include #include #include +#include #include namespace Web::FileAPI { @@ -101,6 +102,12 @@ class FileReader : public DOM::EventTarget { static WebIDL::ExceptionOr blob_package_data(JS::Realm& realm, ByteBuffer, FileReader::Type type, Optional const&, Optional const& encoding_name); + void queue_a_task(GC::Ref>); + + // Internal state to handle aborting the FileReader. + HashTable m_pending_tasks; + bool m_is_aborted { false }; + // A FileReader has an associated state, that is "empty", "loading", or "done". It is initially "empty". // https://w3c.github.io/FileAPI/#filereader-state State m_state { State::Empty }; diff --git a/Tests/LibWeb/Text/expected/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.txt b/Tests/LibWeb/Text/expected/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.txt new file mode 100644 index 000000000000..262ee240073d --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.txt @@ -0,0 +1,11 @@ +Harness status: OK + +Found 6 tests + +6 Pass +Pass test FileReader InvalidStateError exception for readAsText +Pass test FileReader InvalidStateError exception for readAsDataURL +Pass test FileReader InvalidStateError exception for readAsArrayBuffer +Pass test FileReader InvalidStateError exception in onloadstart event for readAsArrayBuffer +Pass test FileReader no InvalidStateError exception in loadend event handler for readAsArrayBuffer +Pass test abort and restart in onloadstart event for readAsText \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.html b/Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.html new file mode 100644 index 000000000000..9a1fd54a4250 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.html @@ -0,0 +1,15 @@ + + +FileReader: starting new reads while one is in progress + + + + +
+ diff --git a/Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.js b/Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.js new file mode 100644 index 000000000000..4b19c69b4251 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/FileAPI/reading-data-section/FileReader-multiple-reads.any.js @@ -0,0 +1,81 @@ +// META: title=FileReader: starting new reads while one is in progress + +test(function() { + var blob_1 = new Blob(['TEST000000001']) + var blob_2 = new Blob(['TEST000000002']) + var reader = new FileReader(); + reader.readAsText(blob_1) + assert_equals(reader.readyState, FileReader.LOADING, "readyState Must be LOADING") + assert_throws_dom("InvalidStateError", function () { + reader.readAsText(blob_2) + }) +}, 'test FileReader InvalidStateError exception for readAsText'); + +test(function() { + var blob_1 = new Blob(['TEST000000001']) + var blob_2 = new Blob(['TEST000000002']) + var reader = new FileReader(); + reader.readAsDataURL(blob_1) + assert_equals(reader.readyState, FileReader.LOADING, "readyState Must be LOADING") + assert_throws_dom("InvalidStateError", function () { + reader.readAsDataURL(blob_2) + }) +}, 'test FileReader InvalidStateError exception for readAsDataURL'); + +test(function() { + var blob_1 = new Blob(['TEST000000001']) + var blob_2 = new Blob(['TEST000000002']) + var reader = new FileReader(); + reader.readAsArrayBuffer(blob_1) + assert_equals(reader.readyState, FileReader.LOADING, "readyState Must be LOADING") + assert_throws_dom("InvalidStateError", function () { + reader.readAsArrayBuffer(blob_2) + }) +}, 'test FileReader InvalidStateError exception for readAsArrayBuffer'); + +async_test(function() { + var blob_1 = new Blob(['TEST000000001']) + var blob_2 = new Blob(['TEST000000002']) + var reader = new FileReader(); + var triggered = false; + reader.onloadstart = this.step_func_done(function() { + assert_false(triggered, "Only one loadstart event should be dispatched"); + triggered = true; + assert_equals(reader.readyState, FileReader.LOADING, + "readyState must be LOADING") + assert_throws_dom("InvalidStateError", function () { + reader.readAsArrayBuffer(blob_2) + }) + }); + reader.readAsArrayBuffer(blob_1) + assert_equals(reader.readyState, FileReader.LOADING, "readyState Must be LOADING") +}, 'test FileReader InvalidStateError exception in onloadstart event for readAsArrayBuffer'); + +async_test(function() { + var blob_1 = new Blob(['TEST000000001']) + var blob_2 = new Blob(['TEST000000002']) + var reader = new FileReader(); + reader.onloadend = this.step_func_done(function() { + assert_equals(reader.readyState, FileReader.DONE, + "readyState must be DONE") + reader.readAsArrayBuffer(blob_2) + assert_equals(reader.readyState, FileReader.LOADING, "readyState Must be LOADING") + }); + reader.readAsArrayBuffer(blob_1) + assert_equals(reader.readyState, FileReader.LOADING, "readyState Must be LOADING") +}, 'test FileReader no InvalidStateError exception in loadend event handler for readAsArrayBuffer'); + +async_test(function() { + var blob_1 = new Blob([new Uint8Array(0x414141)]); + var blob_2 = new Blob(['TEST000000002']); + var reader = new FileReader(); + reader.onloadstart = this.step_func(function() { + reader.abort(); + reader.onloadstart = null; + reader.onloadend = this.step_func_done(function() { + assert_equals('TEST000000002', reader.result); + }); + reader.readAsText(blob_2); + }); + reader.readAsText(blob_1); +}, 'test abort and restart in onloadstart event for readAsText');