[Webkit-unassigned] [Bug 222720] REGRESSION(r272900): Nullptr crash in ComposedTreeIterator::traverseNextInShadowTree() via ShadowRoot::hostChildElementDidChange

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=222720

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #20 from Ryosuke Niwa <rniwa at webkit.org> ---
> It collects 3 children to be
> removed: #text 0x7f6c29a30010, #text 0x7f6c280f7b00 and summary (shadow
> root) 0x7f6c280f7b60. When dispatchChildRemovalEvents() is called for the
> summary element, the DOMNodeRemoved is dispatched and on_details_click() is
> called again, prepending a new node. This new node inserted while deleting
> children is he one becoming nullptr in the assigned nodes list. At this
> point the list of assigned nodes for slot 0x7f6c280f7a80 contains the three
> elements. 
> Now the actual child removal happens, that includes the newly added node
> that is the first one to be removed. The main difference is that the new
> node added is destroyed when removed, while the others are kept alive while
> a bit longer. When the summary is removed
> ShadowRoot::hostChildElementDidChange() is called and the assigned node list
> is: nullptr, #text 0x7f6c29a30010, #text 0x7f6c280f7b00. When
> removeAllChildrenWithScriptAssertion finishes the rest of the nodes are
> destroyed too.
> 
> So, I think the fix is to keep the newly node alive too until
> removeAllChildrenWithScriptAssertion() finishes. I wonder if adding new
> nodes while removing children is allowed, though.

I see, that's a good analysis. Adding a node during dispatchChildRemovalEvents is totally okay / expected. That's the whole point of that section of the code in removeAllChildrenWithScriptAssertion.

In this case, we'd be calling SlotAssignment::willRemoveAllChildren before any element is being removed (i.e. before ShadowRoot::hostChildElementDidChange() is called).

SlotAssignment::assignedNodesForSlot will set m_willBeRemovingAllChildren to true. So perhaps we could delete nullptr entries in SlotAssignment::Slot::assignedNodes in SlotAssignment::assignedNodesForSlot when that flag is set to true.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210316/0dd55247/attachment.htm>