[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

--- Comment #22 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Ryosuke Niwa from comment #20)
> > 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).

Not really, because replaceAllChildrenWithNewText() is called on the details element that is not shadow root.

> 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/e9dc99bc/attachment.htm>