Skip to content

Commit

Permalink
DOM: Fix moveBefore null parent crash
Browse files Browse the repository at this point in the history
Before this CL, we didn't early-return from `Node::moveBefore()` when
`insertBefore()` threw an error due to an invalid node hierarchy. This
wasn't necessary until we introduced the `MovedFrom()` hook, since
there was no significant code after the `insertBefore()` call. But once
we introduced that hook, we didn't add the early-return before calling
the hook, which caused this crash.

R=masonf, nrosenthal

Bug: 40150299, 388934346
Change-Id: Ie5f7401ed5097b7f64097daa557f315a9889664f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6179644
Commit-Queue: Dominic Farolino <[email protected]>
Reviewed-by: Noam Rosenthal <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1407973}
  • Loading branch information
domfarolino authored and chromium-wpt-export-bot committed Jan 17, 2025
1 parent d2b5a40 commit d0faf28
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions dom/nodes/moveBefore/tentative/Node-moveBefore.html
Original file line number Diff line number Diff line change
Expand Up @@ -329,4 +329,14 @@
await Promise.resolve();
assert_array_equals(reactions, []);
}, "No custom element callbacks are run during disconnected moveBefore()");

// This is a regression test for a Chromium crash: https://crbug.com/388934346.
test(t => {
// This test caused a crash in Chromium because after the detection of invalid
// /node hierarchy, and throwing the JS error, we did not return from native
// code, and continued to operate on the node tree on bad assumptions.
const outer = document.createElement('div');
const div = outer.appendChild(document.createElement('div'));
assert_throws_dom("HIERARCHY_REQUEST_ERR", () => div.moveBefore(outer, null));
}, "Invalid node hierarchy with null old parent does not crash");
</script>

0 comments on commit d0faf28

Please sign in to comment.