[wpt-sync] Sync PR 27001 - Reland "Fix several crashes when nested slots are removed from a ShadowRoot"
Categories
(Core :: DOM: Core & HTML, task, P4)
Tracking
()
Tracking | Status | |
---|---|---|
firefox86 | --- | fixed |
People
(Reporter: wpt-sync, Unassigned)
References
()
Details
(Whiteboard: [wptsync downstream])
Sync web-platform-tests PR 27001 into mozilla-central (this bug is closed when the sync is complete).
PR: https://github.com/web-platform-tests/wpt/pull/27001
Details from upstream follow.
b'Mason Freed <masonfreed@chromium.org>' wrote:
Reland "Fix several crashes when nested slots are removed from a ShadowRoot"
This is a reland of dbfed21f94881a2918223792ebde3476b8fd69e6
Original change's description:
Fix several crashes when nested slots are removed from a ShadowRoot
There were some crashes caused by nested slots (e.g.
\<slot>\<slot>Content\</slot>\</slot>) being removed from the tree.
These crashes were triggered by [1], which removed Shadow DOM v0, but
my theory is that due to the old V0 shadow root code, more calls were
being made to SlotAssignment::RecalcAssignment(). Now that the V0 code
is gone, it has exposed some missing code.Three issues are being fixed here:
- In Node::CheckSlotChange(), while removing the inner nested slot,
the parent_slot will have already been removed from the tree, so we
only need to call DidSlotChange if not. This used to be a DCHECK.- In TreeOrderedMap::Get(), while removing a key that previously had
more than one element, we may walk the tree and find that none of
the pre-existing elements are present. I.e. we're in a RemoveScope.
In this case, the key should be removed from the map.- In SlotAssignment::DidRemoveSlotInternal(), given #2 above, we can
just early-out if the slot isn't present in the map.I added a test for the crash conditions (variations on removing nested
named and unnamed slots), plus I added a test for the TreeOrderedMap
class, since there was none previously. The last test in the set
documents the new Get() behavior. I also tried to improve some of the
comments along the way. Finally, this CL rolls back a mitigation [2]
previously landed for this crash.[1] https://chromium-review.googlesource.com/c/chromium/src/+/2586019
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2595967Bug: 1159328, 1159727
Change-Id: I47fbf33b2313b9ae2efe229443af6e8c9a1920a9
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2597040
Commit-Queue: Mason Freed \<masonfreed@chromium.org>
Reviewed-by: Yu Han \<yuzhehan@chromium.org>
Reviewed-by: Joey Arhar \<jarhar@chromium.org>
Auto-Submit: Mason Freed \<masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838974}Bug: 1159328
Bug: 1159727
Change-Id: I0025c0f00d6b3876de8f40a60fdc34f726ddc85cReviewed-on: https://chromium-review.googlesource.com/2601051
WPT-Export-Revision: abb8b3ee796dba93fbdfc6a7b8191c4e16009bec
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
Assignee | ||
Comment 2•4 years ago
|
||
Assignee | ||
Comment 3•4 years ago
|
||
CI Results
Ran 14 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI
Total 1 tests
Status Summary
Firefox
PASS: 1
Chrome
PASS: 1
Safari
PASS: 1
Links
Comment 5•4 years ago
|
||
bugherder |
Description
•