Skip to content

Commit

Permalink
Add append_to argument to Clone() to avoid extra InsertedInto calls
Browse files Browse the repository at this point in the history
Because of the order of operations for Clone(), previous to this CL,
the typical sequence would be:

 1. Clone the element
 2. Clone the children of the element (recursing to step #1).
 3. AppendChild() each cloned child to its parent cloned element.
 4. (in the caller of Clone) AppendChild the cloned element to its
    eventual parent.

Because each AppendChild triggers a call to Node::InsertedInto() for
*all descendants of the appended element* [1], the fact that the tree
is constructed bottom-up (leaf nodes first) means that InsertedInto()
is called N^2 times, where N is the depth of the cloned tree.

Because clone-and-append is a very common pattern, this CL adds an
`append_to` argument to `Clone()`, which appends to the parent before
appending the children.

This CL also adds a perf test for this scenario (cloning a deep tree).
Locally, on a debug build, this test gives 0.13 runs/s before this CL,
and 0.40 runs/s after, for a 3.1X speedup.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/container_node.cc;l=1006;drc=5d60017dba57e86d477634812e1340127734f8a7

Bug: 1453291
Change-Id: Icdd75c45aa5ecc4fe8bb5d1ff0b7a2b27bec2171
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4728983
Reviewed-by: David Baron <[email protected]>
Commit-Queue: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1177922}
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Aug 1, 2023
1 parent ad7dbb3 commit d73d9dc
Showing 1 changed file with 18 additions and 0 deletions.
18 changes: 18 additions & 0 deletions custom-elements/form-associated/form-disabled-callback.html
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,23 @@
fieldset.disabled = false;
assert_array_equals(container.querySelector('my-control').disabledHistory(), [true]);
}, 'Toggling "disabled" attribute on a <fieldset> does not trigger a callback on disabled custom element descendant');

test(() => {
const template = document.createElement('template');
template.innerHTML = '<my-control></my-control>';
const container = document.createElement('fieldset');
document.body.appendChild(container);
container.disabled = true;
container.appendChild(template.content.cloneNode(true));
assert_array_equals(container.querySelector('my-control').disabledHistory(), [true]);
}, 'Callback triggered during a clone/append operation, with disabled state provided by ancestor');

test(() => {
const container = document.createElement('div');
document.body.appendChild(container);
container.innerHTML = '<fieldset disabled><my-control></my-control></fieldset>';
const clone = container.cloneNode(true);
assert_array_equals(container.querySelector('my-control').disabledHistory(), [true]);
}, 'Callback triggered during a clone operation, with disabled state provided by ancestor');
</script>
</body>

0 comments on commit d73d9dc

Please sign in to comment.