New issue
Advanced search Search tips

Issue 849599 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 847727



Sign in to add a comment

Detaching a node causes a stack-overflow; a *cycle* would be possible.

Project Member Reported by hayato@chromium.org, Jun 5 2018

Issue description

See the attached test case. The points are:

1) Prepare the following tree of trees:

a
└── host
    ├──/shadow-root
    │   └── slot
    └── child

The child is assigned to the slot.

2) Then, mutate DOM carefully so that the followings are satisfied:

- child doesn't need attach
- host and slot need attach
- slot's assigned_nodes still includes child
- child is a shadow-including ancestor of the slot

a
└── child
    └── host
        └──/shadow-root
            └── slot

3) Then, detach a node. e.g. a.remove()

It would cause a cycle in detaching; a -> child -> host -> shadow-root -> slot -> child -> host -> shadow-root -> child -> ....


The other points are:
- This is not a recent regression. It looks this issue has been there, at least, since 2018 Oct.
- This would happen regardless of incremental shadow dom or not.


 
my-repro.html
485 bytes View Download
Blocking: 847727
> since 2018 Oct.

Correction: since 2017 Oct. I've not tried to repro before that yet.


Comment 3 by kochi@chromium.org, Jun 6 2018

Used bisect-builds.py and the regression range was narrowed down to,
https://chromium.googlesource.com/chromium/src/+log/c9228e4ef8d877ad45075ea28087a1a12a2fceea..27b6c9581a94a58d6342a3a425edcc4afa7a09b5 (only 2 CLs)

and the culprit is
https://chromium.googlesource.com/chromium/src/+/27b6c9581a94a58d6342a3a425edcc4afa7a09b5
Rewrite Shadow DOM v1 distribution engine on the top of a new slotchange concept

So the problem existed since the major rewrite of v1 distribution.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b04a57e793d228aca724525a50d2c5cb7572e13

commit 4b04a57e793d228aca724525a50d2c5cb7572e13
Author: Hayato Ito <hayato@chromium.org>
Date: Thu Jun 07 17:33:53 2018

Clear slot's assigned nodes if the slot assignment is dirty when the slot is removed

If we don't clear slot's assigned nodes when detaching a slot, if the slot assignment
recalc flag is dirty (the actual condition is more complex, see the code),
*cycle* would happen in detaching (after attaching a slot again) because a series of DOM mutations
can create a tree of trees as such:

- one of the slot's formerly assigned node, call it |A|, is a shadow-including ancestor of
  the slot.
- |A| is already attached.

In this case, detaching the slot will try to lazy-reattach |A|, and lazy-reattaching |A| doesn't
return early (because |A| doesn't need Attach). As a result, detaching the slot *cycles* and causes
an infinite loop (... -> |A| -> .. -> host -> shadow root -> .. -> slot -> |A| -> ...).

See the test (or  bug 849599 ) for a concrete example.

Bug:  849599 
Change-Id: I1c1ddb06ca9777af0052260aa721c2438da3c62b
Reviewed-on: https://chromium-review.googlesource.com/1090420
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565323}
[add] https://crrev.com/4b04a57e793d228aca724525a50d2c5cb7572e13/third_party/WebKit/LayoutTests/shadow-dom/crashes/cyclic-detach-crash.html
[modify] https://crrev.com/4b04a57e793d228aca724525a50d2c5cb7572e13/third_party/blink/renderer/core/html/html_slot_element.cc

Status: Fixed (was: Assigned)

Sign in to add a comment