LibWeb: Avoid O(n^2) traversal in play-or-cancel-animations logic
Some checks are pending
CI / Lagom (x86_64, Sanitizer_CI, false, ubuntu-24.04, Linux, GNU) (push) Waiting to run
CI / Lagom (arm64, Sanitizer_CI, false, macos-15, macOS, Clang) (push) Waiting to run
CI / Lagom (x86_64, Fuzzers_CI, false, ubuntu-24.04, Linux, Clang) (push) Waiting to run
CI / Lagom (x86_64, Sanitizer_CI, true, ubuntu-24.04, Linux, Clang) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (arm64, macos-15, macOS, macOS-universal2) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (x86_64, ubuntu-24.04, Linux, Linux-x86_64) (push) Waiting to run
Run test262 and test-wasm / run_and_update_results (push) Waiting to run
Lint Code / lint (push) Waiting to run
Label PRs with merge conflicts / auto-labeler (push) Waiting to run
Push notes / build (push) Waiting to run

The play_or_cancel_animations_after_display_property_change() helper
was being called by Node::inserted() and Node::removed_from() and then
recursing into the shadow-including subtree.

This had quadratic complexity since inserted() and removed_from() are
themselves already invoked recursively for everything in the
shadow-including subtree.

Only one caller of this API actually needed the recursive behavior,
so this patch moves that responsibility to the caller and puts the logic
in style recomputation instead.

1.02x speedup on Speedometer's TodoMVC-jQuery.
This commit is contained in:
Andreas Kling 2025-05-11 12:55:51 +02:00 committed by Andreas Kling
commit 096eed35cc
Notes: github-actions[bot] 2025-05-11 18:23:14 +00:00
4 changed files with 44 additions and 44 deletions

View file

@ -1645,16 +1645,12 @@ void Node::post_connection()
void Node::inserted()
{
set_needs_style_update(true);
play_or_cancel_animations_after_display_property_change();
}
void Node::removed_from(Node*, Node&)
{
m_layout_node = nullptr;
m_paintable = nullptr;
play_or_cancel_animations_after_display_property_change();
}
// https://dom.spec.whatwg.org/#concept-node-move-ext
@ -3119,44 +3115,6 @@ bool Node::has_inclusive_ancestor_with_display_none()
return false;
}
void Node::play_or_cancel_animations_after_display_property_change()
{
// OPTIMIZATION: We don't care about animations in disconnected subtrees.
if (!is_connected())
return;
// https://www.w3.org/TR/css-animations-1/#animations
// Setting the display property to none will terminate any running animation applied to the element and its descendants.
// If an element has a display of none, updating display to a value other than none will start all animations applied to
// the element by the animation-name property, as well as all animations applied to descendants with display other than none.
auto has_display_none_inclusive_ancestor = this->has_inclusive_ancestor_with_display_none();
auto play_or_cancel_depending_on_display = [&](Animations::Animation& animation) {
if (has_display_none_inclusive_ancestor) {
animation.cancel();
} else {
HTML::TemporaryExecutionContext context(realm());
animation.play().release_value_but_fixme_should_propagate_errors();
}
};
for_each_shadow_including_inclusive_descendant([&](auto& node) {
if (!node.is_element())
return TraversalDecision::Continue;
auto const& element = static_cast<Element const&>(node);
if (auto animation = element.cached_animation_name_animation({}))
play_or_cancel_depending_on_display(*animation);
for (auto i = 0; i < to_underlying(CSS::PseudoElement::KnownPseudoElementCount); i++) {
auto pseudo_element = static_cast<CSS::PseudoElement>(i);
if (auto animation = element.cached_animation_name_animation(pseudo_element))
play_or_cancel_depending_on_display(*animation);
}
return TraversalDecision::Continue;
});
}
}
namespace IPC {