← WebKit — 2026-W36
da79705ade01755efb0bf2144b1636e4e744a592 ScrollingStateTree::insertNode does redundant work reordering children on pages with many sibling scrolling nodes
severity medium
class OOB-write
confidence 0.5
signal score 8.97
Source/WTF/
patch-now
Primitive: ScrollingStateTree::insertNode called insertChild at a childIndex that could be out of range after removeChild shrank the children vector.
Triage note: Replaces parent->removeChild(node) + insertChild(node, childIndex) with a find() of the current index and a bounds-clamped children.moveTo(currentIndex, min(childIndex, size-1)); the new Vector::moveTo uses RELEASE_ASSERT_WITH_SECURITY_IMPLICATION bounds checks. This removes a potential out-of-range insert, though it also reads as a reorder correctness refactor and has no RC marker, so confidence is moderate.
Triage note: Replaces parent->removeChild(node) + insertChild(node, childIndex) with a find() of the current index and a bounds-clamped children.moveTo(currentIndex, min(childIndex, size-1)); the new Vector::moveTo uses RELEASE_ASSERT_WITH_SECURITY_IMPLICATION bounds checks. This removes a potential out-of-range insert, though it also reads as a reorder correctness refactor and has no RC marker, so confidence is moderate.
Structural signals: security_assert (+4.87), clamp_saturate (+1.8), area:Source/WTF/ (+1.0), surgical (+1.3)
Commit message
ScrollingStateTree::insertNode does redundant work reordering children on pages with many sibling scrolling nodes
https://bugs.webkit.org/show_bug.cgi?id=322891
rdar://186143008
Reviewed by Chris Dumez and Simon Fraser.
ScrollingStateNode keeps its children in a Vector. When insertNode() moves an
existing node to a new index it called removeChild() (a linear scan plus a tail
memmove) followed by insertChild()/appendChild() (another tail memmove). Since
registerScrollingNodeID() runs this once per layer during every compositing
update, a parent with N children that need reordering does more shifting than
necessary.
Add Vector::moveTo(), which relocates the element at one index to another by
lifting it out, shifting only the elements between the current and target
positions into the gap with Vector's own TypeOperations::moveOverlapping(), and
dropping it back at its new home. Every element outside that range stays put.
ScrollingStateTree::insertNode() now finds the node once and calls moveTo()
instead of remove + insert.
This is a constant-factor improvement, not an asymptotic one: the find() is
still O(n) and moveTo() shifts O(distance) elements, so a full reordering of N
siblings remains O(n^2) in the worst case. What it removes is the redundant
work each reorder did before -- one bounded shift instead of a full-length
remove memmove plus a full-length insert memmove -- and, in the common case
where a node moves only a short distance, the shift touches just the elements
in between rather than the whole tail.
The reorder path is unchanged behaviorally, so it stays covered by existing
tests. A childIndex of notFound (or otherwise past the end) still means
"append": it is clamped to the last slot, preserving the tolerance
insertChild() has long needed for the out-of-range indices that crash data
shows do occur, rather than tripping moveTo()'s bounds assertion. moveTo()
itself gets direct coverage in TestWebKitAPI, including same-position and
move-only element cases.
* Source/WTF/wtf/Vector.h:
(WTF::Vector::moveTo):
* Source/WebCore/page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::insertNode):
* Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:
(TestWebKitAPI::TEST):
Canonical link: https://commits.webkit.org/320527@main
Before / after
Loading diff…
On This Page