We have a rather elaborate logic to lazily resolve slot elements when there are multiple ones of the same name. Get rid of this code since that's a very rare case, and it doesn't really benefit perf much since the only case we'd get faster is when scripts inserts & removes slot elements repeatedly.
Actually, it's even more stupid. It only benefits details element since whenever slot change may be needed, we'd go through the slow path anyway. This is really stupid.
Created attachment 436980 [details] Patch
Comment on attachment 436980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436980&action=review > Source/WebCore/dom/SlotAssignment.cpp:-125 > - if (!m_slotAssignmentsIsValid) > - assignSlots(shadowRoot); FYI, this code was totally unnecessary. hasAssignedNodes calls it internally where it's needed In all other cases hasAssignedNodes is not called, we're just waiting CPU cycle traversing through nodes.
Ugh... I need to special case when we're in the middle of shadow root teardown.
Created attachment 437003 [details] Fixed the crash
Comment on attachment 437003 [details] Fixed the crash View in context: https://bugs.webkit.org/attachment.cgi?id=437003&action=review > Source/WebCore/ChangeLog:8 > + This patch makes the resolution of slot elements eager. Lazily resolution stopping making any sense stopping -> stopped? > Source/WebCore/ChangeLog:11 > + Right now, this lazy optimzation only applies when scripts repeatedly inserts & removes a slot element Typo: optimzation > Source/WebCore/ChangeLog:13 > + and there are assigned nodes to the slot. There is no reason to overcomplicate the slot assignemnt code typo: assignemnt
Will address these. Thanks! (In reply to Chris Dumez from comment #6) > Comment on attachment 437003 [details] > Fixed the crash > > View in context: > https://bugs.webkit.org/attachment.cgi?id=437003&action=review > > > Source/WebCore/ChangeLog:8 > > + This patch makes the resolution of slot elements eager. Lazily resolution stopping making any sense > > stopping -> stopped? > > > Source/WebCore/ChangeLog:11 > > + Right now, this lazy optimzation only applies when scripts repeatedly inserts & removes a slot element > > Typo: optimzation > > > Source/WebCore/ChangeLog:13 > > + and there are assigned nodes to the slot. There is no reason to overcomplicate the slot assignemnt code > > typo: assignemnt
Committed r281878 (241208@main): <https://commits.webkit.org/241208@main>
<rdar://problem/82646116>
(In reply to Ryosuke Niwa from comment #3) > Comment on attachment 436980 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=436980&action=review > > > Source/WebCore/dom/SlotAssignment.cpp:-125 > > - if (!m_slotAssignmentsIsValid) > > - assignSlots(shadowRoot); > > FYI, this code was totally unnecessary. hasAssignedNodes calls it internally > where it's needed > In all other cases hasAssignedNodes is not called, we're just waiting CPU > cycle traversing through nodes. This was totally wrong LOL.
Committed r282120 (241417@main): <https://commits.webkit.org/241417@main>