Skip to content

Commit

Permalink
[CSS] Don't construct CSSSelectorList when not needed
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=265518
rdar://118931010

Reviewed by Antti Koivisto.

CSSSelectorList is an optimized data structure,
we should only construct it when it's actually needed (so not when empty or error)
and keep the editable CSSParserSelector data structure until then.

* Source/WebCore/css/parser/CSSParserImpl.cpp:
(WebCore::CSSParserImpl::consumeStyleRule):
* Source/WebCore/css/parser/CSSSelectorParser.cpp:
(WebCore::parseCSSParserSelectorList):
(WebCore::parseCSSSelectorList):
(WebCore::CSSSelectorParser::consumeSelectorList):
(WebCore::CSSSelectorParser::consumeComplexSelectorList):
(WebCore::CSSSelectorParser::consumeRelativeSelectorList):
(WebCore::CSSSelectorParser::consumeNestedSelectorList):
(WebCore::CSSSelectorParser::consumeForgivingSelectorList):
(WebCore::CSSSelectorParser::consumeComplexForgivingSelectorList):
(WebCore::CSSSelectorParser::consumeNestedComplexForgivingSelectorList):
(WebCore::CSSSelectorParser::consumeCompoundSelectorList):
(WebCore::CSSSelectorParser::consumePseudo):
* Source/WebCore/css/parser/CSSSelectorParser.h:

Canonical link: https://commits.webkit.org/271290@main
  • Loading branch information
@mdubet
mdubet committed Nov 29, 2023
1 parent 95bfda0 commit 3ff376e
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 47 deletions.
17 changes: 11 additions & 6 deletions Source/WebCore/css/parser/CSSParserImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "CSSParserObserverWrapper.h"
#include "CSSParserSelector.h"
#include "CSSPropertyParser.h"
#include "CSSSelectorList.h"
#include "CSSSelectorParser.h"
#include "CSSStyleSheet.h"
#include "CSSSupportsParser.h"
Expand Down Expand Up @@ -1206,13 +1207,17 @@ static void observeSelectors(CSSParserObserverWrapper& wrapper, CSSParserTokenRa

RefPtr<StyleRuleBase> CSSParserImpl::consumeStyleRule(CSSParserTokenRange prelude, CSSParserTokenRange block)
{
auto selectorList = parseCSSSelectorList(prelude, m_context, m_styleSheet.get(), isNestedContext() ? CSSParserEnum::IsNestedContext::Yes : CSSParserEnum::IsNestedContext::No);
auto preludeCopyForInspector = prelude;
auto parserSelectorList = parseCSSParserSelectorList(prelude, m_context, m_styleSheet.get(), isNestedContext() ? CSSParserEnum::IsNestedContext::Yes : CSSParserEnum::IsNestedContext::No, CSSParserEnum::IsForgiving::No);

if (!selectorList)
if (parserSelectorList.isEmpty())
return nullptr; // Parse error, invalid selector list

auto selectorList = CSSSelectorList { WTFMove(parserSelectorList) };
ASSERT(!selectorList.isEmpty());

if (m_observerWrapper)
observeSelectors(*m_observerWrapper, prelude);
observeSelectors(*m_observerWrapper, preludeCopyForInspector);

RefPtr<StyleRuleBase> styleRule;

Expand All @@ -1226,10 +1231,10 @@ RefPtr<StyleRuleBase> CSSParserImpl::consumeStyleRule(CSSParserTokenRange prelud
auto properties = createStyleProperties(topContext().m_parsedProperties, m_context.mode);

// We save memory by creating a simple StyleRule instead of a heavier StyleRuleWithNesting when we don't need the CSS Nesting features.
if (nestedRules.isEmpty() && !selectorList->hasExplicitNestingParent() && !isNestedContext())
styleRule = StyleRule::create(WTFMove(properties), m_context.hasDocumentSecurityOrigin, WTFMove(*selectorList));
if (nestedRules.isEmpty() && !selectorList.hasExplicitNestingParent() && !isNestedContext())
styleRule = StyleRule::create(WTFMove(properties), m_context.hasDocumentSecurityOrigin, WTFMove(selectorList));
else {
styleRule = StyleRuleWithNesting::create(WTFMove(properties), m_context.hasDocumentSecurityOrigin, WTFMove(*selectorList), WTFMove(nestedRules));
styleRule = StyleRuleWithNesting::create(WTFMove(properties), m_context.hasDocumentSecurityOrigin, WTFMove(selectorList), WTFMove(nestedRules));
m_styleSheet->setHasNestingRules();
}
});
Expand Down
76 changes: 43 additions & 33 deletions Source/WebCore/css/parser/CSSSelectorParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@
#include "config.h"
#include "CSSSelectorParser.h"

#include "CSSParserEnum.h"
#include "CSSParserIdioms.h"
#include "CSSParserSelector.h"
#include "CSSSelector.h"
#include "CommonAtomStrings.h"
#include "DeprecatedGlobalSettings.h"
#include "Document.h"
#include "SelectorPseudoTypeMap.h"
#include "css/parser/CSSParserEnum.h"
#include <memory>
#include <wtf/OptionSet.h>
#include <wtf/SetForScope.h>
Expand All @@ -49,7 +49,7 @@ namespace WebCore {
static AtomString serializeANPlusB(const std::pair<int, int>&);
static bool consumeANPlusB(CSSParserTokenRange&, std::pair<int, int>&);

std::optional<CSSSelectorList> parseCSSSelectorList(CSSParserTokenRange range, const CSSSelectorParserContext& context, StyleSheetContents* styleSheet, CSSParserEnum::IsNestedContext isNestedContext, CSSParserEnum::IsForgiving isForgiving)
CSSParserSelectorList parseCSSParserSelectorList(CSSParserTokenRange& range, const CSSSelectorParserContext& context, StyleSheetContents* styleSheet, CSSParserEnum::IsNestedContext isNestedContext, CSSParserEnum::IsForgiving isForgiving)
{
CSSSelectorParser parser(context, styleSheet, isNestedContext);
range.consumeWhitespace();
Expand All @@ -62,12 +62,22 @@ std::optional<CSSSelectorList> parseCSSSelectorList(CSSParserTokenRange range, c
return parser.consumeComplexForgivingSelectorList(range);
return parser.consumeComplexSelectorList(range);
};
CSSSelectorList result = consume();
auto result = consume();
if (result.isEmpty() || !range.atEnd())
return { };
return result;
}

std::optional<CSSSelectorList> parseCSSSelectorList(CSSParserTokenRange range, const CSSSelectorParserContext& context, StyleSheetContents* styleSheet, CSSParserEnum::IsNestedContext isNestedContext, CSSParserEnum::IsForgiving isForgiving)
{
auto result = parseCSSParserSelectorList(range, context, styleSheet, isNestedContext, isForgiving);

if (result.isEmpty() || !range.atEnd())
return { };

return CSSSelectorList { WTFMove(result) };
}

CSSSelectorParser::CSSSelectorParser(const CSSSelectorParserContext& context, StyleSheetContents* styleSheet, CSSParserEnum::IsNestedContext isNestedContext)
: m_context(context)
, m_styleSheet(styleSheet)
Expand All @@ -76,7 +86,7 @@ CSSSelectorParser::CSSSelectorParser(const CSSSelectorParserContext& context, St
}

template <typename ConsumeSelector>
CSSSelectorList CSSSelectorParser::consumeSelectorList(CSSParserTokenRange& range, ConsumeSelector&& consumeSelector)
CSSParserSelectorList CSSSelectorParser::consumeSelectorList(CSSParserTokenRange& range, ConsumeSelector&& consumeSelector)
{
Vector<std::unique_ptr<CSSParserSelector>> selectorList;
auto selector = consumeSelector(range);
Expand All @@ -95,32 +105,32 @@ CSSSelectorList CSSSelectorParser::consumeSelectorList(CSSParserTokenRange& rang
if (m_failedParsing)
return { };

return CSSSelectorList { WTFMove(selectorList) };
return selectorList;
}

CSSSelectorList CSSSelectorParser::consumeComplexSelectorList(CSSParserTokenRange& range)
CSSParserSelectorList CSSSelectorParser::consumeComplexSelectorList(CSSParserTokenRange& range)
{
return consumeSelectorList(range, [&] (CSSParserTokenRange& range) {
return consumeComplexSelector(range);
});
}

CSSSelectorList CSSSelectorParser::consumeRelativeSelectorList(CSSParserTokenRange& range)
CSSParserSelectorList CSSSelectorParser::consumeRelativeSelectorList(CSSParserTokenRange& range)
{
return consumeSelectorList(range, [&](CSSParserTokenRange& range) {
return consumeRelativeScopeSelector(range);
});
}

CSSSelectorList CSSSelectorParser::consumeNestedSelectorList(CSSParserTokenRange& range)
CSSParserSelectorList CSSSelectorParser::consumeNestedSelectorList(CSSParserTokenRange& range)
{
return consumeSelectorList(range, [&] (CSSParserTokenRange& range) {
return consumeNestedComplexSelector(range);
});
}

template<typename ConsumeSelector>
CSSSelectorList CSSSelectorParser::consumeForgivingSelectorList(CSSParserTokenRange& range, ConsumeSelector&& consumeSelector)
CSSParserSelectorList CSSSelectorParser::consumeForgivingSelectorList(CSSParserTokenRange& range, ConsumeSelector&& consumeSelector)
{
if (m_failedParsing)
return { };
Expand Down Expand Up @@ -178,17 +188,17 @@ CSSSelectorList CSSSelectorParser::consumeForgivingSelectorList(CSSParserTokenRa
return { };
}

return CSSSelectorList { WTFMove(selectorList) };
return selectorList;
}

CSSSelectorList CSSSelectorParser::consumeComplexForgivingSelectorList(CSSParserTokenRange& range)
CSSParserSelectorList CSSSelectorParser::consumeComplexForgivingSelectorList(CSSParserTokenRange& range)
{
return consumeForgivingSelectorList(range, [&](CSSParserTokenRange& range) {
return consumeComplexSelector(range);
});
}

CSSSelectorList CSSSelectorParser::consumeNestedComplexForgivingSelectorList(CSSParserTokenRange& range)
CSSParserSelectorList CSSSelectorParser::consumeNestedComplexForgivingSelectorList(CSSParserTokenRange& range)
{
return consumeForgivingSelectorList(range, [&] (CSSParserTokenRange& range) {
return consumeNestedComplexSelector(range);
Expand Down Expand Up @@ -218,23 +228,23 @@ bool CSSSelectorParser::supportsComplexSelector(CSSParserTokenRange range, const
return !containsUnknownWebKitPseudoElements(*complexSelector);
}

CSSSelectorList CSSSelectorParser::consumeCompoundSelectorList(CSSParserTokenRange& range)
CSSParserSelectorList CSSSelectorParser::consumeCompoundSelectorList(CSSParserTokenRange& range)
{
Vector<std::unique_ptr<CSSParserSelector>> selectorList;
auto selector = consumeCompoundSelector(range);
range.consumeWhitespace();
if (!selector)
return CSSSelectorList();
return { };
selectorList.append(WTFMove(selector));
while (!range.atEnd() && range.peek().type() == CommaToken) {
range.consumeIncludingWhitespace();
selector = consumeCompoundSelector(range);
range.consumeWhitespace();
if (!selector)
return CSSSelectorList();
return { };
selectorList.append(WTFMove(selector));
}
return CSSSelectorList { WTFMove(selectorList) };
return selectorList;
}

static PossiblyQuotedIdentifier consumePossiblyQuotedIdentifier(CSSParserTokenRange& range)
Expand Down Expand Up @@ -833,11 +843,10 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumePseudo(CSSParserTok
switch (selector->pseudoClassType()) {
case CSSSelector::PseudoClassType::Not: {
SetForScope resistDefaultNamespace(m_resistDefaultNamespace, true);
auto selectorList = makeUnique<CSSSelectorList>();
*selectorList = consumeComplexSelectorList(block);
if (!selectorList->first() || !block.atEnd())
auto selectorList = consumeComplexSelectorList(block);
if (selectorList.size() < 1 || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
selector->setSelectorList(makeUnique<CSSSelectorList>(WTFMove(selectorList)));
return selector;
}
case CSSSelector::PseudoClassType::NthChild:
Expand All @@ -860,11 +869,10 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumePseudo(CSSParserTok
if (!equalLettersIgnoringASCIICase(ident.value(), "of"_s))
return nullptr;
block.consumeWhitespace();
auto selectorList = makeUnique<CSSSelectorList>();
*selectorList = consumeComplexSelectorList(block);
if (selectorList->isEmpty() || !block.atEnd())
auto selectorList = consumeComplexSelectorList(block);
if (selectorList.isEmpty() || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
selector->setSelectorList(makeUnique<CSSSelectorList>(WTFMove(selectorList)));
}
selector->setNth(ab.first, ab.second);
return selector;
Expand All @@ -882,9 +890,13 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumePseudo(CSSParserTok
case CSSSelector::PseudoClassType::Any: {
SetForScope resistDefaultNamespace(m_resistDefaultNamespace, true);
auto selectorList = makeUnique<CSSSelectorList>();
*selectorList = consumeComplexForgivingSelectorList(block);
auto consumedBlock = consumeComplexForgivingSelectorList(block);
if (!block.atEnd())
return nullptr;
if (consumedBlock.isEmpty())
*selectorList = CSSSelectorList { };
else
*selectorList = CSSSelectorList { WTFMove(consumedBlock) };
selector->setSelectorList(WTFMove(selectorList));
return selector;
}
Expand All @@ -901,11 +913,10 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumePseudo(CSSParserTok
return nullptr;
SetForScope resistDefaultNamespace(m_resistDefaultNamespace, true);
SetForScope disallowNestedHas(m_disallowHasPseudoClass, true);
auto selectorList = makeUnique<CSSSelectorList>();
*selectorList = consumeRelativeSelectorList(block);
if (selectorList->isEmpty() || !block.atEnd())
auto selectorList = consumeRelativeSelectorList(block);
if (selectorList.isEmpty() || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
selector->setSelectorList(makeUnique<CSSSelectorList>(WTFMove(selectorList)));
return selector;
}
case CSSSelector::PseudoClassType::Dir: {
Expand All @@ -924,11 +935,10 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumePseudo(CSSParserTok
switch (selector->pseudoElementType()) {
#if ENABLE(VIDEO)
case CSSSelector::PseudoElementCue: {
auto selectorList = makeUnique<CSSSelectorList>();
*selectorList = consumeCompoundSelectorList(block);
if (selectorList->isEmpty() || !block.atEnd())
auto selectorList = consumeCompoundSelectorList(block);
if (selectorList.isEmpty() || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
selector->setSelectorList(makeUnique<CSSSelectorList>(WTFMove(selectorList)));
return selector;
}
#endif
Expand Down
19 changes: 11 additions & 8 deletions Source/WebCore/css/parser/CSSSelectorParser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,26 @@ class CSSSelectorList;
class StyleSheetContents;
class StyleRule;

using CSSParserSelectorList = Vector<std::unique_ptr<CSSParserSelector>>;

class CSSSelectorParser {
public:
CSSSelectorParser(const CSSSelectorParserContext&, StyleSheetContents*, CSSParserEnum::IsNestedContext = CSSParserEnum::IsNestedContext::No);

CSSSelectorList consumeComplexSelectorList(CSSParserTokenRange&);
CSSSelectorList consumeNestedSelectorList(CSSParserTokenRange&);
CSSSelectorList consumeComplexForgivingSelectorList(CSSParserTokenRange&);
CSSSelectorList consumeNestedComplexForgivingSelectorList(CSSParserTokenRange&);
CSSParserSelectorList consumeComplexSelectorList(CSSParserTokenRange&);
CSSParserSelectorList consumeNestedSelectorList(CSSParserTokenRange&);
CSSParserSelectorList consumeComplexForgivingSelectorList(CSSParserTokenRange&);
CSSParserSelectorList consumeNestedComplexForgivingSelectorList(CSSParserTokenRange&);

static bool supportsComplexSelector(CSSParserTokenRange, const CSSSelectorParserContext&, CSSParserEnum::IsNestedContext);
static CSSSelectorList resolveNestingParent(const CSSSelectorList& nestedSelectorList, const CSSSelectorList* parentResolvedSelectorList);

private:
template<typename ConsumeSelector> CSSSelectorList consumeSelectorList(CSSParserTokenRange&, ConsumeSelector&&);
template<typename ConsumeSelector> CSSSelectorList consumeForgivingSelectorList(CSSParserTokenRange&, ConsumeSelector&&);
template<typename ConsumeSelector> CSSParserSelectorList consumeSelectorList(CSSParserTokenRange&, ConsumeSelector&&);
template<typename ConsumeSelector> CSSParserSelectorList consumeForgivingSelectorList(CSSParserTokenRange&, ConsumeSelector&&);

CSSSelectorList consumeCompoundSelectorList(CSSParserTokenRange&);
CSSSelectorList consumeRelativeSelectorList(CSSParserTokenRange&);
CSSParserSelectorList consumeCompoundSelectorList(CSSParserTokenRange&);
CSSParserSelectorList consumeRelativeSelectorList(CSSParserTokenRange&);

std::unique_ptr<CSSParserSelector> consumeComplexSelector(CSSParserTokenRange&);
std::unique_ptr<CSSParserSelector> consumeNestedComplexSelector(CSSParserTokenRange&);
Expand Down Expand Up @@ -109,5 +111,6 @@ class CSSSelectorParser {
};

std::optional<CSSSelectorList> parseCSSSelectorList(CSSParserTokenRange, const CSSSelectorParserContext&, StyleSheetContents* = nullptr, CSSParserEnum::IsNestedContext = CSSParserEnum::IsNestedContext::No, CSSParserEnum::IsForgiving = CSSParserEnum::IsForgiving::No);
CSSParserSelectorList parseCSSParserSelectorList(CSSParserTokenRange&, const CSSSelectorParserContext&, StyleSheetContents*, CSSParserEnum::IsNestedContext, CSSParserEnum::IsForgiving);

} // namespace WebCore

0 comments on commit 3ff376e

Please sign in to comment.