Add iterator to SpaceSplitString

https://bugs.webkit.org/show_bug.cgi?id=273988 Reviewed by Ryosuke Niwa. Add iterator to SpaceSplitString. This results in nicer code and may be a bit more efficient than using operator[] which does a bounds check. This also allows us to use more efficient functions to construct or populate Vectors (such as WTF:::map()). * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::addRelation): * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::elementsFromAttribute const): * Source/WebCore/css/SelectorChecker.cpp: (WebCore::SelectorChecker::checkOne const): * Source/WebCore/css/SelectorFilter.cpp: (WebCore::SelectorFilter::collectElementIdentifierHashes): * Source/WebCore/dom/CustomElementDefaultARIA.cpp: (WebCore::CustomElementDefaultARIA::elementsForAttribute const): * Source/WebCore/dom/Element.cpp: (WebCore::Element::getElementsArrayAttribute const): * Source/WebCore/dom/SpaceSplitString.h: (WebCore::SpaceSplitStringData::begin const): (WebCore::SpaceSplitStringData::end const): (WebCore::SpaceSplitStringData::begin): (WebCore::SpaceSplitStringData::end): (WebCore::SpaceSplitStringData::tokenArrayStart const): (WebCore::SpaceSplitString::begin const): (WebCore::SpaceSplitString::end const): (WebCore::SpaceSplitString::begin): (WebCore::SpaceSplitString::end): * Source/WebCore/html/HTMLAnchorElement.cpp: (WebCore::HTMLAnchorElement::sendPings): * Source/WebCore/page/EventHandler.cpp: (WebCore::findDropZone): * Source/WebCore/style/ClassChangeInvalidation.cpp: (WebCore::Style::collectClasses): (WebCore::Style::computeClassChanges): * Source/WebCore/style/ElementRuleCollector.cpp: (WebCore::Style::ElementRuleCollector::collectMatchingRules): * Source/WebCore/style/PseudoClassChangeInvalidation.cpp: (WebCore::Style::makePseudoClassInvalidationKeys): * Source/WebCore/style/StyleSharingResolver.cpp: (WebCore::Style::SharingResolver::classNamesAffectedByRules const): Canonical link: https://commits.webkit.org/278601@main
WebKit · May 10, 2024 · c1735d0 · c1735d0
1 parent 4b58804
commit c1735d0
Show file tree

Hide file tree

Showing 13 changed files with 59 additions and 57 deletions.
diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -5118,8 +5118,8 @@ bool AXObjectCache::addRelation(Element& origin, const QualifiedName& attribute)
     }
 
     SpaceSplitString ids(value, SpaceSplitString::ShouldFoldCase::No);
-    for (size_t i = 0; i < ids.size(); ++i) {
-        RefPtr target = origin.treeScope().getElementById(ids[i]);
+    for (auto& id : ids) {
+        RefPtr target = origin.treeScope().getElementById(id);
         if (!target || target == &origin)
             continue;
 

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -4069,15 +4069,11 @@ Vector<Ref<Element>> AccessibilityObject::elementsFromAttribute(const QualifiedN
         return { };
     }
 
-    Vector<Ref<Element>> elements;
     auto& treeScope = element->treeScope();
-    SpaceSplitString spaceSplitString(idsString, SpaceSplitString::ShouldFoldCase::No);
-    size_t length = spaceSplitString.size();
-    for (size_t i = 0; i < length; ++i) {
-        if (RefPtr element = treeScope.getElementById(spaceSplitString[i]))
-            elements.append(element.releaseNonNull());
-    }
-    return elements;
+    SpaceSplitString ids(idsString, SpaceSplitString::ShouldFoldCase::No);
+    return WTF::compactMap(ids, [&](auto& id) {
+        return treeScope.getElementById(id);
+    });
 }
 
 #if PLATFORM(COCOA)

diff --git a/Source/WebCore/css/SelectorChecker.cpp b/Source/WebCore/css/SelectorChecker.cpp
@@ -1199,14 +1199,15 @@ bool SelectorChecker::checkOne(CheckingContext& checkingContext, const LocalCont
             return matchRecursively(checkingContext, subcontext, ignoredDynamicPseudo).match == Match::SelectorMatches;
         }
         case CSSSelector::PseudoElement::Part: {
-            auto translatePartNameToRuleScope = [&](AtomString partName) {
-                Vector<AtomString, 1> mappedNames { partName };
-
-                if (checkingContext.styleScopeOrdinal == Style::ScopeOrdinal::Element)
-                    return mappedNames;
+            auto appendTranslatedPartNameToRuleScope = [&](auto& translatedPartNames, AtomString partName) {
+                if (checkingContext.styleScopeOrdinal == Style::ScopeOrdinal::Element) {
+                    translatedPartNames.append(partName);
+                    return;
+                }
 
                 auto* ruleScopeHost = Style::hostForScopeOrdinal(*context.element, checkingContext.styleScopeOrdinal);
 
+                Vector<AtomString, 1> mappedNames { partName };
                 for (auto* shadowRoot = element.containingShadowRoot(); shadowRoot; shadowRoot = shadowRoot->host()->containingShadowRoot()) {
                     // Apply mappings up to the scope the rules are coming from.
                     if (shadowRoot->host() == ruleScopeHost)
@@ -1220,12 +1221,12 @@ bool SelectorChecker::checkOne(CheckingContext& checkingContext, const LocalCont
                     if (mappedNames.isEmpty())
                         break;
                 }
-                return mappedNames;
+                translatedPartNames.appendVector(mappedNames);
             };
 
             Vector<AtomString, 4> translatedPartNames;
-            for (unsigned i = 0; i < element.partNames().size(); ++i)
-                translatedPartNames.appendVector(translatePartNameToRuleScope(element.partNames()[i]));
+            for (auto& partName : element.partNames())
+                appendTranslatedPartNameToRuleScope(translatedPartNames, partName);
 
             for (auto& part : *selector.argumentList()) {
                 if (!translatedPartNames.contains(part))

diff --git a/Source/WebCore/css/SelectorFilter.cpp b/Source/WebCore/css/SelectorFilter.cpp
@@ -55,10 +55,9 @@ void SelectorFilter::collectElementIdentifierHashes(const Element& element, Vect
         identifierHashes.append(id.impl()->existingHash() * IdSalt);
 
     if (element.hasClass()) {
-        const SpaceSplitString& classNames = element.classNames();
-        size_t count = classNames.size();
-        for (size_t i = 0; i < count; ++i)
-            identifierHashes.append(classNames[i].impl()->existingHash() * ClassSalt);
+        identifierHashes.appendContainerWithMapping(element.classNames(), [](auto& className) {
+            return className.impl()->existingHash() * ClassSalt;
+        });
     }
 
     if (element.hasAttributesWithoutUpdate()) {

diff --git a/Source/WebCore/dom/CustomElementDefaultARIA.cpp b/Source/WebCore/dom/CustomElementDefaultARIA.cpp
@@ -115,20 +115,18 @@ Vector<Ref<Element>> CustomElementDefaultARIA::elementsForAttribute(const Elemen
     if (it == m_map.end())
         return result;
     std::visit(WTF::makeVisitor([&](const AtomString& stringValue) {
-        SpaceSplitString idList { stringValue, SpaceSplitString::ShouldFoldCase::No };
-        result.reserveCapacity(idList.size());
         if (thisElement.isInTreeScope()) {
-            for (unsigned i = 0; i < idList.size(); ++i) {
-                if (RefPtr element = thisElement.treeScope().getElementById(idList[i]))
-                    result.append(element.releaseNonNull());
-            }
+            SpaceSplitString idList { stringValue, SpaceSplitString::ShouldFoldCase::No };
+            result = WTF::compactMap(idList, [&](auto& id) {
+                return thisElement.treeScope().getElementById(id);
+            });
         }
     }, [&](const WeakPtr<Element, WeakPtrImplWithEventTargetData>& weakElementValue) {
         RefPtr element = weakElementValue.get();
         if (element && isElementVisible(*element, thisElement))
             result.append(element.releaseNonNull());
     }, [&](const Vector<WeakPtr<Element, WeakPtrImplWithEventTargetData>>& elements) {
-        result.reserveCapacity(elements.size());
+        result.reserveInitialCapacity(elements.size());
         for (auto& weakElement : elements) {
             if (RefPtr element = weakElement.get(); element && isElementVisible(*element, thisElement))
                 result.append(element.releaseNonNull());

diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
@@ -2269,12 +2269,9 @@ std::optional<Vector<Ref<Element>>> Element::getElementsArrayAttribute(const Qua
         return std::nullopt;
 
     SpaceSplitString ids(getAttribute(attr), SpaceSplitString::ShouldFoldCase::No);
-    Vector<Ref<Element>> elements;
-    for (unsigned i = 0; i < ids.size(); ++i) {
-        if (RefPtr element = treeScope().getElementById(ids[i]))
-            elements.append(element.releaseNonNull());
-    }
-    return elements;
+    return WTF::compactMap(ids, [&](auto& id) {
+        return treeScope().getElementById(id);
+    });
 }
 
 void Element::setElementsArrayAttribute(const QualifiedName& attributeName, std::optional<Vector<Ref<Element>>>&& elements)

diff --git a/Source/WebCore/dom/SpaceSplitString.h b/Source/WebCore/dom/SpaceSplitString.h
@@ -31,6 +31,11 @@ class SpaceSplitStringData {
 public:
     static RefPtr<SpaceSplitStringData> create(const AtomString&);
 
+    auto begin() const { return tokenArrayStart(); }
+    auto end() const { return tokenArrayStart() + size(); }
+    auto begin() { return tokenArrayStart(); }
+    auto end() { return tokenArrayStart() + size(); }
+
     bool contains(const AtomString& string)
     {
         const AtomString* data = tokenArrayStart();
@@ -90,6 +95,7 @@ class SpaceSplitStringData {
     static void destroy(SpaceSplitStringData*);
 
     AtomString* tokenArrayStart() { return reinterpret_cast<AtomString*>(this + 1); }
+    const AtomString* tokenArrayStart() const { return reinterpret_cast<const AtomString*>(this + 1); }
 
     AtomString m_keyString;
     unsigned m_refCount;
@@ -118,6 +124,11 @@ class SpaceSplitString {
         return (*m_data)[i];
     }
 
+    auto begin() const { return m_data ? m_data->begin() : nullptr; }
+    auto end() const { return m_data ? m_data->end() : nullptr; }
+    auto begin() { return m_data ? m_data->begin() : nullptr; }
+    auto end() { return m_data ? m_data->end() : nullptr; }
+
     static bool spaceSplitStringContainsValue(StringView spaceSplitString, StringView value, ShouldFoldCase);
 
 private:

diff --git a/Source/WebCore/html/HTMLAnchorElement.cpp b/Source/WebCore/html/HTMLAnchorElement.cpp
@@ -374,8 +374,8 @@ void HTMLAnchorElement::sendPings(const URL& destinationURL)
         return;
 
     SpaceSplitString pingURLs(pingValue, SpaceSplitString::ShouldFoldCase::No);
-    for (unsigned i = 0; i < pingURLs.size(); i++)
-        PingLoader::sendPing(*document().frame(), document().completeURL(pingURLs[i]), destinationURL);
+    for (auto& pingURL : pingURLs)
+        PingLoader::sendPing(*document().frame(), document().completeURL(pingURL), destinationURL);
 }
 
 #if USE(SYSTEM_PREVIEW)

diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
@@ -2509,12 +2509,12 @@ static bool findDropZone(Node& target, DataTransfer& dataTransfer)
         SpaceSplitString keywords(element->attributeWithoutSynchronization(webkitdropzoneAttr), SpaceSplitString::ShouldFoldCase::Yes);
         bool matched = false;
         std::optional<DragOperation> dragOperation;
-        for (unsigned i = 0, size = keywords.size(); i < size; ++i) {
-            if (auto operationFromKeyword = convertDropZoneOperationToDragOperation(keywords[i])) {
+        for (auto& keyword : keywords) {
+            if (auto operationFromKeyword = convertDropZoneOperationToDragOperation(keyword)) {
                 if (!dragOperation)
                     dragOperation = operationFromKeyword;
             } else
-                matched = matched || hasDropZoneType(dataTransfer, keywords[i].string());
+                matched = matched || hasDropZoneType(dataTransfer, keyword.string());
             if (matched && dragOperation)
                 break;
         }

diff --git a/Source/WebCore/style/ClassChangeInvalidation.cpp b/Source/WebCore/style/ClassChangeInvalidation.cpp
@@ -42,41 +42,41 @@ struct ClassChange {
     ClassChangeType type;
 };
 
-using ClassChangeVector = Vector<ClassChange, 4>;
+constexpr size_t classChangeVectorInlineCapacity = 4;
+using ClassChangeVector = Vector<ClassChange, classChangeVectorInlineCapacity>;
 
 static ClassChangeVector collectClasses(const SpaceSplitString& classes, ClassChangeType changeType)
 {
-    return ClassChangeVector(classes.size(), [&](size_t i) {
-        return ClassChange { classes[i].impl(), changeType };
+    return WTF::map<classChangeVectorInlineCapacity>(classes, [changeType](auto& className) {
+        return ClassChange { className.impl(), changeType };
     });
 }
 
 static ClassChangeVector computeClassChanges(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses)
 {
     unsigned oldSize = oldClasses.size();
-    unsigned newSize = newClasses.size();
 
     if (!oldSize)
         return collectClasses(newClasses, ClassChangeType::Add);
-    if (!newSize)
+    if (newClasses.isEmpty())
         return collectClasses(oldClasses, ClassChangeType::Remove);
 
     ClassChangeVector changedClasses;
 
     BitVector remainingClassBits;
     remainingClassBits.ensureSize(oldSize);
     // Class vectors tend to be very short. This is faster than using a hash table.
-    for (unsigned i = 0; i < newSize; ++i) {
+    for (auto& newClass : newClasses) {
         bool foundFromBoth = false;
-        for (unsigned j = 0; j < oldSize; ++j) {
-            if (newClasses[i] == oldClasses[j]) {
-                remainingClassBits.quickSet(j);
+        for (unsigned i = 0; i < oldSize; ++i) {
+            if (newClass == oldClasses[i]) {
+                remainingClassBits.quickSet(i);
                 foundFromBoth = true;
             }
         }
         if (foundFromBoth)
             continue;
-        changedClasses.append({ newClasses[i].impl(), ClassChangeType::Add });
+        changedClasses.append({ newClass.impl(), ClassChangeType::Add });
     }
     for (unsigned i = 0; i < oldSize; ++i) {
         // If the bit is not set the corresponding class has been removed.

diff --git a/Source/WebCore/style/ElementRuleCollector.cpp b/Source/WebCore/style/ElementRuleCollector.cpp
@@ -209,8 +209,8 @@ void ElementRuleCollector::collectMatchingRules(const MatchRequest& matchRequest
     if (!id.isNull())
         collectMatchingRulesForList(matchRequest.ruleSet.idRules(id), matchRequest);
     if (element.hasClass()) {
-        for (size_t i = 0; i < element.classNames().size(); ++i)
-            collectMatchingRulesForList(matchRequest.ruleSet.classRules(element.classNames()[i]), matchRequest);
+        for (auto& className : element.classNames())
+            collectMatchingRulesForList(matchRequest.ruleSet.classRules(className), matchRequest);
     }
     if (element.hasAttributesWithoutUpdate() && matchRequest.ruleSet.hasAttributeRules()) {
         Vector<const RuleSet::RuleDataVector*, 4> ruleVectors;

diff --git a/Source/WebCore/style/PseudoClassChangeInvalidation.cpp b/Source/WebCore/style/PseudoClassChangeInvalidation.cpp
@@ -41,9 +41,9 @@ Vector<PseudoClassInvalidationKey, 4> makePseudoClassInvalidationKeys(CSSSelecto
         keys.append(makePseudoClassInvalidationKey(pseudoClass, InvalidationKeyType::Id, element.idForStyleResolution()));
 
     if (element.hasClass()) {
-        auto classCount = element.classNames().size();
-        for (size_t i = 0; i < classCount; ++i)
-            keys.append(makePseudoClassInvalidationKey(pseudoClass, InvalidationKeyType::Class, element.classNames()[i]));
+        keys.appendContainerWithMapping(element.classNames(), [&](auto& className) {
+            return makePseudoClassInvalidationKey(pseudoClass, InvalidationKeyType::Class, className);
+        });
     }
 
     keys.append(makePseudoClassInvalidationKey(pseudoClass, InvalidationKeyType::Tag, element.localNameLowercase()));

diff --git a/Source/WebCore/style/StyleSharingResolver.cpp b/Source/WebCore/style/StyleSharingResolver.cpp
@@ -348,8 +348,8 @@ bool SharingResolver::sharingCandidateHasIdenticalStyleAffectingAttributes(const
 
 bool SharingResolver::classNamesAffectedByRules(const SpaceSplitString& classNames) const
 {
-    for (unsigned i = 0; i < classNames.size(); ++i) {
-        if (m_ruleSets.features().classRules.contains(classNames[i]))
+    for (auto& className : classNames) {
+        if (m_ruleSets.features().classRules.contains(className))
             return true;
     }
     return false;