Skip to content
Permalink
Browse files
REGRESSION: Occasional assertion failure in JSEventListener::ensureJS…
…Function via HTMLSlotElement::dispatchSlotChangeEvent()

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

Reviewed by Wenson Hsieh.

The bug was caused by ManualSlotAssignment enqueueing slotchange event on a shadow tree that has already started getting
destructed unlike NamedSlotAssignment which avoided this by checking ShadowRoot::shouldFireSlotchangeEvent().
Fixed the bug by checking ShadowRoot::shouldFireSlotchangeEvent() in ManualSlotAssignment.

Unfortunately no new tests since we don't have a reliable way of reproducing this assertion failure.

* Source/WebCore/dom/SlotAssignment.cpp:
(WebCore::ManualSlotAssignment::addSlotElementByName):
(WebCore::ManualSlotAssignment::removeSlotElementByName):
(WebCore::ManualSlotAssignment::slotManualAssignmentDidChange):
(WebCore::ManualSlotAssignment::didRemoveManuallyAssignedNode):
(WebCore::ManualSlotAssignment::willRemoveAssignedNode):

Canonical link: https://commits.webkit.org/253365@main
  • Loading branch information
rniwa committed Aug 12, 2022
1 parent 3ae8891 commit 0069c1e3e527d4ec271f370ed1799c0a346eb5d9
Showing 1 changed file with 14 additions and 2 deletions.
@@ -463,6 +463,10 @@ void ManualSlotAssignment::addSlotElementByName(const AtomString&, HTMLSlotEleme
shadowRoot.host()->setHasShadowRootContainingSlots(true);
++m_slotElementCount;
++m_slottableVersion;

if (!shadowRoot.shouldFireSlotchangeEvent())
return;

if (assignedNodesForSlot(slot, shadowRoot))
slot.enqueueSlotChangeEvent();
}
@@ -472,6 +476,10 @@ void ManualSlotAssignment::removeSlotElementByName(const AtomString&, HTMLSlotEl
RELEASE_ASSERT(m_slotElementCount);
--m_slotElementCount;
++m_slottableVersion;

if (!shadowRoot.shouldFireSlotchangeEvent())
return;

for (auto& node : slot.manuallyAssignedNodes()) {
if (node && node->parentNode() == shadowRoot.host()) {
slot.enqueueSlotChangeEvent();
@@ -517,6 +525,9 @@ void ManualSlotAssignment::slotManualAssignmentDidChange(HTMLSlotElement& slot,
RenderTreeUpdater::tearDownRenderersAfterSlotChange(*shadowRoot.host());
shadowRoot.host()->invalidateStyleForSubtree();

if (!shadowRoot.shouldFireSlotchangeEvent())
return;

if (affectedSlots.isEmpty()) {
scheduleSlotChangeEventIfNeeded();
return;
@@ -536,7 +547,8 @@ void ManualSlotAssignment::didRemoveManuallyAssignedNode(HTMLSlotElement& slot,
++m_slottableVersion;
RenderTreeUpdater::tearDownRenderersAfterSlotChange(*shadowRoot.host());
shadowRoot.host()->invalidateStyleForSubtree();
slot.enqueueSlotChangeEvent();
if (shadowRoot.shouldFireSlotchangeEvent())
slot.enqueueSlotChangeEvent();
}

void ManualSlotAssignment::slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&)
@@ -556,7 +568,7 @@ void ManualSlotAssignment::hostChildElementDidChangeSlotAttribute(Element&, cons
void ManualSlotAssignment::willRemoveAssignedNode(const Node& node, ShadowRoot& shadowRoot)
{
++m_slottableVersion;
if (RefPtr slot = node.assignedSlot(); slot && slot->containingShadowRoot() == &shadowRoot)
if (RefPtr slot = node.assignedSlot(); slot && slot->containingShadowRoot() == &shadowRoot && shadowRoot.shouldFireSlotchangeEvent())
slot->enqueueSlotChangeEvent();
}

0 comments on commit 0069c1e

Please sign in to comment.