commit 1c5816e523d00ad3854082245a52f09f7bdb0ff5
author Findit <findit-for-me@appspot.gserviceaccount.com> Wed Dec 23 02:57:59 2020
committer Blink WPT Bot <blink-w3c-test-autoroller@chromium.org> Wed Dec 23 03:18:47 2020
tree c8214a8a372998ebf493360204c2bf02772e9340
parent e5a6179c53234a33249f66013a8dd3ffeab07bba

Revert "Fix several crashes when nested slots are removed from a ShadowRoot"

This reverts commit dbfed21f94881a2918223792ebde3476b8fd69e6.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 838974 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2RiZmVkMjFmOTQ4ODFhMjkxODIyMzc5MmViZGUzNDc2YjhmZDY5ZTYM

Sample Failed Build: https://ci.chromium.org/b/8860163671563368608

Sample Failed Step: webkit_unit_tests

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:
>  1. 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.
>  2. 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.
>  3. 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/+/2595967
>
> Bug: 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}

Change-Id: I97202c545f74df090124e82775fe79ce978d3d63
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1159328, 1159727
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601758
Cr-Commit-Position: refs/heads/master@{#839038}