From 67c727177e43bdfab8492c50b1247ef00d7eb802 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Tue, 26 Sep 2023 11:44:56 +0200 Subject: [PATCH] LibWeb: Clear all active timers when document is destroyed This change implements a step from the document's destroy procedure in the specification, saying that all active timers should be cleared. By doing this, we also fix the leaking of a document in case where we have navigated away from a page that has scheduled timers that haven't yet been triggered. --- Userland/Libraries/LibWeb/DOM/Document.cpp | 24 ++++++++++++++++++- Userland/Libraries/LibWeb/DOM/Document.h | 2 ++ Userland/Libraries/LibWeb/HTML/Timer.cpp | 6 +++++ Userland/Libraries/LibWeb/HTML/Timer.h | 1 + .../LibWeb/HTML/WindowOrWorkerGlobalScope.cpp | 11 +++++++++ .../LibWeb/HTML/WindowOrWorkerGlobalScope.h | 1 + 6 files changed, 44 insertions(+), 1 deletion(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 128a991fcf..2d74867e52 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -2656,6 +2656,27 @@ Vector> Document::list_of_descendant_browsing_ return list; } +// https://html.spec.whatwg.org/multipage/document-lifecycle.html#unloading-document-cleanup-steps +void Document::run_unloading_cleanup_steps() +{ + // 1. Let window be document's relevant global object. + auto* window = dynamic_cast(&HTML::relevant_global_object(*this)); + VERIFY(window); + + // FIXME: 2. For each WebSocket object webSocket whose relevant global object is window, make disappear webSocket. + // If this affected any WebSocket objects, then set document's salvageable state to false. + + // FIXME: 3. For each WebTransport object transport whose relevant global object is window, run the context cleanup steps given transport. + + // 4. If document's salvageable state is false, then: + if (m_salvageable) { + // FIXME: 1. For each EventSource object eventSource whose relevant global object is equal to window, forcibly close eventSource. + + // 2. Clear window's map of active timers. + window->clear_map_of_active_timers(); + } +} + // https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document void Document::destroy() { @@ -2668,7 +2689,8 @@ void Document::destroy() // 2. Set document's salvageable state to false. m_salvageable = false; - // FIXME: 3. Run any unloading document cleanup steps for document that are defined by this specification and other applicable specifications. + // 3. Run any unloading document cleanup steps for document that are defined by this specification and other applicable specifications. + run_unloading_cleanup_steps(); // 5. Remove any tasks whose document is document from any task queue (without running those tasks). HTML::main_thread_event_loop().task_queue().remove_tasks_matching([this](auto& task) { diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h index 1fb3507960..703516575f 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.h +++ b/Userland/Libraries/LibWeb/DOM/Document.h @@ -545,6 +545,8 @@ private: void tear_down_layout_tree(); + void run_unloading_cleanup_steps(); + void evaluate_media_rules(); WebIDL::ExceptionOr run_the_document_write_steps(StringView); diff --git a/Userland/Libraries/LibWeb/HTML/Timer.cpp b/Userland/Libraries/LibWeb/HTML/Timer.cpp index 8357cf758a..fa0d509b0f 100644 --- a/Userland/Libraries/LibWeb/HTML/Timer.cpp +++ b/Userland/Libraries/LibWeb/HTML/Timer.cpp @@ -34,6 +34,7 @@ void Timer::visit_edges(Cell::Visitor& visitor) Timer::~Timer() { + VERIFY(!m_timer->is_active()); } void Timer::start() @@ -41,4 +42,9 @@ void Timer::start() m_timer->start(); } +void Timer::stop() +{ + m_timer->stop(); +} + } diff --git a/Userland/Libraries/LibWeb/HTML/Timer.h b/Userland/Libraries/LibWeb/HTML/Timer.h index fb931e7be5..bae09f1d92 100644 --- a/Userland/Libraries/LibWeb/HTML/Timer.h +++ b/Userland/Libraries/LibWeb/HTML/Timer.h @@ -24,6 +24,7 @@ public: virtual ~Timer() override; void start(); + void stop(); private: Timer(JS::Object& window, i32 milliseconds, Function callback, i32 id); diff --git a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp index 1f3b03e47e..ea4c48ac84 100644 --- a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp +++ b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp @@ -183,15 +183,26 @@ i32 WindowOrWorkerGlobalScopeMixin::set_interval(TimerHandler handler, i32 timeo // https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-cleartimeout void WindowOrWorkerGlobalScopeMixin::clear_timeout(i32 id) { + if (auto timer = m_timers.get(id); timer.has_value()) + timer.value()->stop(); m_timers.remove(id); } // https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-clearinterval void WindowOrWorkerGlobalScopeMixin::clear_interval(i32 id) { + if (auto timer = m_timers.get(id); timer.has_value()) + timer.value()->stop(); m_timers.remove(id); } +void WindowOrWorkerGlobalScopeMixin::clear_map_of_active_timers() +{ + for (auto& it : m_timers) + it.value->stop(); + m_timers.clear(); +} + // https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timer-initialisation-steps // With no active script fix from https://github.com/whatwg/html/pull/9712 i32 WindowOrWorkerGlobalScopeMixin::run_timer_initialization_steps(TimerHandler handler, i32 timeout, JS::MarkedVector arguments, Repeat repeat, Optional previous_id) diff --git a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h index ba0533238f..a21146cbb6 100644 --- a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h +++ b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h @@ -46,6 +46,7 @@ public: i32 set_interval(TimerHandler, i32 timeout, JS::MarkedVector arguments); void clear_timeout(i32); void clear_interval(i32); + void clear_map_of_active_timers(); PerformanceTimeline::PerformanceEntryTuple& relevant_performance_entry_tuple(FlyString const& entry_type); void queue_performance_entry(JS::NonnullGCPtr new_entry);