From 3b4230e0b035dab2abdb353e1f34290c4bc912cf Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Mon, 11 Mar 2024 19:12:52 +0100 Subject: [PATCH] LibWeb: Visit custom element's lifecycle callbacks ...instead of using JS::Handle which causes leaks when object holding the callback can be reached by visiting the callback's dependencies. --- Userland/Libraries/LibWeb/CMakeLists.txt | 1 + Userland/Libraries/LibWeb/DOM/Element.cpp | 2 +- .../CustomElements/CustomElementDefinition.cpp | 18 ++++++++++++++++++ .../CustomElements/CustomElementDefinition.h | 4 +++- .../CustomElements/CustomElementRegistry.cpp | 6 +++--- 5 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.cpp diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index f03058f923..f827a0b866 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -250,6 +250,7 @@ set(SOURCES HTML/CORSSettingAttribute.cpp HTML/CrossOrigin/AbstractOperations.cpp HTML/CrossOrigin/Reporting.cpp + HTML/CustomElements/CustomElementDefinition.cpp HTML/CustomElements/CustomElementName.cpp HTML/CustomElements/CustomElementReactionNames.cpp HTML/CustomElements/CustomElementRegistry.cpp diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 6995481d18..26ac8b0f86 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -1910,7 +1910,7 @@ void Element::enqueue_a_custom_element_callback_reaction(FlyString const& callba if (callback_iterator == definition->lifecycle_callbacks().end()) return; - if (callback_iterator->value.is_null()) + if (!callback_iterator->value) return; // 4. If callbackName is "attributeChangedCallback", then: diff --git a/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.cpp b/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.cpp new file mode 100644 index 0000000000..c8576f5954 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.cpp @@ -0,0 +1,18 @@ +/* + * Copyright (c) 2024, Aliaksandr Kalenik + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include + +namespace Web::HTML { + +void CustomElementDefinition::visit_edges(Visitor& visitor) +{ + Base::visit_edges(visitor); + for (auto& callback : m_lifecycle_callbacks) + visitor.visit(callback.value); +} + +} diff --git a/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.h b/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.h index 83fcb46a77..a6a4f47688 100644 --- a/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.h +++ b/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.h @@ -20,7 +20,7 @@ class CustomElementDefinition : public JS::Cell { JS_CELL(CustomElementDefinition, JS::Cell); JS_DECLARE_ALLOCATOR(CustomElementDefinition); - using LifecycleCallbacksStorage = OrderedHashMap>; + using LifecycleCallbacksStorage = OrderedHashMap>; using ConstructionStackStorage = Vector, AlreadyConstructedCustomElementMarker>>; static JS::NonnullGCPtr create(JS::Realm& realm, String const& name, String const& local_name, WebIDL::CallbackType& constructor, Vector&& observed_attributes, LifecycleCallbacksStorage&& lifecycle_callbacks, bool form_associated, bool disable_internals, bool disable_shadow) @@ -60,6 +60,8 @@ private: { } + virtual void visit_edges(Visitor& visitor) override; + // https://html.spec.whatwg.org/multipage/custom-elements.html#concept-custom-element-definition-name // A name // A valid custom element name diff --git a/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementRegistry.cpp b/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementRegistry.cpp index ec029675c8..52d60139e6 100644 --- a/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementRegistry.cpp +++ b/Userland/Libraries/LibWeb/HTML/CustomElements/CustomElementRegistry.cpp @@ -201,14 +201,14 @@ JS::ThrowCompletionOr CustomElementRegistry::define(String const& name, We // 2. If callbackValue is not undefined, then set the value of the entry in lifecycleCallbacks with key callbackName to the result of converting callbackValue to the Web IDL Function callback type. Rethrow any exceptions from the conversion. if (!callback_value.is_undefined()) { auto callback = TRY(convert_value_to_callback_function(vm, callback_value)); - lifecycle_callbacks.set(callback_name, JS::make_handle(callback)); + lifecycle_callbacks.set(callback_name, callback); } } // 5. If the value of the entry in lifecycleCallbacks with key "attributeChangedCallback" is not null, then: auto attribute_changed_callback_iterator = lifecycle_callbacks.find(CustomElementReactionNames::attributeChangedCallback); VERIFY(attribute_changed_callback_iterator != lifecycle_callbacks.end()); - if (!attribute_changed_callback_iterator->value.is_null()) { + if (attribute_changed_callback_iterator->value) { // 1. Let observedAttributesIterable be ? Get(constructor, "observedAttributes"). auto observed_attributes_iterable = TRY(constructor->callback->get(JS::PropertyKey { "observedAttributes" })); @@ -253,7 +253,7 @@ JS::ThrowCompletionOr CustomElementRegistry::define(String const& name, We // 2. If callbackValue is not undefined, then set the value of the entry in lifecycleCallbacks with key callbackName to the result of converting callbackValue to the Web IDL Function callback type. Rethrow any exceptions from the conversion. if (!callback_value.is_undefined()) - lifecycle_callbacks.set(callback_name, JS::make_handle(TRY(convert_value_to_callback_function(vm, callback_value)))); + lifecycle_callbacks.set(callback_name, TRY(convert_value_to_callback_function(vm, callback_value))); } }