LibWeb: Retain calculated Element::ordinal_value for lists

`Element::ordinal_value` is called for every `li` element in
a list (ul, ol, menu).

Before:
  `ordinal_value` iterates through all of the children of the list
  owner. It is called once for each element: complexity $O(n^2)$.

After:
  - Save the result of the first calculation in `m_ordinal_value`
  then return it in subsequent calls.
  - Tree modifications are intercepted and trigger invalidation
    of the first node's `m_ordinal_value`:
    - insert_before
    - append
    - remove
  Results in noticeable performance improvement rendering' large
  lists: from 20s to 4s for 20K elements.

Libraries/LibWeb/DOM/Element.cpp

@@ -1,12 +1,16 @@
 /*
  * Copyright (c) 2018-2024, Andreas Kling <andreas@ladybird.org>
- * Copyright (c) 2022-2023, San Atkins <atkinssj@serenityos.org>
+ * Copyright (c) 2022-2023, Sam Atkins <atkinssj@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #include <AK/AnyOf.h>
+#include <AK/Assertions.h>
+#include <AK/Checked.h>
 #include <AK/Debug.h>
+#include <AK/IterationDecision.h>
+#include <AK/NumericLimits.h>
 #include <AK/StringBuilder.h>
 #include <LibJS/Runtime/NativeFunction.h>
 #include <LibUnicode/CharacterTypes.h>
@@ -3184,23 +3188,22 @@ bool Element::skips_its_contents()
     return false;
 }
 
-size_t Element::number_of_owned_list_items() const
+i32 Element::number_of_owned_list_items() const
 {
-    auto number_of_owned_li_elements = 0;
+    AK::Checked<i32> number_of_owned_li_elements = 0;
-    for_each_child_of_type<DOM::Element>([&](auto& child) {
+    for_each_numbered_item_owned_by_list_owner([&number_of_owned_li_elements]([[maybe_unused]] Element* item) {
-        if (child.list_owner() == this) {
+        number_of_owned_li_elements++;
-            number_of_owned_li_elements++;
-        }
         return IterationDecision::Continue;
     });
-    return number_of_owned_li_elements;
+
+    return number_of_owned_li_elements.value();
 }
 
 // https://html.spec.whatwg.org/multipage/grouping-content.html#list-owner
 Element* Element::list_owner() const
 {
     // Any element whose computed value of 'display' is 'list-item' has a list owner, which is determined as follows:
-    if (!computed_properties() || !computed_properties()->display().is_list_item())
+    if (!m_is_contained_in_list_subtree && (!computed_properties() || !computed_properties()->display().is_list_item()))
         return nullptr;
 
     // 1. If the element is not being rendered, return null; the element has no list owner.
@@ -3216,8 +3219,8 @@ Element* Element::list_owner() const
 
     // 3. If the element has an ol, ul, or menu ancestor, set ancestor to the closest such ancestor element.
     for_each_ancestor([&ancestor](GC::Ref<Node> node) {
-        if (is<HTML::HTMLOListElement>(*node) || is<HTML::HTMLUListElement>(*node) || is<HTML::HTMLMenuElement>(*node)) {
+        if (node->is_html_ol_ul_menu_element()) {
-            ancestor = static_cast<Element const*>(node.ptr());
+            ancestor = static_cast<Element*>(node.ptr());
             return IterationDecision::Break;
         }
         return IterationDecision::Continue;
@@ -3226,7 +3229,7 @@ Element* Element::list_owner() const
     // 4. Return the closest inclusive ancestor of ancestor that produces a CSS box.
     ancestor->for_each_inclusive_ancestor([&ancestor](GC::Ref<Node> node) {
         if (is<Element>(*node) && node->paintable_box()) {
-            ancestor = static_cast<Element const*>(node.ptr());
+            ancestor = static_cast<Element*>(node.ptr());
             return IterationDecision::Break;
         }
         return IterationDecision::Continue;
@@ -3234,62 +3237,75 @@ Element* Element::list_owner() const
     return const_cast<Element*>(ancestor.ptr());
 }
 
-// https://html.spec.whatwg.org/multipage/grouping-content.html#ordinal-value
+void Element::maybe_invalidate_ordinals_for_list_owner(Optional<Element*> skip_node)
-size_t Element::ordinal_value() const
 {
-    // NOTE: The spec provides an algorithm to determine the ordinal value of each element owned by a given list owner.
+    if (Element* owner = list_owner())
-    //       However, we are only interested in the ordinal value of this element.
+        owner->for_each_numbered_item_owned_by_list_owner([&](Element* item) {
+            if (skip_node.has_value() && item == skip_node.value())
+                return IterationDecision::Continue;
 
-    // FIXME: 1. Let i be 1.
+            item->m_ordinal_value = {};
 
-    // 2. If owner is an ol element, let numbering be owner's starting value. Otherwise, let numbering be 1.
+            // Invalidate just the first ordinal in the list of numbered items.
-    auto const* owner = list_owner();
+            // NOTE: This works since this item is the first accessed (preorder) when rendering the list.
-    if (!owner) {
+            //       It will trigger a recalculation of all ordinals on the [first] call to ordinal_value().
+            return IterationDecision::Break;
+        });
+}
+
+// https://html.spec.whatwg.org/multipage/grouping-content.html#ordinal-value
+i32 Element::ordinal_value()
+{
+    if (m_ordinal_value.has_value())
+        return m_ordinal_value.value();
+
+    auto* owner = list_owner();
+    if (!owner)
         return 1;
-    }
 
-    auto numbering = 1;
+    // 1. Let i be 1. [Not necessary]
+    // 2. If owner is an ol element, let numbering be owner's starting value. Otherwise, let numbering be 1.
+    AK::Checked<i32> numbering = 1;
     auto reversed = false;
-    if (is<HTML::HTMLOListElement>(owner)) {
+
+    if (owner->is_html_olist_element()) {
         auto const* ol_element = static_cast<const HTML::HTMLOListElement*>(owner);
         numbering = ol_element->starting_value().value();
         reversed = ol_element->has_attribute(HTML::AttributeNames::reversed);
     }
 
-    // FIXME: 3. Loop : If i is greater than the number of list items that owner owns, then return; all of owner's owned list items have been assigned ordinal values.
+    // 3. Loop : If i is greater than the number of list items that owner owns, then return; all of owner's owned list items have been assigned ordinal values.
-    // FIXME: 4. Let item be the ith of owner's owned list items, in tree order.
+    // NOTE: We use `owner->for_each_numbered_item_in_list` to iterate through the owner's list of owned elements.
+    //       As a result, we don't need `i` as counter (spec) in the list of children, with no material consequences.
+    owner->for_each_numbered_item_owned_by_list_owner([&](Element* item) {
+        // 4. Let item be the ith of owner's owned list items, in tree order. [Not necessary]
+        // 5. If item is an li element that has a value attribute, then:
+        auto value_attribute = item->get_attribute(HTML::AttributeNames::value);
+        if (item->is_html_li_element() && value_attribute.has_value()) {
+            // 1. Let parsed be the result of parsing the value of the attribute as an integer.
+            auto parsed = HTML::parse_integer(value_attribute.value());
 
-    owner->for_each_child_of_type<DOM::Element>([&](auto& item) {
+            // 2. If parsed is not an error, then set numbering to parsed.
-        if (item.list_owner() == owner) {
+            if (parsed.has_value())
-            // 5. If item is an li element that has a value attribute, then:
+                numbering = parsed.value();
-            auto value_attribute = item.get_attribute(HTML::AttributeNames::value);
-            if (is<HTML::HTMLLIElement>(item) && value_attribute.has_value()) {
-                // 1. Let parsed be the result of parsing the value of the attribute as an integer.
-                auto parsed = HTML::parse_integer(value_attribute.value());
-
-                // 2. If parsed is not an error, then set numbering to parsed.
-                if (parsed.has_value())
-                    numbering = parsed.value();
-            }
-
-            // FIXME: 6. The ordinal value of item is numbering.
-            if (&item == this)
-                return IterationDecision::Break;
-
-            // 7. If owner is an ol element, and owner has a reversed attribute, decrement numbering by 1; otherwise, increment numbering by 1.
-            if (reversed) {
-                numbering--;
-            } else {
-                numbering++;
-            }
-
-            // FIXME: 8. Increment i by 1.
         }
+
+        // 6. The ordinal value of item is numbering.
+        item->m_ordinal_value = numbering.value();
+
+        // 7. If owner is an ol element, and owner has a reversed attribute, decrement numbering by 1; otherwise, increment numbering by 1.
+        if (reversed) {
+            numbering--;
+        } else {
+            numbering++;
+        }
+
+        // 8. Increment i by 1. [Not necessary]
+        // 9. Go to the step labeled loop.
         return IterationDecision::Continue;
     });
 
-    // FIXME: 9. Go to the step labeled loop.
+    return m_ordinal_value.value_or(1);
-    return numbering;
 }
 
 bool Element::id_reference_exists(String const& id_reference) const

Libraries/LibWeb/DOM/Element.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/IterationDecision.h>
 #include <AK/Optional.h>
 #include <LibWeb/ARIA/ARIAMixin.h>
 #include <LibWeb/ARIA/AttributeNames.h>
@@ -14,6 +15,7 @@
 #include <LibWeb/Bindings/Intrinsics.h>
 #include <LibWeb/Bindings/ShadowRootPrototype.h>
 #include <LibWeb/CSS/CascadedProperties.h>
+#include <LibWeb/CSS/ComputedProperties.h>
 #include <LibWeb/CSS/CountersSet.h>
 #include <LibWeb/CSS/Selector.h>
 #include <LibWeb/CSS/StyleInvalidation.h>
@@ -465,9 +467,46 @@ public:
         return affected_by_direct_sibling_combinator() || affected_by_indirect_sibling_combinator() || affected_by_sibling_position_or_count_pseudo_class() || affected_by_nth_child_pseudo_class();
     }
 
-    size_t number_of_owned_list_items() const;
+    i32 number_of_owned_list_items() const;
     Element* list_owner() const;
-    size_t ordinal_value() const;
+    void maybe_invalidate_ordinals_for_list_owner(Optional<Element*> skip_node = {});
+    i32 ordinal_value();
+
+    template<typename Callback>
+    void for_each_numbered_item_owned_by_list_owner(Callback callback) const
+    {
+        const_cast<Element*>(this)->for_each_numbered_item_owned_by_list_owner(move(callback));
+    }
+
+    template<typename Callback>
+    void for_each_numbered_item_owned_by_list_owner(Callback callback)
+    {
+        for (auto* node = this->first_child(); node != nullptr; node = node->next_in_pre_order(this)) {
+            if (!is<Element>(*node))
+                continue;
+
+            static_cast<Element*>(node)->m_is_contained_in_list_subtree = true;
+
+            if (node->is_html_ol_ul_menu_element()) {
+                // Skip list nodes and their descendents. They have their own, unrelated ordinals.
+                while (node->last_child() != nullptr) // Find the last node (preorder) in the subtree headed by node. O(1).
+                    node = node->last_child();
+
+                continue;
+            }
+
+            if (!node->layout_node())
+                continue; // Skip nodes that do not participate in the layout.
+
+            auto* element = static_cast<Element*>(node);
+
+            if (!element->computed_properties()->display().is_list_item())
+                continue; // Skip nodes that are not list items.
+
+            if (callback(element) == IterationDecision::Break)
+                return;
+        }
+    }
 
     void set_pointer_capture(WebIDL::Long pointer_id);
     void release_pointer_capture(WebIDL::Long pointer_id);
@@ -594,6 +633,10 @@ private:
 
     // https://drafts.csswg.org/css-contain/#proximity-to-the-viewport
     ProximityToTheViewport m_proximity_to_the_viewport { ProximityToTheViewport::NotDetermined };
+
+    // https://html.spec.whatwg.org/multipage/grouping-content.html#ordinal-value
+    Optional<i32> m_ordinal_value;
+    bool m_is_contained_in_list_subtree { false };
 };
 
 template<>

Libraries/LibWeb/DOM/Node.cpp

@@ -805,6 +805,16 @@ void Node::insert_before(GC::Ref<Node> node, GC::Ptr<Node> child, bool suppress_
         set_needs_layout_tree_update(true, SetNeedsLayoutTreeUpdateReason::NodeInsertBefore);
     }
 
+    // AD-HOC: invalidate the ordinal of the first list_item of the list_owner of the child node, if any.
+    if (child && child->is_element())
+        static_cast<Element*>(child.ptr())->maybe_invalidate_ordinals_for_list_owner();
+    else if (this->is_element() && !this->is_html_ol_ul_menu_element())
+        static_cast<Element*>(this)->maybe_invalidate_ordinals_for_list_owner();
+    // NOTE: If the child node is null and the parent node is an ol, ul or menu element then:
+    //       the new node will be the first in the list of a potential list owner and it will not have
+    //       an ordinal value (default from constructor).
+    // FIXME: This will not work if the child or the parent is not an element. Is insert_before even possible in this situation?
+
     document().bump_dom_tree_version();
 }
 
@@ -854,6 +864,11 @@ WebIDL::ExceptionOr<GC::Ref<Node>> Node::append_child(GC::Ref<Node> node)
 {
     // To append a node to a parent, pre-insert node into parent before null.
     return pre_insert(node, nullptr);
+
+    // AD-HOC: invalidate the ordinal of the first list_item of the first child sibling of the appended node, if any.
+    // NOTE: This works since ordinal values are accessed (for layout and paint) in the preorder of list_item nodes !!
+    if (auto* first_child_element = this->first_child_of_type<Element>())
+        first_child_element->maybe_invalidate_ordinals_for_list_owner();
 }
 
 // https://dom.spec.whatwg.org/#live-range-pre-remove-steps
@@ -917,6 +932,12 @@ void Node::remove(bool suppress_observers)
     // 6. Let oldNextSibling be node’s next sibling.
     GC::Ptr<Node> old_next_sibling = next_sibling();
 
+    // AD-HOC: invalidate the ordinal of the first list_item of the list_owner of the removed node, if any.
+    if (is_element()) {
+        auto* this_element = static_cast<Element*>(this);
+        this_element->maybe_invalidate_ordinals_for_list_owner(this_element);
+    }
+
     if (is_connected()) {
         // Since the tree structure is about to change, we need to invalidate both style and layout.
         // In the future, we should find a way to only invalidate the parts that actually need it.

Libraries/LibWeb/HTML/HTMLLIElement.cpp

@@ -34,6 +34,7 @@ void HTMLLIElement::attribute_changed(FlyString const& local_name, Optional<Stri
     if (local_name == HTML::AttributeNames::value) {
         if (auto* owner = list_owner()) {
             owner->set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::HTMLOListElementOrdinalValues);
+            maybe_invalidate_ordinals_for_list_owner();
         }
     }
 }

Libraries/LibWeb/HTML/HTMLOListElement.cpp

@@ -36,6 +36,8 @@ void HTMLOListElement::attribute_changed(FlyString const& local_name, Optional<S
 
     if (local_name.is_one_of(HTML::AttributeNames::reversed, HTML::AttributeNames::start, HTML::AttributeNames::type)) {
         set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::HTMLOListElementOrdinalValues);
+        if (has_children())
+            first_child_of_type<Element>()->maybe_invalidate_ordinals_for_list_owner();
     }
 }