Compute the linkMatchType inside the CSSSelectorParser. (issue 1149913008 by esprehn@chromium.org)

[esp...@chromium.org]

Reviewers: Timothy Loh, rune,

Description:
Compute the linkMatchType inside the CSSSelectorParser.

Store the linkMatchType on the first CSSSelector in each CSSSelectorList
instead of computing it every time we build the RuleSets.

R=tim...@chromium.org

Please review this at https://codereview.chromium.org/1149913008/

Base URL: svn://svn.chromium.org/blink/trunk

Affected files (+22, -18 lines):
M Source/core/css/CSSSelector.h
M Source/core/css/CSSSelector.cpp
M Source/core/css/RuleSet.h
M Source/core/css/RuleSet.cpp
M Source/core/css/parser/CSSParserValues.h
M Source/core/css/parser/CSSSelectorParser.cpp


Index: Source/core/css/CSSSelector.cpp
diff --git a/Source/core/css/CSSSelector.cpp
b/Source/core/css/CSSSelector.cpp
index
901343b96cec615c663eb461dc23f542a204d604..a5723f0fea2225de06faea615c2f3ed4a04fa39e
100644
--- a/Source/core/css/CSSSelector.cpp
+++ b/Source/core/css/CSSSelector.cpp
@@ -853,10 +853,8 @@ bool CSSSelector::isCompound() const
return true;
}

-unsigned CSSSelector::computeLinkMatchType() const
+void CSSSelector::updateLinkMatchType()
{
- unsigned linkMatchType = MatchAll;
-
// Determine if this selector will match a link in visited, unvisited
or any state, or never.
// :visited never matches other elements than the innermost link
element.
for (const CSSSelector* current = this; current; current =
current->tagHistory()) {
@@ -868,17 +866,17 @@ unsigned CSSSelector::computeLinkMatchType() const
for (const CSSSelector* subSelector =
current->selectorList()->first(); subSelector; subSelector =
subSelector->tagHistory()) {
PseudoType subType = subSelector->pseudoType();
if (subType == PseudoVisited)
- linkMatchType &= ~MatchVisited;
+ m_linkMatchType &= ~MatchVisited;
else if (subType == PseudoLink)
- linkMatchType &= ~MatchLink;
+ m_linkMatchType &= ~MatchLink;
}
}
break;
case PseudoLink:
- linkMatchType &= ~MatchVisited;
+ m_linkMatchType &= ~MatchVisited;
break;
case PseudoVisited:
- linkMatchType &= ~MatchLink;
+ m_linkMatchType &= ~MatchLink;
break;
default:
// We don't support :link and :visited inside :-webkit-any.
@@ -888,11 +886,10 @@ unsigned CSSSelector::computeLinkMatchType() const
if (relation == SubSelector)
continue;
if (relation != Descendant && relation != Child)
- return linkMatchType;
- if (linkMatchType != MatchAll)
- return linkMatchType;
+ return;
+ if (m_linkMatchType != MatchAll)
+ return;
}
- return linkMatchType;
}

void CSSSelector::setNth(int a, int b)
Index: Source/core/css/CSSSelector.h
diff --git a/Source/core/css/CSSSelector.h b/Source/core/css/CSSSelector.h
index
a787c3647b947457c07250392cb6e75cb352368e..6932ca13fc11b0ba5c18eb91c82e4bc7d409012d
100644
--- a/Source/core/css/CSSSelector.h
+++ b/Source/core/css/CSSSelector.h
@@ -296,8 +296,11 @@ namespace blink {
// http://dev.w3.org/csswg/selectors4/#compound
bool isCompound() const;

+ // Note: the link match type is only valid for the start of each
selector
+ // in a CSSSelectorList.
enum LinkMatchMask { MatchLink = 1, MatchVisited = 2, MatchAll =
MatchLink | MatchVisited };
- unsigned computeLinkMatchType() const;
+ void updateLinkMatchType();
+ unsigned linkMatchType() const { return m_linkMatchType; }

bool isForPage() const { return m_isForPage; }
void setForPage() { m_isForPage = true; }
@@ -306,15 +309,16 @@ namespace blink {
void setRelationIsAffectedByPseudoContent() {
m_relationIsAffectedByPseudoContent = true; }

private:
- unsigned m_relation : 3; // enum Relation
- mutable unsigned m_match : 4; // enum Match
- mutable unsigned m_pseudoType : 8; // PseudoType
+ unsigned m_relation : 3; // enum Relation
+ unsigned m_match : 4; // enum Match
+ unsigned m_pseudoType : 8; // PseudoType
unsigned m_isLastInSelectorList : 1;
unsigned m_isLastInTagHistory : 1;
unsigned m_hasRareData : 1;
unsigned m_isForPage : 1;
unsigned m_tagIsImplicit : 1;
unsigned m_relationIsAffectedByPseudoContent : 1;
+ unsigned m_linkMatchType : 2; // LinkMatchMask

void setPseudoType(PseudoType pseudoType)
{
@@ -417,6 +421,7 @@ inline CSSSelector::CSSSelector()
, m_isForPage(false)
, m_tagIsImplicit(false)
, m_relationIsAffectedByPseudoContent(false)
+ , m_linkMatchType(MatchAll)
{
}

@@ -430,6 +435,7 @@ inline CSSSelector::CSSSelector(const QualifiedName&
tagQName, bool tagIsImplici
, m_isForPage(false)
, m_tagIsImplicit(tagIsImplicit)
, m_relationIsAffectedByPseudoContent(false)
+ , m_linkMatchType(MatchAll)
{
m_data.m_tagQName = tagQName.impl();
m_data.m_tagQName->ref();
@@ -445,6 +451,7 @@ inline CSSSelector::CSSSelector(const CSSSelector& o)
, m_isForPage(o.m_isForPage)
, m_tagIsImplicit(o.m_tagIsImplicit)
,
m_relationIsAffectedByPseudoContent(o.m_relationIsAffectedByPseudoContent)
+ , m_linkMatchType(MatchAll)
{
if (o.m_match == Tag) {
m_data.m_tagQName = o.m_data.m_tagQName;
Index: Source/core/css/RuleSet.cpp
diff --git a/Source/core/css/RuleSet.cpp b/Source/core/css/RuleSet.cpp
index
856011966c9b3b01a0a12ed3c43ae7e49b0f4625..4c1b757a712d945b88e23fd8dd44277392547e9b
100644
--- a/Source/core/css/RuleSet.cpp
+++ b/Source/core/css/RuleSet.cpp
@@ -112,7 +112,6 @@ RuleData::RuleData(StyleRule* rule, unsigned
selectorIndex, unsigned position, A
, m_position(position)
, m_specificity(selector().specificity())
,
m_containsUncommonAttributeSelector(blink::containsUncommonAttributeSelector(selector()))
- , m_linkMatchType(selector().computeLinkMatchType())
, m_hasDocumentSecurityOrigin(addRuleFlags &
RuleHasDocumentSecurityOrigin)
, m_propertyWhitelistType(determinePropertyWhitelistType(addRuleFlags,
selector()))
{
Index: Source/core/css/RuleSet.h
diff --git a/Source/core/css/RuleSet.h b/Source/core/css/RuleSet.h
index
396860e42622c682af3e66c67702172014660b7a..6ca2c2909acfe77cb56b43f164438ec837bf5b73
100644
--- a/Source/core/css/RuleSet.h
+++ b/Source/core/css/RuleSet.h
@@ -84,7 +84,7 @@ public:

bool containsUncommonAttributeSelector() const { return
m_containsUncommonAttributeSelector; }
unsigned specificity() const { return m_specificity; }
- unsigned linkMatchType() const { return m_linkMatchType; }
+ unsigned linkMatchType() const { return selector().linkMatchType(); }
bool hasDocumentSecurityOrigin() const { return
m_hasDocumentSecurityOrigin; }
PropertyWhitelistType propertyWhitelistType(bool isMatchingUARules =
false) const { return isMatchingUARules ? PropertyWhitelistNone :
static_cast<PropertyWhitelistType>(m_propertyWhitelistType); }
// Try to balance between memory usage (there can be lots of RuleData
objects) and good filtering performance.
@@ -102,7 +102,6 @@ private:
unsigned m_position : 18;
unsigned m_specificity : 24;
unsigned m_containsUncommonAttributeSelector : 1;
- unsigned m_linkMatchType : 2; // CSSSelector::LinkMatchMask
unsigned m_hasDocumentSecurityOrigin : 1;
unsigned m_propertyWhitelistType : 2;
// Use plain array instead of a Vector to minimize memory overhead.
Index: Source/core/css/parser/CSSParserValues.h
diff --git a/Source/core/css/parser/CSSParserValues.h
b/Source/core/css/parser/CSSParserValues.h
index
a08b4c3fd6d60a09548e7eb117d4e94e0fa41fbb..1209f60ae07f611f367726d408647707f7754512
100644
--- a/Source/core/css/parser/CSSParserValues.h
+++ b/Source/core/css/parser/CSSParserValues.h
@@ -215,6 +215,7 @@ public:
bool relationIsAffectedByPseudoContent() const { return
m_selector->relationIsAffectedByPseudoContent(); }

void updatePseudoType(const AtomicString& value, bool hasArguments =
false) const { m_selector->updatePseudoType(value, hasArguments); }
+ void updateLinkMatchType() { m_selector->updateLinkMatchType(); }

void adoptSelectorVector(Vector<OwnPtr<CSSParserSelector>>&
selectorVector);
void setSelectorList(PassOwnPtr<CSSSelectorList>);
Index: Source/core/css/parser/CSSSelectorParser.cpp
diff --git a/Source/core/css/parser/CSSSelectorParser.cpp
b/Source/core/css/parser/CSSSelectorParser.cpp
index
418544ee70e32a76acfff019803f5a167fe22114..c638d9d3d462053e2dde3659e486275b52c2aad1
100644
--- a/Source/core/css/parser/CSSSelectorParser.cpp
+++ b/Source/core/css/parser/CSSSelectorParser.cpp
@@ -91,6 +91,7 @@ void
CSSSelectorParser::consumeComplexSelectorList(CSSParserTokenRange& range, C
selector = consumeComplexSelector(range);
if (!selector)
return;
+ selector->updateLinkMatchType();
selectorList.append(selector.release());
}

['commit-bot: I haz the power' via codereview.chromium.org]

Dry run: CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1149913008/1


https://codereview.chromium.org/1149913008/

[tim...@chromium.org]

lgtm




https://codereview.chromium.org/1149913008/diff/1/Source/core/css/parser/CSSSelectorParser.cpp
File Source/core/css/parser/CSSSelectorParser.cpp (right):

https://codereview.chromium.org/1149913008/diff/1/Source/core/css/parser/CSSSelectorParser.cpp#newcode94
Source/core/css/parser/CSSSelectorParser.cpp:94:
selector->updateLinkMatchType();
Probably better to be in consumeComplexSelector?

https://codereview.chromium.org/1149913008/

[esp...@chromium.org]

Yeah.

https://codereview.chromium.org/1149913008/

['commit-bot: I haz the power' via codereview.chromium.org]

Dry run: CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/patch-status/1149913008/20001


https://codereview.chromium.org/1149913008/

['commit-bot: I haz the power' via codereview.chromium.org]

Dry run: Try jobs failed on following builders:
linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/64364)

https://codereview.chromium.org/1149913008/