← All issues

Consolidate DocumentFragment insertions into a single notification

0bc30db

Source/WebCore/dom/ContainerNode.cpp

+template<typename DOMInsertionWork>
+static ALWAYS_INLINE void executeNodeInsertionWithScriptAssertion(ContainerNode& containerNode, NodeVector& children, Node* beforeChild,
+ ContainerNode::ChildChange::Source source, ReplacedAllChildren replacedAllChildren, NOESCAPE const DOMInsertionWork& doNodeInsertion)
+{
+ if (children.isEmpty())
+ return;
+
+ auto childChange = makeChildChangeForInsertion(containerNode, children, beforeChild, source, replacedAllChildren);
+
+ NodeVector postInsertionNotificationTargets;
+ {
+ WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+ Style::ChildChangeInvalidation styleInvalidation(containerNode, childChange);
+
+ if (containerNode.isShadowRoot() || containerNode.isInShadowTree()) [[unlikely]]
+ containerNode.containingShadowRoot()->resolveSlotsBeforeNodeInsertionOrRemoval();
+
+ for (auto& child : children) {
+ doNodeInsertion(child);
+ ChildListMutationScope(containerNode).childAdded(child);
+ notifyChildNodeInserted(containerNode, child, postInsertionNotificationTargets);
+ }
+ }
+
+ containerNode.childrenChanged(childChange);
+
+ for (auto& target : postInsertionNotificationTargets)
+ target->didFinishInsertingNode();
+
+ if (source == ContainerNode::ChildChange::Source::API) {
+ for (auto& child : children)
+ dispatchChildInsertionEvents(child);
+ }
+}

The DOM spec requires that inserting a DocumentFragment is atomic: the fragment's children are moved to the target as a unit, and no scripts or mutation events fire until all nodes are in place. WebKit's old implementation called executeNodeInsertionWithScriptAssertion once per child node, which meant childrenChanged and mutation event dispatch happened incrementally — a MutationObserver callback firing after node N but before node N+1 could modify the tree, changing sibling relationships or node counts that subsequent insertion steps depended on.

The new Vector-based variant of executeNodeInsertionWithScriptAssertion drives all child insertions within a single ScriptDisallowedScope block. The loop calls doNodeInsertion, ChildListMutationScope::childAdded, and notifyChildNodeInserted for each child while scripts are blocked. Only after the scope exits do childrenChanged (once for the batch), didFinishInsertingNode (per target), and dispatchChildInsertionEvents (per child) fire. Elements with post-insertion logic — HTMLSelectElement, HTMLDetailsElement, SVGAnimateMotionElement — were updated to handle multi-node callbacks.

Before (per-node loop):
  container.appendChild(fragment):
    insert node[0] → childrenChanged() → MutationEvent  ← script runs
    insert node[1] → childrenChanged() → MutationEvent  ← script runs

After (atomic batch):
  container.appendChild(fragment):
    ScriptDisallowedScope {
      insert node[0]; insert node[1]; ...
    }
    childrenChanged(all nodes)
    didFinishInsertingNode(each)
    dispatchChildInsertionEvents(node[0])  ← script only after all inserted
    dispatchChildInsertionEvents(node[1])

This closes a class of TOCTOU windows where mutation observers or inline event handlers could observe — and modify — the DOM mid-fragment-insertion while it was in a partially-assembled state. It also changes MutationObserver record structure: all simultaneously-inserted nodes now appear in a single record's addedNodes list rather than across multiple records, which affects any code that assumes one record per inserted node.

🔒

New multi-node callback paths in security-sensitive element types and style invalidation — several edge cases are worth security investigation.

Subscribe to read more