From 8d2194bdbdcce8ee743344b3269b38747bf70516 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 27 Jun 2020 18:30:29 +0200 Subject: [PATCH] LibWeb: Make DOM timers cancellable and stop leaking them This patch adds a Web::Timer object that represents a single timer registration made with window.setTimeout() or window.setInterval(). All live timers are owned by the DOM Window object. The timers can be stopped via clearTimeout() or clearInterval(). Note that those API's are actually interchangeable, but we have to support both. --- Libraries/LibWeb/Bindings/WindowObject.cpp | 36 +++++++++++-- Libraries/LibWeb/Bindings/WindowObject.h | 2 + Libraries/LibWeb/CMakeLists.txt | 1 + Libraries/LibWeb/DOM/Timer.cpp | 60 +++++++++++++++++++++ Libraries/LibWeb/DOM/Timer.h | 63 ++++++++++++++++++++++ Libraries/LibWeb/DOM/Window.cpp | 56 +++++++++++++------ Libraries/LibWeb/DOM/Window.h | 14 ++++- Libraries/LibWeb/Forward.h | 1 + 8 files changed, 212 insertions(+), 21 deletions(-) create mode 100644 Libraries/LibWeb/DOM/Timer.cpp create mode 100644 Libraries/LibWeb/DOM/Timer.h diff --git a/Libraries/LibWeb/Bindings/WindowObject.cpp b/Libraries/LibWeb/Bindings/WindowObject.cpp index 2382dd16de..98ac43dc91 100644 --- a/Libraries/LibWeb/Bindings/WindowObject.cpp +++ b/Libraries/LibWeb/Bindings/WindowObject.cpp @@ -62,6 +62,8 @@ void WindowObject::initialize() define_native_function("confirm", confirm); define_native_function("setInterval", set_interval, 1); define_native_function("setTimeout", set_timeout, 1); + define_native_function("clearInterval", clear_interval, 1); + define_native_function("clearTimeout", clear_timeout, 1); define_native_function("requestAnimationFrame", request_animation_frame, 1); define_native_function("cancelAnimationFrame", cancel_animation_frame, 1); define_native_function("atob", atob, 1); @@ -152,8 +154,8 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_interval) interval = 0; } - impl->set_interval(*static_cast(callback_object), interval); - return JS::js_undefined(); + auto timer_id = impl->set_interval(*static_cast(callback_object), interval); + return JS::Value(timer_id); } JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_timeout) @@ -178,7 +180,35 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_timeout) interval = 0; } - impl->set_timeout(*static_cast(callback_object), interval); + auto timer_id = impl->set_timeout(*static_cast(callback_object), interval); + return JS::Value(timer_id); +} + +JS_DEFINE_NATIVE_FUNCTION(WindowObject::clear_timeout) +{ + auto* impl = impl_from(interpreter, global_object); + if (!impl) + return {}; + if (!interpreter.argument_count()) + return interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "clearTimeout"); + i32 timer_id = interpreter.argument(0).to_i32(interpreter); + if (interpreter.exception()) + return {}; + impl->clear_timeout(timer_id); + return JS::js_undefined(); +} + +JS_DEFINE_NATIVE_FUNCTION(WindowObject::clear_interval) +{ + auto* impl = impl_from(interpreter, global_object); + if (!impl) + return {}; + if (!interpreter.argument_count()) + return interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "clearInterval"); + i32 timer_id = interpreter.argument(0).to_i32(interpreter); + if (interpreter.exception()) + return {}; + impl->clear_timeout(timer_id); return JS::js_undefined(); } diff --git a/Libraries/LibWeb/Bindings/WindowObject.h b/Libraries/LibWeb/Bindings/WindowObject.h index 731c2083d9..57bfeb5186 100644 --- a/Libraries/LibWeb/Bindings/WindowObject.h +++ b/Libraries/LibWeb/Bindings/WindowObject.h @@ -58,6 +58,8 @@ private: JS_DECLARE_NATIVE_FUNCTION(confirm); JS_DECLARE_NATIVE_FUNCTION(set_interval); JS_DECLARE_NATIVE_FUNCTION(set_timeout); + JS_DECLARE_NATIVE_FUNCTION(clear_interval); + JS_DECLARE_NATIVE_FUNCTION(clear_timeout); JS_DECLARE_NATIVE_FUNCTION(request_animation_frame); JS_DECLARE_NATIVE_FUNCTION(cancel_animation_frame); JS_DECLARE_NATIVE_FUNCTION(atob); diff --git a/Libraries/LibWeb/CMakeLists.txt b/Libraries/LibWeb/CMakeLists.txt index 261eb226f0..f301791e3b 100644 --- a/Libraries/LibWeb/CMakeLists.txt +++ b/Libraries/LibWeb/CMakeLists.txt @@ -57,6 +57,7 @@ set(SOURCES DOM/ParentNode.cpp DOM/TagNames.cpp DOM/Text.cpp + DOM/Timer.cpp DOM/Window.cpp DOM/XMLHttpRequest.cpp DOMTreeModel.cpp diff --git a/Libraries/LibWeb/DOM/Timer.cpp b/Libraries/LibWeb/DOM/Timer.cpp new file mode 100644 index 0000000000..84846eaf36 --- /dev/null +++ b/Libraries/LibWeb/DOM/Timer.cpp @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2020, Andreas Kling + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include +#include +#include +#include + +namespace Web { + +NonnullRefPtr Timer::create_interval(Window& window, int milliseconds, JS::Function& callback) +{ + return adopt(*new Timer(window, Type::Interval, milliseconds, callback)); +} + +NonnullRefPtr Timer::create_timeout(Window& window, int milliseconds, JS::Function& callback) +{ + return adopt(*new Timer(window, Type::Timeout, milliseconds, callback)); +} + +Timer::Timer(Window& window, Type type, int milliseconds, JS::Function& callback) + : m_window(window) + , m_type(type) + , m_callback(JS::make_handle(&callback)) +{ + 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); +} + +Timer::~Timer() +{ +} + +} diff --git a/Libraries/LibWeb/DOM/Timer.h b/Libraries/LibWeb/DOM/Timer.h new file mode 100644 index 0000000000..562a3cf542 --- /dev/null +++ b/Libraries/LibWeb/DOM/Timer.h @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2020, Andreas Kling + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#pragma once + +#include +#include +#include +#include + +namespace Web { + +class Timer final : public RefCounted { +public: + enum class Type { + Interval, + Timeout, + }; + + static NonnullRefPtr create_interval(Window&, int milliseconds, JS::Function&); + static NonnullRefPtr create_timeout(Window&, int milliseconds, JS::Function&); + + ~Timer(); + + i32 id() const { return m_id; } + Type type() const { return m_type; } + + JS::Function& callback() { return *m_callback.cell(); } + +private: + Timer(Window&, Type, int ms, JS::Function&); + + Window& m_window; + RefPtr m_timer; + Type m_type; + int m_id { 0 }; + JS::Handle m_callback; +}; + +} diff --git a/Libraries/LibWeb/DOM/Window.cpp b/Libraries/LibWeb/DOM/Window.cpp index e0f7f38ab4..53e658b088 100644 --- a/Libraries/LibWeb/DOM/Window.cpp +++ b/Libraries/LibWeb/DOM/Window.cpp @@ -24,13 +24,13 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include #include #include #include #include #include #include +#include #include #include #include @@ -67,25 +67,49 @@ bool Window::confirm(const String& message) return confirm_result == GUI::Dialog::ExecResult::ExecOK; } -void Window::set_interval(JS::Function& callback, i32 interval) +i32 Window::set_interval(JS::Function& callback, i32 interval) { - // FIXME: This leaks the interval timer and makes it unstoppable. - (void)Core::Timer::construct(interval, [this, handle = make_handle(&callback)] { - auto& function = const_cast(static_cast(*handle.cell())); - auto& interpreter = function.interpreter(); - interpreter.call(function, wrapper()); - }).leak_ref(); + auto timer = Timer::create_interval(*this, interval, callback); + m_timers.set(timer->id(), timer); + return timer->id(); } -void Window::set_timeout(JS::Function& callback, i32 interval) +i32 Window::set_timeout(JS::Function& callback, i32 interval) { - // FIXME: This leaks the interval timer and makes it unstoppable. - auto& timer = Core::Timer::construct(interval, [this, handle = make_handle(&callback)] { - auto& function = const_cast(static_cast(*handle.cell())); - auto& interpreter = function.interpreter(); - interpreter.call(function, wrapper()); - }).leak_ref(); - timer.set_single_shot(true); + auto timer = Timer::create_timeout(*this, interval, callback); + m_timers.set(timer->id(), timer); + return timer->id(); +} + +void Window::timer_did_fire(Badge, Timer& timer) +{ + // We should not be here if there's no JS wrapper for the Window object. + ASSERT(wrapper()); + + // NOTE: This protector pointer keeps the timer alive until the end of this function no matter what. + NonnullRefPtr protector(timer); + + if (timer.type() == Timer::Type::Timeout) { + m_timers.remove(timer.id()); + } + + auto& interpreter = wrapper()->interpreter(); + interpreter.call(timer.callback(), wrapper()); +} + +i32 Window::allocate_timer_id(Badge) +{ + return m_timer_id_allocator.allocate(); +} + +void Window::clear_timeout(i32 timer_id) +{ + m_timers.remove(timer_id); +} + +void Window::clear_interval(i32 timer_id) +{ + m_timers.remove(timer_id); } i32 Window::request_animation_frame(JS::Function& callback) diff --git a/Libraries/LibWeb/DOM/Window.h b/Libraries/LibWeb/DOM/Window.h index 06b3b27f5d..4a551fb973 100644 --- a/Libraries/LibWeb/DOM/Window.h +++ b/Libraries/LibWeb/DOM/Window.h @@ -27,6 +27,7 @@ #pragma once #include +#include #include #include #include @@ -46,8 +47,11 @@ public: bool confirm(const String&); i32 request_animation_frame(JS::Function&); void cancel_animation_frame(i32); - void set_interval(JS::Function&, i32); - void set_timeout(JS::Function&, i32); + + i32 set_timeout(JS::Function&, i32); + i32 set_interval(JS::Function&, i32); + void clear_timeout(i32); + void clear_interval(i32); void did_set_location_href(Badge, const String& new_href); void did_call_location_reload(Badge); @@ -57,11 +61,17 @@ public: void set_wrapper(Badge, Bindings::WindowObject&); + i32 allocate_timer_id(Badge); + void timer_did_fire(Badge, Timer&); + private: explicit Window(Document&); Document& m_document; WeakPtr m_wrapper; + + IDAllocator m_timer_id_allocator; + HashMap> m_timers; }; } diff --git a/Libraries/LibWeb/Forward.h b/Libraries/LibWeb/Forward.h index 59b309e32a..141870b362 100644 --- a/Libraries/LibWeb/Forward.h +++ b/Libraries/LibWeb/Forward.h @@ -69,6 +69,7 @@ class StyleResolver; class StyleRule; class StyleSheet; class Text; +class Timer; class Window; class XMLHttpRequest;