LibWeb: Implement caching of reflected element array attributes

For attributes like Element.ariaControlsElements, which are a reflection
of FrozenArray<Element>, we must return the same JS::Array object every
time the attribute is invoked - until its contents have changed. This
patch implements caching of the reflected array in accordance with the
spec.

Libraries/LibWeb/ARIA/ARIAMixin.cpp

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <LibJS/Runtime/Array.h>
 #include <LibWeb/ARIA/ARIAMixin.h>
 #include <LibWeb/ARIA/Roles.h>
 #include <LibWeb/DOM/Element.h>
@@ -14,6 +15,13 @@ namespace Web::ARIA {
 ARIAMixin::ARIAMixin() = default;
 ARIAMixin::~ARIAMixin() = default;
 
+void ARIAMixin::visit_edges(GC::Cell::Visitor& visitor) {
+#define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute) \
+    visitor.visit(m_cached_##attribute);
+    ENUMERATE_ARIA_ELEMENT_LIST_REFERENCING_ATTRIBUTES
+#undef __ENUMERATE_ARIA_ATTRIBUTE
+}
+
 // https://www.w3.org/TR/wai-aria-1.2/#introroles
 Optional<Role> ARIAMixin::role_from_role_attribute_value() const
 {
@@ -247,6 +255,16 @@ ENUMERATE_ARIA_ELEMENT_REFERENCING_ATTRIBUTES
     void ARIAMixin::set_##attribute(Optional<Vector<WeakPtr<DOM::Element>>> value) \
     {                                                                              \
         m_##attribute = move(value);                                               \
+    }                                                                              \
+                                                                                   \
+    GC::Ptr<JS::Array> ARIAMixin::cached_##attribute() const                       \
+    {                                                                              \
+        return m_cached_##attribute;                                               \
+    }                                                                              \
+                                                                                   \
+    void ARIAMixin::set_cached_##attribute(GC::Ptr<JS::Array> value)               \
+    {                                                                              \
+        m_cached_##attribute = value;                                              \
     }
 ENUMERATE_ARIA_ELEMENT_LIST_REFERENCING_ATTRIBUTES
 #undef __ENUMERATE_ARIA_ATTRIBUTE

Libraries/LibWeb/ARIA/ARIAMixin.h

@@ -68,13 +68,18 @@ public:
 
 #define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute)  \
     Optional<Vector<WeakPtr<DOM::Element>>> const& attribute() const; \
-    void set_##attribute(Optional<Vector<WeakPtr<DOM::Element>>> value);
+    void set_##attribute(Optional<Vector<WeakPtr<DOM::Element>>>);    \
+                                                                      \
+    GC::Ptr<JS::Array> cached_##attribute() const;                    \
+    void set_cached_##attribute(GC::Ptr<JS::Array>);
     ENUMERATE_ARIA_ELEMENT_LIST_REFERENCING_ATTRIBUTES
 #undef __ENUMERATE_ARIA_ATTRIBUTE
 
 protected:
     ARIAMixin();
 
+    void visit_edges(GC::Cell::Visitor&);
+
     virtual bool id_reference_exists(String const&) const = 0;
 
 private:
@@ -84,7 +89,8 @@ private:
 #undef __ENUMERATE_ARIA_ATTRIBUTE
 
 #define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute) \
-    Optional<Vector<WeakPtr<DOM::Element>>> m_##attribute;
+    Optional<Vector<WeakPtr<DOM::Element>>> m_##attribute;           \
+    GC::Ptr<JS::Array> m_cached_##attribute;
     ENUMERATE_ARIA_ELEMENT_LIST_REFERENCING_ATTRIBUTES
 #undef __ENUMERATE_ARIA_ATTRIBUTE
 };

Libraries/LibWeb/DOM/Element.cpp

@@ -106,6 +106,7 @@ void Element::visit_edges(Cell::Visitor& visitor)
     Base::visit_edges(visitor);
     SlottableMixin::visit_edges(visitor);
     Animatable::visit_edges(visitor);
+    ARIAMixin::visit_edges(visitor);
 
     visitor.visit(m_attributes);
     visitor.visit(m_inline_style);

Libraries/LibWeb/WebIDL/AbstractOperations.cpp

@@ -528,4 +528,25 @@ template JS::ThrowCompletionOr<UnsignedLong> convert_to_int(JS::VM& vm, JS::Valu
 template JS::ThrowCompletionOr<LongLong> convert_to_int(JS::VM& vm, JS::Value, EnforceRange, Clamp);
 template JS::ThrowCompletionOr<UnsignedLongLong> convert_to_int(JS::VM& vm, JS::Value, EnforceRange, Clamp);
 
+// AD-HOC: For same-object caching purposes, this can be used to compare a cached JS array of DOM::Elements with another
+//         list. Either list can be null, in which case they are considered the same only if they are both null.
+bool lists_contain_same_elements(GC::Ptr<JS::Array> array, Optional<GC::RootVector<GC::Ref<DOM::Element>>> const& elements)
+{
+    if (!array || !elements.has_value())
+        return !array && !elements.has_value();
+
+    bool is_equivalent = array->indexed_properties().array_like_size() == elements->size();
+
+    for (size_t i = 0; is_equivalent && i < elements->size(); ++i) {
+        auto cached_value = array->get_without_side_effects(JS::PropertyKey { i });
+        auto const& cached_element = as<DOM::Element>(cached_value.as_object());
+
+        auto it = elements->find_if([&](auto const& element) { return element.ptr() == &cached_element; });
+        if (it == elements->end())
+            is_equivalent = false;
+    }
+
+    return is_equivalent;
+}
+
 }

Libraries/LibWeb/WebIDL/AbstractOperations.h

@@ -10,6 +10,7 @@
 
 #include <AK/Forward.h>
 #include <LibGC/Ptr.h>
+#include <LibGC/RootVector.h>
 #include <LibJS/Forward.h>
 #include <LibWeb/Forward.h>
 
@@ -54,4 +55,6 @@ enum class Clamp {
 template<Integral T>
 JS::ThrowCompletionOr<T> convert_to_int(JS::VM& vm, JS::Value, EnforceRange enforce_range = EnforceRange::No, Clamp clamp = Clamp::No);
 
+bool lists_contain_same_elements(GC::Ptr<JS::Array> array, Optional<GC::RootVector<GC::Ref<DOM::Element>>> const& elements);
+
 }

Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp

@@ -3748,6 +3748,9 @@ static void generate_prototype_or_global_mixin_definitions(IDL::Interface const&
     for (auto& attribute : interface.attributes) {
         if (attribute.extended_attributes.contains("FIXME"))
             continue;
+
+        bool generated_reflected_element_array = false;
+
         auto attribute_generator = generator.fork();
         attribute_generator.set("attribute.name", attribute.name);
         attribute_generator.set("attribute.getter_callback", attribute.getter_callback_name);
@@ -4054,18 +4057,14 @@ JS_DEFINE_NATIVE_FUNCTION(@class_name@::@attribute.getter_callback@)
             // inherits from Element, then with attr being the reflected content attribute name:
             // FIXME: Handle "an interface that inherits from Element".
             else if (is_nullable_frozen_array_of_single_type(attribute.type, "Element"sv)) {
+                generated_reflected_element_array = true;
+
                 // 1. Let elements be the result of running this's get the attr-associated elements.
                 attribute_generator.append(R"~~~(
     static auto content_attribute = "@attribute.reflect_name@"_fly_string;
 
     auto retval = impl->get_the_attribute_associated_elements(content_attribute, TRY(throw_dom_exception_if_needed(vm, [&] { return impl->@attribute.cpp_name@(); })));
 )~~~");
-
-                // FIXME: 2. If the contents of elements is equal to the contents of this's cached attr-associated elements, then return
-                //           this's cached attr-associated elements object.
-                // FIXME: 3. Let elementsAsFrozenArray be elements, converted to a FrozenArray<T>?.
-                // FIXME: 4. Set this's cached attr-associated elements to elements.
-                // FIXME: 5. Set this's cached attr-associated elements object to elementsAsFrozenArray.
             } else {
                 attribute_generator.append(R"~~~(
     auto retval = impl->get_attribute_value("@attribute.reflect_name@"_fly_string);
@@ -4083,6 +4082,19 @@ JS_DEFINE_NATIVE_FUNCTION(@class_name@::@attribute.getter_callback@)
     Bindings::invoke_custom_element_reactions(queue);
 )~~~");
             }
+
+            if (generated_reflected_element_array) {
+                // 2. If the contents of elements is equal to the contents of this's cached attr-associated elements,
+                //    then return this's cached attr-associated elements object.
+                attribute_generator.append(R"~~~(
+    auto cached_@attribute.cpp_name@ = TRY(throw_dom_exception_if_needed(vm, [&] { return impl->cached_@attribute.cpp_name@(); }));
+    if (WebIDL::lists_contain_same_elements(cached_@attribute.cpp_name@, retval))
+        return cached_@attribute.cpp_name@;
+
+    auto result = TRY([&]() -> JS::ThrowCompletionOr<JS::Value> {
+)~~~");
+            }
+
         } else {
             if (!attribute.extended_attributes.contains("CEReactions")) {
                 attribute_generator.append(R"~~~(
@@ -4127,6 +4139,24 @@ JS_DEFINE_NATIVE_FUNCTION(@class_name@::@attribute.getter_callback@)
 
         generate_return_statement(generator, *attribute.type, interface);
 
+        if (generated_reflected_element_array) {
+            // 3. Let elementsAsFrozenArray be elements, converted to a FrozenArray<T>?.
+            // 4. Set this's cached attr-associated elements to elements.
+            // 5. Set this's cached attr-associated elements object to elementsAsFrozenArray.
+            attribute_generator.append(R"~~~(
+    }());
+
+    if (result.is_null()) {
+        TRY(throw_dom_exception_if_needed(vm, [&] { impl->set_cached_@attribute.cpp_name@({}); }));
+    } else {
+        auto& array = as<JS::Array>(result.as_object());
+        TRY(throw_dom_exception_if_needed(vm, [&] { impl->set_cached_@attribute.cpp_name@(&array); }));
+    }
+
+    return result;
+)~~~");
+        }
+
         attribute_generator.append(R"~~~(
 }
 )~~~");

Tests/LibWeb/Text/expected/wpt-import/html/dom/aria-element-reflection.txt

@@ -2,8 +2,8 @@ Harness status: OK
 
 Found 27 tests
 
-22 Pass
+25 Pass
-5 Fail
+2 Fail
 Pass	aria-activedescendant element reflection
 Pass	aria-activedescendant If the content attribute is set directly, the IDL attribute getter always returns the first element whose ID matches the content attribute.
 Pass	aria-activedescendant Setting the IDL attribute to an element which is not the first element in DOM order with its ID causes the content attribute to be an empty string
@@ -16,7 +16,7 @@ Pass	aria-activedescendant Changing the ID of an element doesn't lose the refere
 Pass	aria-activedescendant Reparenting an element into a descendant shadow scope hides the element reference.
 Pass	aria-activedescendant Reparenting referenced element cannot cause retargeting of reference.
 Pass	aria-activedescendant Element reference set in invalid scope remains intact throughout move to valid scope.
-Fail	aria-labelledby.
+Pass	aria-labelledby.
 Pass	aria-controls.
 Pass	aria-describedby.
 Pass	aria-flowto.
@@ -28,6 +28,6 @@ Pass	aria-activedescendant Reparenting.
 Pass	aria-activedescendant Attaching element reference before it's inserted into the DOM.
 Pass	aria-activedescendant Cross-document references and moves.
 Pass	aria-activedescendant Adopting element keeps references.
-Fail	Caching invariant different attributes.
+Pass	Caching invariant different attributes.
-Fail	Caching invariant different elements.
+Pass	Caching invariant different elements.
 Pass	Passing values of the wrong type should throw a TypeError