Parcourir la source

LibWeb: Make DOM::NamedNodeMap forward its ref count to DOM::Element

This allows JS to keep an element alive by retaining a reference to
element.attributes
Andreas Kling il y a 3 ans
Parent
commit
7fc770cfac

+ 7 - 21
Userland/Libraries/LibWeb/DOM/NamedNodeMap.cpp

@@ -11,16 +11,14 @@
 
 namespace Web::DOM {
 
-NonnullRefPtr<NamedNodeMap> NamedNodeMap::create(Element const& associated_element)
+NonnullRefPtr<NamedNodeMap> NamedNodeMap::create(Element& associated_element)
 {
     return adopt_ref(*new NamedNodeMap(associated_element));
 }
 
-NamedNodeMap::NamedNodeMap(Element const& associated_element)
-    : m_associated_element(associated_element)
+NamedNodeMap::NamedNodeMap(Element& associated_element)
+    : RefCountForwarder(associated_element)
 {
-    // Note: To avoid a reference cycle between Element, NamedNodeMap, and Attribute, do not store a
-    // strong reference to the associated element.
 }
 
 // https://dom.spec.whatwg.org/#ref-for-dfn-supported-property-indices%E2%91%A3
@@ -32,10 +30,6 @@ bool NamedNodeMap::is_supported_property_index(u32 index) const
 // https://dom.spec.whatwg.org/#ref-for-dfn-supported-property-names%E2%91%A0
 Vector<String> NamedNodeMap::supported_property_names() const
 {
-    auto associated_element = m_associated_element.strong_ref();
-    if (!associated_element)
-        return {};
-
     // 1. Let names be the qualified names of the attributes in this NamedNodeMap object’s attribute list, with duplicates omitted, in order.
     Vector<String> names;
     names.ensure_capacity(m_attributes.size());
@@ -47,7 +41,7 @@ Vector<String> NamedNodeMap::supported_property_names() const
 
     // 2. If this NamedNodeMap object’s element is in the HTML namespace and its node document is an HTML document, then for each name in names:
     // FIXME: Handle the second condition, assume it is an HTML document for now.
-    if (associated_element->namespace_uri() == Namespace::HTML) {
+    if (associated_element().namespace_uri() == Namespace::HTML) {
         // 1. Let lowercaseName be name, in ASCII lowercase.
         // 2. If lowercaseName is not equal to name, remove name from names.
         names.remove_all_matching([](auto const& name) { return name != name.to_lowercase(); });
@@ -103,16 +97,12 @@ Attribute* NamedNodeMap::get_attribute(StringView qualified_name, size_t* item_i
 // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name
 Attribute const* NamedNodeMap::get_attribute(StringView qualified_name, size_t* item_index) const
 {
-    auto associated_element = m_associated_element.strong_ref();
-    if (!associated_element)
-        return nullptr;
-
     if (item_index)
         *item_index = 0;
 
     // 1. If element is in the HTML namespace and its node document is an HTML document, then set qualifiedName to qualifiedName in ASCII lowercase.
     // FIXME: Handle the second condition, assume it is an HTML document for now.
-    bool compare_as_lowercase = associated_element->namespace_uri() == Namespace::HTML;
+    bool compare_as_lowercase = associated_element().namespace_uri() == Namespace::HTML;
 
     // 2. Return the first attribute in element’s attribute list whose qualified name is qualifiedName; otherwise null.
     for (auto const& attribute : m_attributes) {
@@ -134,12 +124,8 @@ Attribute const* NamedNodeMap::get_attribute(StringView qualified_name, size_t*
 // https://dom.spec.whatwg.org/#concept-element-attributes-set
 ExceptionOr<Attribute const*> NamedNodeMap::set_attribute(Attribute& attribute)
 {
-    auto associated_element = m_associated_element.strong_ref();
-    if (!associated_element)
-        return nullptr;
-
     // 1. If attr’s element is neither null nor element, throw an "InUseAttributeError" DOMException.
-    if ((attribute.owner_element() != nullptr) && (attribute.owner_element() != associated_element))
+    if ((attribute.owner_element() != nullptr) && (attribute.owner_element() != &associated_element()))
         return InUseAttributeError::create("Attribute must not already be in use"sv);
 
     // 2. Let oldAttr be the result of getting an attribute given attr’s namespace, attr’s local name, and element.
@@ -193,7 +179,7 @@ void NamedNodeMap::append_attribute(Attribute& attribute)
     m_attributes.append(attribute);
 
     // 3. Set attribute’s element to element.
-    attribute.set_owner_element(m_associated_element);
+    attribute.set_owner_element(&associated_element());
 }
 
 // https://dom.spec.whatwg.org/#concept-element-attributes-remove-by-name

+ 7 - 4
Userland/Libraries/LibWeb/DOM/NamedNodeMap.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <AK/NonnullRefPtrVector.h>
+#include <AK/RefCountForwarder.h>
 #include <AK/RefCounted.h>
 #include <AK/String.h>
 #include <AK/StringView.h>
@@ -19,13 +20,13 @@ namespace Web::DOM {
 
 // https://dom.spec.whatwg.org/#interface-namednodemap
 class NamedNodeMap final
-    : public RefCounted<NamedNodeMap>
+    : public RefCountForwarder<Element>
     , public Bindings::Wrappable {
 
 public:
     using WrapperType = Bindings::NamedNodeMapWrapper;
 
-    static NonnullRefPtr<NamedNodeMap> create(Element const& associated_element);
+    static NonnullRefPtr<NamedNodeMap> create(Element& associated_element);
     ~NamedNodeMap() = default;
 
     bool is_supported_property_index(u32 index) const;
@@ -49,9 +50,11 @@ public:
     Attribute const* remove_attribute(StringView qualified_name);
 
 private:
-    explicit NamedNodeMap(Element const& associated_element);
+    explicit NamedNodeMap(Element& associated_element);
+
+    Element& associated_element() { return ref_count_target(); }
+    Element const& associated_element() const { return ref_count_target(); }
 
-    WeakPtr<Element> m_associated_element;
     NonnullRefPtrVector<Attribute> m_attributes;
 };