Virtualize SlotAssignment in preparation for imperative slot API

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

Reviewed by Antti Koivisto.

This patch introduces SlotAssignment abstract interface, which is implemented by NamedSlotAssignment
(or ImperativeSlotAssignment in the future). The interface has mostly pure virtual functions except
two that need to be inlined for NamedSlotAssignment to avoid a perf regression.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::childrenChanged):
* Source/WebCore/dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::addSlotElementByName):
* Source/WebCore/dom/ShadowRoot.h:
* Source/WebCore/dom/SlotAssignment.cpp:
(WebCore::slotNameFromAttributeValue):
(WebCore::slotNameFromSlotAttribute):
(WebCore::NamedSlotAssignment::findAssignedSlot): Renamed from SlotAssignment.
(WebCore::NamedSlotAssignment::hasAssignedNodes): Ditto.
(WebCore::NamedSlotAssignment::renameSlotElement): Ditto.
(WebCore::NamedSlotAssignment::addSlotElementByName): Ditto.
(WebCore::NamedSlotAssignment::removeSlotElementByName): Ditto.
(WebCore::NamedSlotAssignment::resolveSlotsAfterSlotMutation): Ditto.
(WebCore::NamedSlotAssignment::slotFallbackDidChange): Ditto.
(WebCore::NamedSlotAssignment::didChangeSlot): Ditto.
(WebCore::NamedSlotAssignment::didRemoveAllChildrenOfShadowHost): Added.
(WebCore::NamedSlotAssignment::didMutateTextNodesOfShadowHost): Added.
(WebCore::NamedSlotAssignment::hostChildElementDidChange): Renamed from SlotAssignment.
(WebCore::NamedSlotAssignment::hostChildElementDidChangeSlotAttribute): Ditto.
(WebCore::NamedSlotAssignment::assignedNodesForSlot): Ditto.
(WebCore::NamedSlotAssignment::willRemoveAssignedNode): Ditto.
(WebCore::NamedSlotAssignment::slotNameForHostChild const): Ditto.
(WebCore::NamedSlotAssignment::findFirstSlotElement): Ditto.
(WebCore::NamedSlotAssignment::assignSlots): Ditto.
(WebCore::NamedSlotAssignment::assignToSlot): Ditto.

* Source/WebCore/dom/SlotAssignment.h:
(WebCore::SlotAssignment): Added.
(WebCore::NamedSlotAssignment): Renamed from SlotAssignment.
(WebCore::NamedSlotAssignment::defaultSlotName): Ditto.
(WebCore::ShadowRoot::didRemoveAllChildrenOfShadowHost): Now call a virtual function.
(WebCore::ShadowRoot::didMutateTextNodesOfShadowHost): Ditto. Renamed from didChangeDefaultSlot.
(WebCore::ShadowRoot::hostChildElementDidChange): Added UNLIKELY.
(WebCore::ShadowRoot::hostChildElementDidChangeSlotAttribute): Now calls a single virtual function
instead of directly calling didChangeSlot and tearing the render tree.

* Source/WebCore/html/HTMLDetailsElement.cpp:
(WebCore::DetailsSlotAssignment::hostChildElementDidChange):
(WebCore::DetailsSlotAssignment::slotNameForHostChild const):

* Source/WebCore/html/HTMLSummaryElement.cpp:
(WebCore::HTMLSummaryElement::create):

Canonical link: https://commits.webkit.org/253266@main

Source/WebCore/dom/Element.cpp

@@ -2820,7 +2820,7 @@ void Element::childrenChanged(const ChildChange& change)
         case ChildChange::Type::TextInserted:
         case ChildChange::Type::TextRemoved:
         case ChildChange::Type::TextChanged:
-            shadowRoot->didChangeDefaultSlot();
+            shadowRoot->didMutateTextNodesOfShadowHost();
             break;
         case ChildChange::Type::NonContentsChildInserted:
         case ChildChange::Type::NonContentsChildRemoved:

Source/WebCore/dom/ShadowRoot.cpp

@@ -250,7 +250,7 @@ void ShadowRoot::addSlotElementByName(const AtomString& name, HTMLSlotElement& s
 {
     ASSERT(&slot.rootNode() == this);
     if (!m_slotAssignment)
-        m_slotAssignment = makeUnique<SlotAssignment>();
+        m_slotAssignment = makeUnique<NamedSlotAssignment>();
 
     return m_slotAssignment->addSlotElementByName(name, slot, *this);
 }

Source/WebCore/dom/ShadowRoot.h

@@ -101,7 +101,7 @@ class ShadowRoot final : public DocumentFragment, public TreeScope {
     void willRemoveAssignedNode(const Node&);
 
     void didRemoveAllChildrenOfShadowHost();
-    void didChangeDefaultSlot();
+    void didMutateTextNodesOfShadowHost();
     void hostChildElementDidChange(const Element&);
     void hostChildElementDidChangeSlotAttribute(Element&, const AtomString& oldValue, const AtomString& newValue);
 

Source/WebCore/dom/SlotAssignment.cpp

@@ -28,22 +28,34 @@
 
 #include "ElementInlines.h"
 #include "HTMLSlotElement.h"
+#include "RenderTreeUpdater.h"
 #include "ShadowRoot.h"
 #include "TypedElementDescendantIterator.h"
 
 namespace WebCore {
 
 using namespace HTMLNames;
 
+struct SameSizeAsNamedSlotAssignment {
+    virtual ~SameSizeAsNamedSlotAssignment() = default;
+    uint32_t values[4];
+    HashMap<void*, void*> pointer;
+#if ASSERT_ENABLED
+    HashSet<void*> hashSet;
+#endif
+};
+
+static_assert(sizeof(NamedSlotAssignment) == sizeof(SameSizeAsNamedSlotAssignment), "NamedSlotAssignment should remain small");
+
 static const AtomString& slotNameFromAttributeValue(const AtomString& value)
 {
-    return value == nullAtom() ? SlotAssignment::defaultSlotName() : value;
+    return value == nullAtom() ? NamedSlotAssignment::defaultSlotName() : value;
 }
 
 static const AtomString& slotNameFromSlotAttribute(const Node& child)
 {
     if (is<Text>(child))
-        return SlotAssignment::defaultSlotName();
+        return NamedSlotAssignment::defaultSlotName();
 
     return slotNameFromAttributeValue(downcast<Element>(child).attributeWithoutSynchronization(slotAttr));
 }
@@ -71,11 +83,11 @@ static HTMLSlotElement* nextSlotElementSkippingSubtree(ContainerNode& startingNo
     return downcast<HTMLSlotElement>(node);
 }
 
-SlotAssignment::SlotAssignment() = default;
+NamedSlotAssignment::NamedSlotAssignment() = default;
 
-SlotAssignment::~SlotAssignment() = default;
+NamedSlotAssignment::~NamedSlotAssignment() = default;
 
-HTMLSlotElement* SlotAssignment::findAssignedSlot(const Node& node)
+HTMLSlotElement* NamedSlotAssignment::findAssignedSlot(const Node& node)
 {
     if (!is<Text>(node) && !is<Element>(node))
         return nullptr;
@@ -87,14 +99,14 @@ HTMLSlotElement* SlotAssignment::findAssignedSlot(const Node& node)
     return findFirstSlotElement(*slot);
 }
 
-inline bool SlotAssignment::hasAssignedNodes(ShadowRoot& shadowRoot, Slot& slot)
+inline bool NamedSlotAssignment::hasAssignedNodes(ShadowRoot& shadowRoot, Slot& slot)
 {
     if (!m_slotAssignmentsIsValid)
         assignSlots(shadowRoot);
     return !slot.assignedNodes.isEmpty();
 }
 
-void SlotAssignment::renameSlotElement(HTMLSlotElement& slotElement, const AtomString& oldName, const AtomString& newName, ShadowRoot& shadowRoot)
+void NamedSlotAssignment::renameSlotElement(HTMLSlotElement& slotElement, const AtomString& oldName, const AtomString& newName, ShadowRoot& shadowRoot)
 {
     ASSERT(m_slotElementsForConsistencyCheck.contains(&slotElement));
 
@@ -104,7 +116,7 @@ void SlotAssignment::renameSlotElement(HTMLSlotElement& slotElement, const AtomS
     addSlotElementByName(newName, slotElement, shadowRoot);
 }
 
-void SlotAssignment::addSlotElementByName(const AtomString& name, HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
+void NamedSlotAssignment::addSlotElementByName(const AtomString& name, HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
 #if ASSERT_ENABLED
     ASSERT(!m_slotElementsForConsistencyCheck.contains(&slotElement));
@@ -139,7 +151,7 @@ void SlotAssignment::addSlotElementByName(const AtomString& name, HTMLSlotElemen
     resolveSlotsAfterSlotMutation(shadowRoot, SlotMutationType::Insertion);
 }
 
-void SlotAssignment::removeSlotElementByName(const AtomString& name, HTMLSlotElement& slotElement, ContainerNode* oldParentOfRemovedTreeForRemoval, ShadowRoot& shadowRoot)
+void NamedSlotAssignment::removeSlotElementByName(const AtomString& name, HTMLSlotElement& slotElement, ContainerNode* oldParentOfRemovedTreeForRemoval, ShadowRoot& shadowRoot)
 {
 #if ASSERT_ENABLED
     ASSERT(m_slotElementsForConsistencyCheck.contains(&slotElement));
@@ -187,7 +199,7 @@ void SlotAssignment::removeSlotElementByName(const AtomString& name, HTMLSlotEle
     }
 }
 
-void SlotAssignment::resolveSlotsAfterSlotMutation(ShadowRoot& shadowRoot, SlotMutationType mutationType, ContainerNode* subtreeToSkip)
+void NamedSlotAssignment::resolveSlotsAfterSlotMutation(ShadowRoot& shadowRoot, SlotMutationType mutationType, ContainerNode* subtreeToSkip)
 {
     if (m_slotResolutionVersion == m_slotMutationVersion)
         return;
@@ -259,7 +271,7 @@ void SlotAssignment::resolveSlotsAfterSlotMutation(ShadowRoot& shadowRoot, SlotM
     }
 }
 
-void SlotAssignment::slotFallbackDidChange(HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
+void NamedSlotAssignment::slotFallbackDidChange(HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
     if (shadowRoot.mode() == ShadowRootMode::UserAgent)
         return;
@@ -269,7 +281,7 @@ void SlotAssignment::slotFallbackDidChange(HTMLSlotElement& slotElement, ShadowR
         slotElement.enqueueSlotChangeEvent();
 }
 
-void SlotAssignment::didChangeSlot(const AtomString& slotAttrValue, ShadowRoot& shadowRoot)
+void NamedSlotAssignment::didChangeSlot(const AtomString& slotAttrValue, ShadowRoot& shadowRoot)
 {
     auto& slotName = slotNameFromAttributeValue(slotAttrValue);
     auto* slot = m_slots.get(slotName);
@@ -290,12 +302,29 @@ void SlotAssignment::didChangeSlot(const AtomString& slotAttrValue, ShadowRoot&
         slotElement->enqueueSlotChangeEvent();
 }
 
-void SlotAssignment::hostChildElementDidChange(const Element& childElement, ShadowRoot& shadowRoot)
+void NamedSlotAssignment::didRemoveAllChildrenOfShadowHost(ShadowRoot& shadowRoot)
+{
+    didChangeSlot(nullAtom(), shadowRoot); // FIXME: This is incorrect when there were no elements or text nodes removed.
+}
+
+void NamedSlotAssignment::didMutateTextNodesOfShadowHost(ShadowRoot& shadowRoot)
+{
+    didChangeSlot(nullAtom(), shadowRoot);
+}
+
+void NamedSlotAssignment::hostChildElementDidChange(const Element& childElement, ShadowRoot& shadowRoot)
 {
     didChangeSlot(childElement.attributeWithoutSynchronization(slotAttr), shadowRoot);
 }
 
-const Vector<WeakPtr<Node>>* SlotAssignment::assignedNodesForSlot(const HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
+void NamedSlotAssignment::hostChildElementDidChangeSlotAttribute(Element& element, const AtomString& oldValue, const AtomString& newValue, ShadowRoot& shadowRoot)
+{
+    didChangeSlot(oldValue, shadowRoot);
+    didChangeSlot(newValue, shadowRoot);
+    RenderTreeUpdater::tearDownRenderers(element);
+}
+
+const Vector<WeakPtr<Node>>* NamedSlotAssignment::assignedNodesForSlot(const HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
     ASSERT(slotElement.containingShadowRoot() == &shadowRoot);
     const AtomString& slotName = slotNameFromAttributeValue(slotElement.attributeWithoutSynchronization(nameAttr));
@@ -319,7 +348,7 @@ const Vector<WeakPtr<Node>>* SlotAssignment::assignedNodesForSlot(const HTMLSlot
     return &slot->assignedNodes;
 }
 
-void SlotAssignment::willRemoveAssignedNode(const Node& node)
+void NamedSlotAssignment::willRemoveAssignedNode(const Node& node)
 {
     if (!m_slotAssignmentsIsValid)
         return;
@@ -336,19 +365,19 @@ void SlotAssignment::willRemoveAssignedNode(const Node& node)
     });
 }
 
-const AtomString& SlotAssignment::slotNameForHostChild(const Node& child) const
+const AtomString& NamedSlotAssignment::slotNameForHostChild(const Node& child) const
 {
     return slotNameFromSlotAttribute(child);
 }
 
-HTMLSlotElement* SlotAssignment::findFirstSlotElement(Slot& slot)
+HTMLSlotElement* NamedSlotAssignment::findFirstSlotElement(Slot& slot)
 {
     ASSERT(!slot.element || m_slotElementsForConsistencyCheck.contains(slot.element.get()));
 
     return slot.element.get();
 }
 
-void SlotAssignment::assignSlots(ShadowRoot& shadowRoot)
+void NamedSlotAssignment::assignSlots(ShadowRoot& shadowRoot)
 {
     ASSERT(!m_slotAssignmentsIsValid);
     m_slotAssignmentsIsValid = true;
@@ -369,7 +398,7 @@ void SlotAssignment::assignSlots(ShadowRoot& shadowRoot)
         entry.value->assignedNodes.shrinkToFit();
 }
 
-void SlotAssignment::assignToSlot(Node& child, const AtomString& slotName)
+void NamedSlotAssignment::assignToSlot(Node& child, const AtomString& slotName)
 {
     ASSERT(!slotName.isNull());
     if (slotName == defaultSlotName()) {

Source/WebCore/dom/SlotAssignment.h

@@ -25,7 +25,6 @@
 
 #pragma once
 
-#include "RenderTreeUpdater.h"
 #include "ShadowRoot.h"
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
@@ -43,29 +42,62 @@ class Node;
 class SlotAssignment {
     WTF_MAKE_NONCOPYABLE(SlotAssignment); WTF_MAKE_FAST_ALLOCATED;
 public:
-    SlotAssignment();
-    virtual ~SlotAssignment();
+    SlotAssignment() = default;
+    virtual ~SlotAssignment() = default;
 
-    static const AtomString& defaultSlotName() { return emptyAtom(); }
+    // These functions are only useful for NamedSlotAssignment but it's here to avoid virtual function calls in perf critical code paths.
+    void resolveSlotsBeforeNodeInsertionOrRemoval();
+    void willRemoveAllChildren();
 
-    HTMLSlotElement* findAssignedSlot(const Node&);
+    virtual HTMLSlotElement* findAssignedSlot(const Node&) = 0;
+    virtual const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&) = 0;
 
-    void renameSlotElement(HTMLSlotElement&, const AtomString& oldName, const AtomString& newName, ShadowRoot&);
-    void addSlotElementByName(const AtomString&, HTMLSlotElement&, ShadowRoot&);
-    void removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode* oldParentOfRemovedTreeForRemoval, ShadowRoot&);
-    void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&);
+    virtual void renameSlotElement(HTMLSlotElement&, const AtomString& oldName, const AtomString& newName, ShadowRoot&) = 0;
+    virtual void addSlotElementByName(const AtomString&, HTMLSlotElement&, ShadowRoot&) = 0;
+    virtual void removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode* oldParentOfRemovedTreeForRemoval, ShadowRoot&) = 0;
+    virtual void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&) = 0;
 
-    void resolveSlotsBeforeNodeInsertionOrRemoval();
-    void willRemoveAllChildren();
+    virtual void hostChildElementDidChange(const Element&, ShadowRoot&) = 0;
+    virtual void hostChildElementDidChangeSlotAttribute(Element&, const AtomString& oldValue, const AtomString& newValue, ShadowRoot&) = 0;
 
-    void didChangeSlot(const AtomString&, ShadowRoot&);
+    virtual void willRemoveAssignedNode(const Node&) = 0;
+    virtual void didRemoveAllChildrenOfShadowHost(ShadowRoot&) = 0;
+    virtual void didMutateTextNodesOfShadowHost(ShadowRoot&) = 0;
 
-    const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&);
-    void willRemoveAssignedNode(const Node&);
+protected:
+    // These flags are used by NamedSlotAssignment but it's here to avoid virtual function calls in perf critical code paths.
+    bool m_slotAssignmentsIsValid { false };
+    bool m_willBeRemovingAllChildren { false };
+    unsigned m_slotMutationVersion { 0 };
+};
+
+class NamedSlotAssignment : public SlotAssignment {
+    WTF_MAKE_NONCOPYABLE(NamedSlotAssignment); WTF_MAKE_FAST_ALLOCATED;
+public:
+    NamedSlotAssignment();
+    virtual ~NamedSlotAssignment();
+
+    static const AtomString& defaultSlotName() { return emptyAtom(); }
 
-    virtual void hostChildElementDidChange(const Element&, ShadowRoot&);
+protected:
+    void didChangeSlot(const AtomString&, ShadowRoot&);
 
 private:
+    HTMLSlotElement* findAssignedSlot(const Node&) final;
+
+    void renameSlotElement(HTMLSlotElement&, const AtomString& oldName, const AtomString& newName, ShadowRoot&) final;
+    void addSlotElementByName(const AtomString&, HTMLSlotElement&, ShadowRoot&) final;
+    void removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode* oldParentOfRemovedTreeForRemoval, ShadowRoot&) final;
+    void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&) final;
+
+    const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&) final;
+    void willRemoveAssignedNode(const Node&) final;
+
+    void didRemoveAllChildrenOfShadowHost(ShadowRoot&) final;
+    void didMutateTextNodesOfShadowHost(ShadowRoot&) final;
+    void hostChildElementDidChange(const Element&, ShadowRoot&) override;
+    void hostChildElementDidChangeSlotAttribute(Element&, const AtomString& oldValue, const AtomString& newValue, ShadowRoot&) final;
+
     struct Slot {
         WTF_MAKE_FAST_ALLOCATED;
     public:
@@ -93,17 +125,14 @@ class SlotAssignment {
     void assignSlots(ShadowRoot&);
     void assignToSlot(Node& child, const AtomString& slotName);
 
+    unsigned m_slotResolutionVersion { 0 };
+    unsigned m_slotElementCount { 0 };
+
     HashMap<AtomString, std::unique_ptr<Slot>> m_slots;
 
 #if ASSERT_ENABLED
     HashSet<HTMLSlotElement*> m_slotElementsForConsistencyCheck;
 #endif
-
-    bool m_slotAssignmentsIsValid { false };
-    bool m_willBeRemovingAllChildren { false };
-    unsigned m_slotMutationVersion { 0 };
-    unsigned m_slotResolutionVersion { 0 };
-    unsigned m_slotElementCount { 0 };
 };
 
 inline void SlotAssignment::resolveSlotsBeforeNodeInsertionOrRemoval()
@@ -132,29 +161,27 @@ inline void ShadowRoot::willRemoveAllChildren(ContainerNode&)
 
 inline void ShadowRoot::didRemoveAllChildrenOfShadowHost()
 {
-    if (m_slotAssignment) // FIXME: This is incorrect when there were no elements or text nodes removed.
-        m_slotAssignment->didChangeSlot(nullAtom(), *this);
+    if (UNLIKELY(m_slotAssignment))
+        m_slotAssignment->didRemoveAllChildrenOfShadowHost(*this);
 }
 
-inline void ShadowRoot::didChangeDefaultSlot()
+inline void ShadowRoot::didMutateTextNodesOfShadowHost()
 {
-    if (m_slotAssignment)
-        m_slotAssignment->didChangeSlot(nullAtom(), *this);
+    if (UNLIKELY(m_slotAssignment))
+        m_slotAssignment->didMutateTextNodesOfShadowHost(*this);
 }
 
 inline void ShadowRoot::hostChildElementDidChange(const Element& childElement)
 {
-    if (m_slotAssignment)
+    if (UNLIKELY(m_slotAssignment))
         m_slotAssignment->hostChildElementDidChange(childElement, *this);
 }
 
 inline void ShadowRoot::hostChildElementDidChangeSlotAttribute(Element& element, const AtomString& oldValue, const AtomString& newValue)
 {
     if (!m_slotAssignment)
         return;
-    m_slotAssignment->didChangeSlot(oldValue, *this);
-    m_slotAssignment->didChangeSlot(newValue, *this);
-    RenderTreeUpdater::tearDownRenderers(element);
+    m_slotAssignment->hostChildElementDidChangeSlotAttribute(element, oldValue, newValue, *this);
 }
 
 inline void ShadowRoot::willRemoveAssignedNode(const Node& node)

Source/WebCore/html/HTMLDetailsElement.cpp

@@ -51,7 +51,7 @@ static const AtomString& summarySlotName()
     return summarySlot;
 }
 
-class DetailsSlotAssignment final : public SlotAssignment {
+class DetailsSlotAssignment final : public NamedSlotAssignment {
 private:
     void hostChildElementDidChange(const Element&, ShadowRoot&) override;
     const AtomString& slotNameForHostChild(const Node&) const override;
@@ -64,7 +64,7 @@ void DetailsSlotAssignment::hostChildElementDidChange(const Element& childElemen
         // since we don't know the answer when this function is called inside Element::removedFrom.
         didChangeSlot(summarySlotName(), shadowRoot);
     } else
-        didChangeSlot(SlotAssignment::defaultSlotName(), shadowRoot);
+        didChangeSlot(NamedSlotAssignment::defaultSlotName(), shadowRoot);
 }
 
 const AtomString& DetailsSlotAssignment::slotNameForHostChild(const Node& child) const
@@ -78,7 +78,7 @@ const AtomString& DetailsSlotAssignment::slotNameForHostChild(const Node& child)
         if (&child == childrenOfType<HTMLSummaryElement>(details).first())
             return summarySlotName();
     }
-    return SlotAssignment::defaultSlotName();
+    return NamedSlotAssignment::defaultSlotName();
 }
 
 Ref<HTMLDetailsElement> HTMLDetailsElement::create(const QualifiedName& tagName, Document& document)