From 18b9d02edd427589eaa7fb7da1a7f6d6696cff4f Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 4 Mar 2022 10:41:12 -0500 Subject: [PATCH] LibWeb: Implement setTimeout/setInterval with ESO according to the spec Our setInterval implementation currently crashes on DuckDuckGo when it's invoked with a string argument. In this path, we were creating a native function to evaluate and execute that string. That evaluation was always returning a Completion, but NativeFunction expects ThrowCompletionOr. The conversion from Completion to ThrowCompletionOr would fail a VERIFY because that conversion is only valid if the Completion is an error; but we would trigger this conversion even on success. This change re-implements setTimeout & setInterval in direct accordance with the spec. So we avoid making that NativeFunction altogether, and DDG can progress past its invocation to the timer. With this change, we also have other features we did not previously support, such as passing any number of arguments to the timers. This does not implement handling of nesting levels yet. --- .../LibWeb/Bindings/WindowObject.cpp | 121 +++++++--------- .../Libraries/LibWeb/Bindings/WindowObject.h | 5 + Userland/Libraries/LibWeb/DOM/Timer.cpp | 28 ++-- Userland/Libraries/LibWeb/DOM/Timer.h | 25 +--- Userland/Libraries/LibWeb/DOM/Window.cpp | 131 +++++++++++++----- Userland/Libraries/LibWeb/DOM/Window.h | 12 +- 6 files changed, 184 insertions(+), 138 deletions(-) diff --git a/Userland/Libraries/LibWeb/Bindings/WindowObject.cpp b/Userland/Libraries/LibWeb/Bindings/WindowObject.cpp index 123d25ff6e..bb2fd96f10 100644 --- a/Userland/Libraries/LibWeb/Bindings/WindowObject.cpp +++ b/Userland/Libraries/LibWeb/Bindings/WindowObject.cpp @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -222,96 +221,80 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::prompt) return JS::js_string(vm, response); } -// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-setinterval -JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_interval) +static JS::ThrowCompletionOr make_timer_handler(JS::GlobalObject& global_object, JS::Value handler) { - // FIXME: Ideally this would share more code with setTimeout() using the "timer initialization steps" - // https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timer-initialisation-steps - auto* impl = TRY(impl_from(vm, global_object)); - if (!vm.argument_count()) - return vm.throw_completion(global_object, JS::ErrorType::BadArgCountAtLeastOne, "setInterval"); - JS::Object* callback_object; - if (vm.argument(0).is_function()) { - callback_object = &vm.argument(0).as_function(); - } else { - auto script_source = TRY(vm.argument(0).to_string(global_object)); - // FIXME: This needs more work once we have a environment settings object. - // The spec wants us to use a task for the "run function or script string" part, - // using a NativeFunction for the latter is a workaround so that we can reuse the - // DOM::Timer API unaltered (always expects a JS::FunctionObject). - callback_object = JS::NativeFunction::create(global_object, "", [impl, script_source = move(script_source)](auto&, auto&) mutable { - auto& settings_object = verify_cast(*impl->associated_document().realm().host_defined()); - auto script = HTML::ClassicScript::create(impl->associated_document().url().to_string(), script_source, settings_object, AK::URL()); - return script->run(); - }); - } - i32 interval = 0; - if (vm.argument_count() >= 2) { - interval = TRY(vm.argument(1).to_i32(global_object)); - if (interval < 0) - interval = 0; - } - - NonnullOwnPtr callback = adopt_own(*new Bindings::CallbackType(JS::make_handle(callback_object), HTML::incumbent_settings_object())); - // FIXME: Pass ...arguments to the callback function when it's invoked - auto timer_id = impl->set_interval(move(callback), interval); - return JS::Value(timer_id); + if (handler.is_function()) + return Bindings::CallbackType(JS::make_handle(handler.as_function()), HTML::incumbent_settings_object()); + return TRY(handler.to_string(global_object)); } // https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_timeout) { - // FIXME: Ideally this would share more code with setInterval() using the "timer initialization steps" - // https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timer-initialisation-steps auto* impl = TRY(impl_from(vm, global_object)); + if (!vm.argument_count()) return vm.throw_completion(global_object, JS::ErrorType::BadArgCountAtLeastOne, "setTimeout"); - JS::Object* callback_object; - if (vm.argument(0).is_function()) { - callback_object = &vm.argument(0).as_function(); - } else { - auto script_source = TRY(vm.argument(0).to_string(global_object)); - // FIXME: This needs more work once we have a environment settings object. - // The spec wants us to use a task for the "run function or script string" part, - // using a NativeFunction for the latter is a workaround so that we can reuse the - // DOM::Timer API unaltered (always expects a JS::FunctionObject). - callback_object = JS::NativeFunction::create(global_object, "", [impl, script_source = move(script_source)](auto&, auto&) mutable { - auto& settings_object = verify_cast(*impl->associated_document().realm().host_defined()); - auto script = HTML::ClassicScript::create(impl->associated_document().url().to_string(), script_source, settings_object, AK::URL()); - return script->run(); - }); - } - i32 interval = 0; - if (vm.argument_count() >= 2) { - interval = TRY(vm.argument(1).to_i32(global_object)); - if (interval < 0) - interval = 0; - } - NonnullOwnPtr callback = adopt_own(*new Bindings::CallbackType(JS::make_handle(callback_object), HTML::incumbent_settings_object())); + auto handler = TRY(make_timer_handler(global_object, vm.argument(0))); - // FIXME: Pass ...arguments to the callback function when it's invoked - auto timer_id = impl->set_timeout(move(callback), interval); - return JS::Value(timer_id); + i32 timeout = 0; + if (vm.argument_count() >= 2) + timeout = TRY(vm.argument(1).to_i32(global_object)); + + JS::MarkedVector arguments { vm.heap() }; + for (size_t i = 2; i < vm.argument_count(); ++i) + arguments.append(vm.argument(i)); + + auto id = impl->set_timeout(move(handler), timeout, move(arguments)); + return JS::Value(id); } +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-setinterval +JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_interval) +{ + auto* impl = TRY(impl_from(vm, global_object)); + + if (!vm.argument_count()) + return vm.throw_completion(global_object, JS::ErrorType::BadArgCountAtLeastOne, "setInterval"); + + auto handler = TRY(make_timer_handler(global_object, vm.argument(0))); + + i32 timeout = 0; + if (vm.argument_count() >= 2) + timeout = TRY(vm.argument(1).to_i32(global_object)); + + JS::MarkedVector arguments { vm.heap() }; + for (size_t i = 2; i < vm.argument_count(); ++i) + arguments.append(vm.argument(i)); + + auto id = impl->set_interval(move(handler), timeout, move(arguments)); + return JS::Value(id); +} + +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-cleartimeout JS_DEFINE_NATIVE_FUNCTION(WindowObject::clear_timeout) { auto* impl = TRY(impl_from(vm, global_object)); - if (!vm.argument_count()) - return vm.throw_completion(global_object, JS::ErrorType::BadArgCountAtLeastOne, "clearTimeout"); - i32 timer_id = TRY(vm.argument(0).to_i32(global_object)); - impl->clear_timeout(timer_id); + + i32 id = 0; + if (vm.argument_count()) + id = TRY(vm.argument(0).to_i32(global_object)); + + impl->clear_timeout(id); return JS::js_undefined(); } +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-clearinterval JS_DEFINE_NATIVE_FUNCTION(WindowObject::clear_interval) { auto* impl = TRY(impl_from(vm, global_object)); - if (!vm.argument_count()) - return vm.throw_completion(global_object, JS::ErrorType::BadArgCountAtLeastOne, "clearInterval"); - i32 timer_id = TRY(vm.argument(0).to_i32(global_object)); - impl->clear_interval(timer_id); + + i32 id = 0; + if (vm.argument_count()) + id = TRY(vm.argument(0).to_i32(global_object)); + + impl->clear_interval(id); return JS::js_undefined(); } diff --git a/Userland/Libraries/LibWeb/Bindings/WindowObject.h b/Userland/Libraries/LibWeb/Bindings/WindowObject.h index 008c5f993d..3488803e04 100644 --- a/Userland/Libraries/LibWeb/Bindings/WindowObject.h +++ b/Userland/Libraries/LibWeb/Bindings/WindowObject.h @@ -8,15 +8,20 @@ #pragma once #include +#include #include #include #include +#include #include #include namespace Web { namespace Bindings { +// https://html.spec.whatwg.org/#timerhandler +using TimerHandler = Variant; + class WindowObject : public JS::GlobalObject , public Weakable { diff --git a/Userland/Libraries/LibWeb/DOM/Timer.cpp b/Userland/Libraries/LibWeb/DOM/Timer.cpp index 5e5afbd50b..262051e854 100644 --- a/Userland/Libraries/LibWeb/DOM/Timer.cpp +++ b/Userland/Libraries/LibWeb/DOM/Timer.cpp @@ -5,31 +5,24 @@ */ #include -#include #include #include namespace Web::DOM { -NonnullRefPtr Timer::create_interval(Window& window, int milliseconds, NonnullOwnPtr callback) +NonnullRefPtr Timer::create(Window& window, i32 milliseconds, Function callback, i32 id) { - return adopt_ref(*new Timer(window, Type::Interval, milliseconds, move(callback))); + return adopt_ref(*new Timer(window, milliseconds, move(callback), id)); } -NonnullRefPtr Timer::create_timeout(Window& window, int milliseconds, NonnullOwnPtr callback) -{ - return adopt_ref(*new Timer(window, Type::Timeout, milliseconds, move(callback))); -} - -Timer::Timer(Window& window, Type type, int milliseconds, NonnullOwnPtr callback) +Timer::Timer(Window& window, i32 milliseconds, Function callback, i32 id) : m_window(window) - , m_type(type) - , m_callback(move(callback)) + , m_id(id) { - m_id = window.allocate_timer_id({}); - m_timer = Core::Timer::construct(milliseconds, [this] { m_window.timer_did_fire({}, *this); }); - if (m_type == Type::Timeout) - m_timer->set_single_shot(true); + m_timer = Core::Timer::create_single_shot(milliseconds, [this, callback = move(callback)] { + NonnullRefPtr strong_timer { *this }; + callback(); + }); } Timer::~Timer() @@ -37,4 +30,9 @@ Timer::~Timer() m_window.deallocate_timer_id({}, m_id); } +void Timer::start() +{ + m_timer->start(); +} + } diff --git a/Userland/Libraries/LibWeb/DOM/Timer.h b/Userland/Libraries/LibWeb/DOM/Timer.h index cd253ead02..de40b93675 100644 --- a/Userland/Libraries/LibWeb/DOM/Timer.h +++ b/Userland/Libraries/LibWeb/DOM/Timer.h @@ -7,38 +7,25 @@ #pragma once #include +#include #include -#include -#include #include namespace Web::DOM { class Timer final : public RefCounted { public: - enum class Type { - Interval, - Timeout, - }; - - static NonnullRefPtr create_interval(Window&, int milliseconds, NonnullOwnPtr callback); - static NonnullRefPtr create_timeout(Window&, int milliseconds, NonnullOwnPtr callback); - + static NonnullRefPtr create(Window& window, i32 milliseconds, Function callback, i32 id); ~Timer(); - i32 id() const { return m_id; } - Type type() const { return m_type; } - - Bindings::CallbackType& callback() { return *m_callback; } + void start(); private: - Timer(Window&, Type, int ms, NonnullOwnPtr callback); + Timer(Window& window, i32 milliseconds, Function callback, i32 id); - Window& m_window; RefPtr m_timer; - Type m_type; - int m_id { 0 }; - NonnullOwnPtr m_callback; + Window& m_window; + i32 m_id { 0 }; }; } diff --git a/Userland/Libraries/LibWeb/DOM/Window.cpp b/Userland/Libraries/LibWeb/DOM/Window.cpp index 6312df3e3a..84e9d74425 100644 --- a/Userland/Libraries/LibWeb/DOM/Window.cpp +++ b/Userland/Libraries/LibWeb/DOM/Window.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -149,41 +150,28 @@ String Window::prompt(String const& message, String const& default_) return {}; } -i32 Window::set_interval(NonnullOwnPtr callback, i32 interval) +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout +i32 Window::set_timeout(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector arguments) { - auto timer = Timer::create_interval(*this, interval, move(callback)); - m_timers.set(timer->id(), timer); - return timer->id(); + return run_timer_initialization_steps(move(handler), timeout, move(arguments), Repeat::No); } -i32 Window::set_timeout(NonnullOwnPtr callback, i32 interval) +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-setinterval +i32 Window::set_interval(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector arguments) { - auto timer = Timer::create_timeout(*this, interval, move(callback)); - m_timers.set(timer->id(), timer); - return timer->id(); + return run_timer_initialization_steps(move(handler), timeout, move(arguments), Repeat::Yes); } -void Window::timer_did_fire(Badge, Timer& timer) +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-cleartimeout +void Window::clear_timeout(i32 id) { - NonnullRefPtr strong_timer { timer }; - - if (timer.type() == Timer::Type::Timeout) { - m_timers.remove(timer.id()); - } - - // We should not be here if there's no JS wrapper for the Window object. - VERIFY(wrapper()); - - HTML::queue_global_task(HTML::Task::Source::TimerTask, *wrapper(), [this, strong_this = NonnullRefPtr(*this), strong_timer = NonnullRefPtr(timer)]() mutable { - auto result = Bindings::IDL::invoke_callback(strong_timer->callback(), wrapper()); - if (result.is_error()) - HTML::report_exception(result); - }); + m_timers.remove(id); } -i32 Window::allocate_timer_id(Badge) +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-clearinterval +void Window::clear_interval(i32 id) { - return m_timer_id_allocator.allocate(); + m_timers.remove(id); } void Window::deallocate_timer_id(Badge, i32 id) @@ -191,14 +179,95 @@ void Window::deallocate_timer_id(Badge, i32 id) m_timer_id_allocator.deallocate(id); } -void Window::clear_timeout(i32 timer_id) +// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timer-initialisation-steps +i32 Window::run_timer_initialization_steps(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector arguments, Repeat repeat, Optional previous_id) { - m_timers.remove(timer_id); -} + // 1. Let thisArg be global if that is a WorkerGlobalScope object; otherwise let thisArg be the WindowProxy that corresponds to global. -void Window::clear_interval(i32 timer_id) -{ - m_timers.remove(timer_id); + // 2. If previousId was given, let id be previousId; otherwise, let id be an implementation-defined integer that is greater than zero and does not already exist in global's map of active timers. + auto id = previous_id.has_value() ? previous_id.value() : m_timer_id_allocator.allocate(); + + // 3. FIXME: If the surrounding agent's event loop's currently running task is a task that was created by this algorithm, then let nesting level be the task's timer nesting level. Otherwise, let nesting level be zero. + + // 4. If timeout is less than 0, then set timeout to 0. + if (timeout < 0) + timeout = 0; + + // 5. FIXME: If nesting level is greater than 5, and timeout is less than 4, then set timeout to 4. + + // 6. Let callerRealm be the current Realm Record, and calleeRealm be global's relevant Realm. + // FIXME: Implement this when step 9.2 is implemented. + + // 7. Let initiating script be the active script. + // 8. Assert: initiating script is not null, since this algorithm is always called from some script. + + // 9. Let task be a task that runs the following substeps: + auto task = [window = NonnullRefPtr(*this), handler = move(handler), timeout, arguments = move(arguments), repeat, id]() mutable { + // 1. If id does not exist in global's map of active timers, then abort these steps. + if (!window->m_timers.contains(id)) + return; + + handler.visit( + // 2. If handler is a Function, then invoke handler given arguments with the callback this value set to thisArg. If this throws an exception, catch it, and report the exception. + [&](Bindings::CallbackType& callback) { + if (auto result = Bindings::IDL::invoke_callback(callback, window->wrapper(), arguments); result.is_error()) + HTML::report_exception(result); + }, + // 3. Otherwise: + [&](String const& source) { + // 1. Assert: handler is a string. + // 2. FIXME: Perform HostEnsureCanCompileStrings(callerRealm, calleeRealm). If this throws an exception, catch it, report the exception, and abort these steps. + + // 3. Let settings object be global's relevant settings object. + auto& settings_object = window->associated_document().relevant_settings_object(); + + // 4. Let base URL be initiating script's base URL. + auto url = window->associated_document().url(); + + // 5. Assert: base URL is not null, as initiating script is a classic script or a JavaScript module script. + + // 6. Let fetch options be a script fetch options whose cryptographic nonce is initiating script's fetch options's cryptographic nonce, integrity metadata is the empty string, parser metadata is "not-parser-inserted", credentials mode is initiating script's fetch options's credentials mode, and referrer policy is initiating script's fetch options's referrer policy. + // 7. Let script be the result of creating a classic script given handler, settings object, base URL, and fetch options. + auto script = HTML::ClassicScript::create(url.basename(), source, settings_object, url); + + // 8. Run the classic script script. + (void)script->run(); + }); + + // 4. If id does not exist in global's map of active timers, then abort these steps. + if (!window->m_timers.contains(id)) + return; + + switch (repeat) { + // 5. If repeat is true, then perform the timer initialization steps again, given global, handler, timeout, arguments, true, and id. + case Repeat::Yes: + window->run_timer_initialization_steps(handler, timeout, move(arguments), repeat, id); + break; + + // 6. Otherwise, remove global's map of active timers[id]. + case Repeat::No: + window->m_timers.remove(id); + break; + } + }; + + // 10. FIXME: Increment nesting level by one. + // 11. FIXME: Set task's timer nesting level to nesting level. + + // 12. Let completionStep be an algorithm step which queues a global task on the timer task source given global to run task. + auto completion_step = [window = NonnullRefPtr(*this), task = move(task)]() mutable { + HTML::queue_global_task(HTML::Task::Source::TimerTask, *window->wrapper(), [task = move(task)]() mutable { + task(); + }); + }; + + // 13. Run steps after a timeout given global, "setTimeout/setInterval", timeout, completionStep, and id. + auto timer = Timer::create(*this, timeout, move(completion_step), id); + m_timers.set(id, timer); + timer->start(); + + // 14. Return id. + return id; } // https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html#run-the-animation-frame-callbacks diff --git a/Userland/Libraries/LibWeb/DOM/Window.h b/Userland/Libraries/LibWeb/DOM/Window.h index d9e0bc9c33..f22f05ae29 100644 --- a/Userland/Libraries/LibWeb/DOM/Window.h +++ b/Userland/Libraries/LibWeb/DOM/Window.h @@ -50,8 +50,8 @@ public: i32 request_animation_frame(NonnullOwnPtr js_callback); void cancel_animation_frame(i32); - i32 set_timeout(NonnullOwnPtr callback, i32); - i32 set_interval(NonnullOwnPtr callback, i32); + i32 set_timeout(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector arguments); + i32 set_interval(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector arguments); void clear_timeout(i32); void clear_interval(i32); @@ -69,9 +69,7 @@ public: void set_wrapper(Badge, Bindings::WindowObject&); - i32 allocate_timer_id(Badge); void deallocate_timer_id(Badge, i32); - void timer_did_fire(Badge, Timer&); HighResolutionTime::Performance& performance() { return *m_performance; } @@ -110,6 +108,12 @@ private: // ^HTML::GlobalEventHandlers virtual DOM::EventTarget& global_event_handlers_to_event_target() override { return *this; } + enum class Repeat { + Yes, + No, + }; + i32 run_timer_initialization_steps(Bindings::TimerHandler handler, i32 timeout, JS::MarkedVector arguments, Repeat repeat, Optional previous_id = {}); + // https://html.spec.whatwg.org/multipage/window-object.html#concept-document-window WeakPtr m_associated_document;