mirrors/ladybird
1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-01-22 09:12:13 -05:00
Code Issues Projects Releases Packages Wiki Activity

LibWeb: Make CustomElementDefinition relationship with GC more sane

Browse source 
1. Stop using GC::Root in member variables, since that usually creates
   a realm leak.

2. Stop putting OrderedHashMap<FlyString, GC::Ptr> on the stack while
   setting these up, since that won't protect the objects from GC.
This commit is contained in:
Andreas Kling 2024-12-26 18:17:41 +01:00 committed by Andreas Kling
parent 062e33438e
commit ceefe7d858
Notes: github-actions[bot] 2024-12-27 09:03:54 +00:00 
Author: https://github.com/awesomekling
Commit: https://github.com/LadybirdBrowser/ladybird/commit/ceefe7d858d
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/3049
5 changed files with 17 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Libraries/LibWeb/DOM/Element.cpp
View file

@ -2181,7 +2181,7 @@ JS::ThrowCompletionOr<void> Element::upgrade_element(GC::Ref<HTML::CustomElement
}
// 6. Add element to the end of definition's construction stack.
custom_element_definition->construction_stack().append(GC::make_root(this));
custom_element_definition->construction_stack().append(GC::Ref { *this });
// 7. Let C be definition's constructor.
auto& constructor = custom_element_definition->constructor();

5
Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.cpp
View file

@ -13,6 +13,11 @@ void CustomElementDefinition::visit_edges(Visitor& visitor)
Base::visit_edges(visitor);
visitor.visit(m_constructor);
visitor.visit(m_lifecycle_callbacks);
for (auto& entry : m_construction_stack) {
entry.visit(
[&](GC::Ref<DOM::Element>& element) { visitor.visit(element); },
[&](AlreadyConstructedCustomElementMarker&) {});
}
}
}

20
Libraries/LibWeb/HTML/CustomElements/CustomElementDefinition.h
View file

@ -20,10 +20,7 @@ class CustomElementDefinition : public JS::Cell {
GC_CELL(CustomElementDefinition, JS::Cell);
GC_DECLARE_ALLOCATOR(CustomElementDefinition);
using LifecycleCallbacksStorage = OrderedHashMap<FlyString, GC::Ptr<WebIDL::CallbackType>>;
using ConstructionStackStorage = Vector<Variant<GC::Root<DOM::Element>, AlreadyConstructedCustomElementMarker>>;
static GC::Ref<CustomElementDefinition> create(JS::Realm& realm, String const& name, String const& local_name, WebIDL::CallbackType& constructor, Vector<String>&& observed_attributes, LifecycleCallbacksStorage&& lifecycle_callbacks, bool form_associated, bool disable_internals, bool disable_shadow)
static GC::Ref<CustomElementDefinition> create(JS::Realm& realm, String const& name, String const& local_name, WebIDL::CallbackType& constructor, Vector<String>&& observed_attributes, OrderedHashMap<FlyString, GC::Root<WebIDL::CallbackType>> lifecycle_callbacks, bool form_associated, bool disable_internals, bool disable_shadow)
{
return realm.create<CustomElementDefinition>(name, local_name, constructor, move(observed_attributes), move(lifecycle_callbacks), form_associated, disable_internals, disable_shadow);
}
@ -38,26 +35,27 @@ class CustomElementDefinition : public JS::Cell {
Vector<String> const& observed_attributes() const { return m_observed_attributes; }
LifecycleCallbacksStorage const& lifecycle_callbacks() const { return m_lifecycle_callbacks; }
auto const& lifecycle_callbacks() const { return m_lifecycle_callbacks; }
ConstructionStackStorage& construction_stack() { return m_construction_stack; }
ConstructionStackStorage const& construction_stack() const { return m_construction_stack; }
auto& construction_stack() { return m_construction_stack; }
auto const& construction_stack() const { return m_construction_stack; }
bool form_associated() const { return m_form_associated; }
bool disable_internals() const { return m_disable_internals; }
bool disable_shadow() const { return m_disable_shadow; }
private:
CustomElementDefinition(String const& name, String const& local_name, WebIDL::CallbackType& constructor, Vector<String>&& observed_attributes, LifecycleCallbacksStorage&& lifecycle_callbacks, bool form_associated, bool disable_internals, bool disable_shadow)
CustomElementDefinition(String const& name, String const& local_name, WebIDL::CallbackType& constructor, Vector<String>&& observed_attributes, OrderedHashMap<FlyString, GC::Root<WebIDL::CallbackType>>&& lifecycle_callbacks, bool form_associated, bool disable_internals, bool disable_shadow)
: m_name(name)
, m_local_name(local_name)
, m_constructor(constructor)
, m_observed_attributes(move(observed_attributes))
, m_lifecycle_callbacks(move(lifecycle_callbacks))
, m_form_associated(form_associated)
, m_disable_internals(disable_internals)
, m_disable_shadow(disable_shadow)
{
for (auto& [key, value] : lifecycle_callbacks)
m_lifecycle_callbacks.set(key, value.cell());
}
virtual void visit_edges(Visitor& visitor) override;
@ -87,13 +85,13 @@ private:
// "formAssociatedCallback", "formDisabledCallback", "formResetCallback", and "formStateRestoreCallback".
// The corresponding values are either a Web IDL Function callback function type value, or null.
// By default the value of each entry is null.
LifecycleCallbacksStorage m_lifecycle_callbacks;
OrderedHashMap<FlyString, GC::Ptr<WebIDL::CallbackType>> m_lifecycle_callbacks;
// https://html.spec.whatwg.org/multipage/custom-elements.html#concept-custom-element-definition-construction-stack
// A construction stack
// A list, initially empty, that is manipulated by the upgrade an element algorithm and the HTML element constructors.
// Each entry in the list will be either an element or an already constructed marker.
ConstructionStackStorage m_construction_stack;
Vector<Variant<GC::Ref<DOM::Element>, AlreadyConstructedCustomElementMarker>> m_construction_stack;
// https://html.spec.whatwg.org/multipage/custom-elements.html#concept-custom-element-definition-form-associated
// A form-associated boolean

2
Libraries/LibWeb/HTML/CustomElements/CustomElementRegistry.cpp
View file

@ -185,7 +185,7 @@ JS::ThrowCompletionOr<void> CustomElementRegistry::define(String const& name, We
// NOTE: This is not in the spec, but is required because of how we catch the exception by using a lambda, meaning we need to define this
// variable outside of it to use it later.
CustomElementDefinition::LifecycleCallbacksStorage lifecycle_callbacks;
OrderedHashMap<FlyString, GC::Root<WebIDL::CallbackType>> lifecycle_callbacks;
// 14. Run the following steps while catching any exceptions:
auto get_definition_attributes_from_constructor = [&]() -> JS::ThrowCompletionOr<void> {

2
Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp
View file

@ -2635,7 +2635,7 @@ static void generate_html_constructor(SourceGenerator& generator, IDL::Construct
return JS::throw_completion(WebIDL::InvalidStateError::create(realm, "Custom element has already been constructed"_string));
// 12. Perform ? element.[[SetPrototypeOf]](prototype).
auto actual_element = element.get<GC::Root<DOM::Element>>();
auto actual_element = element.get<GC::Ref<DOM::Element>>();
TRY(actual_element->internal_set_prototype_of(&prototype.as_object()));
// 13. Replace the last entry in definition's construction stack with an already constructed marker.