Skip to content

Commit

Permalink
Skip class modification if possible
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=274048
rdar://127948676

Reviewed by Ryosuke Niwa.

When changing class attribute via parserSetAttributes,

1. We may be able to skip SpaceSplitString creation. If the ElementData is shareable one,
   it is possible that it already has SpaceSplitString materialized before. We check the keyString,
   and if it is true, then skip the creation.
2. Cache / style invalidation is not necessary at this point yet since it is not attached to the tree,
   and never rendered before.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::attributeChanged):
(WebCore::Element::classAttributeChanged):
* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/SpaceSplitString.h:
(WebCore::SpaceSplitStringData::keyString const):
(WebCore::SpaceSplitString::keyString const):
* Source/WebCore/svg/SVGElement.cpp:
(WebCore::SVGElement::svgAttributeChanged):

Canonical link: https://commits.webkit.org/278733@main
  • Loading branch information
Constellation committed May 14, 2024
1 parent 75e0b11 commit bff7571
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 12 deletions.
30 changes: 20 additions & 10 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2100,14 +2100,14 @@ void Element::notifyAttributeChanged(const QualifiedName& name, const AtomString
}
}

void Element::attributeChanged(const QualifiedName& name, const AtomString& oldValue, const AtomString& newValue, AttributeModificationReason)
void Element::attributeChanged(const QualifiedName& name, const AtomString& oldValue, const AtomString& newValue, AttributeModificationReason reason)
{
if (oldValue == newValue)
return;

switch (name.nodeName()) {
case AttributeNames::classAttr:
classAttributeChanged(newValue);
classAttributeChanged(newValue, reason);
break;
case AttributeNames::idAttr: {
AtomString oldId = elementData()->idForStyleResolution();
Expand Down Expand Up @@ -2298,24 +2298,34 @@ void Element::setElementsArrayAttribute(const QualifiedName& attributeName, std:
}
}

void Element::classAttributeChanged(const AtomString& newClassString)
void Element::classAttributeChanged(const AtomString& newClassString, AttributeModificationReason reason)
{
// Note: We'll need ElementData, but it doesn't have to be UniqueElementData.
if (!elementData())
ensureUniqueElementData();

{
if (hasRareData()) {
if (auto* classList = elementRareData()->classList())
classList->associatedAttributeValueChanged();
}

if (reason == AttributeModificationReason::Parser) {
// If ElementData is ShareableElementData created in parserSetAttributes,
// it is possible that SpaceSplitString is already created and set.
// We also do not need to invalidate caches / styles since it is not inserted to the tree yet.
if (elementData()->classNames().keyString() == newClassString)
return;
auto shouldFoldCase = document().inQuirksMode() ? SpaceSplitString::ShouldFoldCase::Yes : SpaceSplitString::ShouldFoldCase::No;
SpaceSplitString newClassNames(newClassString, shouldFoldCase);
Style::ClassChangeInvalidation styleInvalidation(*this, elementData()->classNames(), newClassNames);
document().invalidateQuerySelectorAllResultsForClassAttributeChange(*this, elementData()->classNames(), newClassNames);
elementData()->setClassNames(WTFMove(newClassNames));
return;
}

if (hasRareData()) {
if (auto* classList = elementRareData()->classList())
classList->associatedAttributeValueChanged();
}
auto shouldFoldCase = document().inQuirksMode() ? SpaceSplitString::ShouldFoldCase::Yes : SpaceSplitString::ShouldFoldCase::No;
SpaceSplitString newClassNames(newClassString, shouldFoldCase);
Style::ClassChangeInvalidation styleInvalidation(*this, elementData()->classNames(), newClassNames);
document().invalidateQuerySelectorAllResultsForClassAttributeChange(*this, elementData()->classNames(), newClassNames);
elementData()->setClassNames(WTFMove(newClassNames));
}

void Element::partAttributeChanged(const AtomString& newValue)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/dom/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ class Element : public ContainerNode {

void setTabIndexExplicitly(std::optional<int>);

void classAttributeChanged(const AtomString& newClassString);
void classAttributeChanged(const AtomString& newClassString, AttributeModificationReason);
void partAttributeChanged(const AtomString& newValue);

void addShadowRoot(Ref<ShadowRoot>&&);
Expand Down
9 changes: 9 additions & 0 deletions Source/WebCore/dom/SpaceSplitString.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class SpaceSplitStringData {
m_refCount = tempRefCount;
}

const AtomString& keyString() const { return m_keyString; }

static ptrdiff_t tokensMemoryOffset() { return sizeof(SpaceSplitStringData); }

private:
Expand Down Expand Up @@ -109,6 +111,13 @@ class SpaceSplitString {
enum class ShouldFoldCase : bool { No, Yes };
SpaceSplitString(const AtomString&, ShouldFoldCase);

const AtomString& keyString() const
{
if (m_data)
return m_data->keyString();
return nullAtom();
}

friend bool operator==(const SpaceSplitString&, const SpaceSplitString&) = default;
void set(const AtomString&, ShouldFoldCase);
void clear() { m_data = nullptr; }
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/svg/SVGElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ void SVGElement::svgAttributeChanged(const QualifiedName& attrName)
}

if (attrName == HTMLNames::classAttr) {
classAttributeChanged(className());
classAttributeChanged(className(), AttributeModificationReason::Directly);
invalidateInstances();
return;
}
Expand Down

0 comments on commit bff7571

Please sign in to comment.