Modified: trunk/Source/WebCore/ChangeLog (171019 => 171020)
--- trunk/Source/WebCore/ChangeLog 2014-07-12 03:54:05 UTC (rev 171019)
+++ trunk/Source/WebCore/ChangeLog 2014-07-12 05:08:33 UTC (rev 171020)
@@ -1,3 +1,37 @@
+2014-07-11 Benjamin Poulain <[email protected]>
+
+ Partition the CSS rules based on the most specific filter of the rightmost fragment
+ https://bugs.webkit.org/show_bug.cgi?id=134828
+
+ Reviewed by Andreas Kling.
+
+ Previously, RuleSet was partitioning each rule based on the rightmost filter.
+ While fast, this had the side effect of putting many selectors with ID match in the class
+ bucket (because the selectors are generally written starting with the ID).
+
+ This patch replace the code of findBestRuleSetAndAdd() by a simple loop going over all
+ the simple selectors in the rightmost fragment to find the best bucket.
+
+ * css/ElementRuleCollector.cpp:
+ (WebCore::ElementRuleCollector::ruleMatches):
+ * css/RuleSet.cpp:
+ (WebCore::isSelectorMatchingHTMLBasedOnRuleHash):
+ I unified ruleData.hasRightmostSelectorMatchingHTMLBasedOnRuleHash() and hasMultipartSelector().
+
+ (WebCore::RuleData::RuleData):
+ (WebCore::rulesCountForName):
+ (WebCore::RuleSet::addRule):
+ I removed the recursive part of findBestRuleSetAndAdd() (which was wrong anyway). The function
+ was useless so I just moved the algorithm to addRule() directly.
+
+ We first loop over all the CSSSelectors related by SubSelector, this correspond to the rightmost fragment.
+ If a filter with high specificity is found, we add the rule immediately and end there.
+ If a filter that is not very specific is found, we keep a pointer to the selector to use it later.
+
+ (WebCore::RuleSet::findBestRuleSetAndAdd): Deleted.
+ * css/RuleSet.h:
+ (WebCore::RuleData::hasMultipartSelector): Deleted.
+
2014-07-11 Alex Christensen <[email protected]>
[WinCairo] Unreviewed build fix after r170937.
Modified: trunk/Source/WebCore/css/RuleSet.cpp (171019 => 171020)
--- trunk/Source/WebCore/css/RuleSet.cpp 2014-07-12 03:54:05 UTC (rev 171019)
+++ trunk/Source/WebCore/css/RuleSet.cpp 2014-07-12 05:08:33 UTC (rev 171020)
@@ -2,7 +2,7 @@
* Copyright (C) 1999 Lars Knoll ([email protected])
* (C) 2004-2005 Allan Sandfeld Jensen ([email protected])
* Copyright (C) 2006, 2007 Nicholas Shanks ([email protected])
- * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2014 Apple Inc. All rights reserved.
* Copyright (C) 2007 Alexey Proskuryakov <[email protected]>
* Copyright (C) 2007, 2008 Eric Seidel <[email protected]>
* Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
@@ -54,20 +54,18 @@
// -----------------------------------------------------------------
-static inline bool isSelectorMatchingHTMLBasedOnRuleHash(const CSSSelector* selector)
+static inline bool isSelectorMatchingHTMLBasedOnRuleHash(const CSSSelector& selector)
{
- ASSERT(selector);
- if (selector->m_match == CSSSelector::Tag) {
- const AtomicString& selectorNamespace = selector->tagQName().namespaceURI();
- if (selectorNamespace != starAtom && selectorNamespace != xhtmlNamespaceURI)
- return false;
- if (selector->relation() == CSSSelector::SubSelector)
- return isSelectorMatchingHTMLBasedOnRuleHash(selector->tagHistory());
- return true;
+ if (selector.tagHistory())
+ return false;
+
+ if (selector.m_match == CSSSelector::Tag) {
+ const AtomicString& selectorNamespace = selector.tagQName().namespaceURI();
+ return selectorNamespace == starAtom || selectorNamespace == xhtmlNamespaceURI;
}
- if (SelectorChecker::isCommonPseudoClassSelector(selector))
+ if (SelectorChecker::isCommonPseudoClassSelector(&selector))
return true;
- return selector->m_match == CSSSelector::Id || selector->m_match == CSSSelector::Class;
+ return selector.m_match == CSSSelector::Id || selector.m_match == CSSSelector::Class;
}
static inline bool selectorListContainsUncommonAttributeSelector(const CSSSelector* selector)
@@ -134,8 +132,7 @@
, m_position(position)
, m_hasFastCheckableSelector((addRuleFlags & RuleCanUseFastCheckSelector) && SelectorCheckerFastPath::canUse(selector()))
, m_specificity(selector()->specificity())
- , m_hasMultipartSelector(!!selector()->tagHistory())
- , m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash(isSelectorMatchingHTMLBasedOnRuleHash(selector()))
+ , m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash(isSelectorMatchingHTMLBasedOnRuleHash(*selector()))
, m_containsUncommonAttributeSelector(WebCore::containsUncommonAttributeSelector(selector()))
, m_linkMatchType(SelectorChecker::determineLinkMatchType(selector()))
, m_hasDocumentSecurityOrigin(addRuleFlags & RuleHasDocumentSecurityOrigin)
@@ -169,7 +166,7 @@
if (ruleData.containsUncommonAttributeSelector())
features.uncommonAttributeRules.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex(), ruleData.hasDocumentSecurityOrigin()));
}
-
+
void RuleSet::addToRuleSet(AtomicStringImpl* key, AtomRuleMap& map, const RuleData& ruleData)
{
if (!key)
@@ -180,68 +177,101 @@
rules->append(ruleData);
}
-bool RuleSet::findBestRuleSetAndAdd(const CSSSelector* component, RuleData& ruleData)
+static unsigned rulesCountForName(const RuleSet::AtomRuleMap& map, AtomicStringImpl* name)
{
- if (component->m_match == CSSSelector::Id) {
- addToRuleSet(component->value().impl(), m_idRules, ruleData);
- return true;
- }
- if (component->m_match == CSSSelector::Class) {
- addToRuleSet(component->value().impl(), m_classRules, ruleData);
- return true;
- }
- if (component->isCustomPseudoElement()) {
- addToRuleSet(component->value().impl(), m_shadowPseudoElementRules, ruleData);
- return true;
- }
+ if (const Vector<RuleData>* rules = map.get(name))
+ return rules->size();
+ return 0;
+}
+
+void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addRuleFlags)
+{
+ RuleData ruleData(rule, selectorIndex, m_ruleCount++, addRuleFlags);
+ collectFeaturesFromRuleData(m_features, ruleData);
+
+ unsigned classBucketSize = 0;
+ const CSSSelector* tagSelector = nullptr;
+ const CSSSelector* classSelector = nullptr;
+ const CSSSelector* linkSelector = nullptr;
+ const CSSSelector* focusSelector = nullptr;
+ const CSSSelector* selector = ruleData.selector();
+ do {
+ if (selector->m_match == CSSSelector::Id) {
+ addToRuleSet(selector->value().impl(), m_idRules, ruleData);
+ return;
+ }
+
#if ENABLE(VIDEO_TRACK)
- if (component->m_match == CSSSelector::PseudoElement && component->pseudoElementType() == CSSSelector::PseudoElementCue) {
- m_cuePseudoRules.append(ruleData);
- return true;
- }
+ if (selector->m_match == CSSSelector::PseudoElement && selector->pseudoElementType() == CSSSelector::PseudoElementCue) {
+ m_cuePseudoRules.append(ruleData);
+ return;
+ }
#endif
- if (SelectorChecker::isCommonPseudoClassSelector(component)) {
- switch (component->pseudoClassType()) {
- case CSSSelector::PseudoClassLink:
- case CSSSelector::PseudoClassVisited:
- case CSSSelector::PseudoClassAnyLink:
- m_linkPseudoClassRules.append(ruleData);
- return true;
- case CSSSelector::PseudoClassFocus:
- m_focusPseudoClassRules.append(ruleData);
- return true;
- default:
- ASSERT_NOT_REACHED();
- return true;
+
+ if (selector->isCustomPseudoElement()) {
+ addToRuleSet(selector->value().impl(), m_shadowPseudoElementRules, ruleData);
+ return;
}
- }
- if (component->m_match == CSSSelector::Tag) {
- // If this is part of a subselector chain, recurse ahead to find a narrower set (ID/class/:pseudo)
- if (component->relation() == CSSSelector::SubSelector) {
- const CSSSelector* nextComponent = component->tagHistory();
- if (nextComponent->m_match == CSSSelector::Class || nextComponent->m_match == CSSSelector::Id || SelectorChecker::isCommonPseudoClassSelector(nextComponent)) {
- if (findBestRuleSetAndAdd(nextComponent, ruleData))
- return true;
+ if (selector->m_match == CSSSelector::Class) {
+ AtomicStringImpl* className = selector->value().impl();
+ if (!classSelector) {
+ classSelector = selector;
+ classBucketSize = rulesCountForName(m_classRules, className);
+ } else if (classBucketSize) {
+ unsigned newClassBucketSize = rulesCountForName(m_classRules, className);
+ if (newClassBucketSize < classBucketSize) {
+ classSelector = selector;
+ classBucketSize = newClassBucketSize;
+ }
}
}
- if (component->tagQName().localName() != starAtom) {
- addToRuleSet(component->tagQName().localName().impl(), m_tagRules, ruleData);
- return true;
+
+ if (selector->m_match == CSSSelector::Tag && selector->tagQName().localName() != starAtom)
+ tagSelector = selector;
+
+ if (SelectorChecker::isCommonPseudoClassSelector(selector)) {
+ switch (selector->pseudoClassType()) {
+ case CSSSelector::PseudoClassLink:
+ case CSSSelector::PseudoClassVisited:
+ case CSSSelector::PseudoClassAnyLink:
+ linkSelector = selector;
+ break;
+ case CSSSelector::PseudoClassFocus:
+ focusSelector = selector;
+ break;
+ default:
+ ASSERT_NOT_REACHED();
+ }
}
+
+ if (selector->relation() != CSSSelector::SubSelector)
+ break;
+ selector = selector->tagHistory();
+ } while (selector);
+
+ if (classSelector) {
+ addToRuleSet(classSelector->value().impl(), m_classRules, ruleData);
+ return;
}
- return false;
-}
-void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addRuleFlags)
-{
- RuleData ruleData(rule, selectorIndex, m_ruleCount++, addRuleFlags);
- collectFeaturesFromRuleData(m_features, ruleData);
+ if (linkSelector) {
+ m_linkPseudoClassRules.append(ruleData);
+ return;
+ }
- if (!findBestRuleSetAndAdd(ruleData.selector(), ruleData)) {
- // If we didn't find a specialized map to stick it in, file under universal rules.
- m_universalRules.append(ruleData);
+ if (focusSelector) {
+ m_focusPseudoClassRules.append(ruleData);
+ return;
}
+
+ if (tagSelector) {
+ addToRuleSet(tagSelector->tagQName().localName().impl(), m_tagRules, ruleData);
+ return;
+ }
+
+ // If we didn't find a specialized map to stick it in, file under universal rules.
+ m_universalRules.append(ruleData);
}
void RuleSet::addPageRule(StyleRulePage* rule)
Modified: trunk/Source/WebCore/css/RuleSet.h (171019 => 171020)
--- trunk/Source/WebCore/css/RuleSet.h 2014-07-12 03:54:05 UTC (rev 171019)
+++ trunk/Source/WebCore/css/RuleSet.h 2014-07-12 05:08:33 UTC (rev 171020)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999 Lars Knoll ([email protected])
- * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2014 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -67,7 +67,6 @@
unsigned selectorIndex() const { return m_selectorIndex; }
bool hasFastCheckableSelector() const { return m_hasFastCheckableSelector; }
- bool hasMultipartSelector() const { return m_hasMultipartSelector; }
bool hasRightmostSelectorMatchingHTMLBasedOnRuleHash() const { return m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash; }
bool containsUncommonAttributeSelector() const { return m_containsUncommonAttributeSelector; }
unsigned specificity() const { return m_specificity; }
@@ -104,7 +103,6 @@
unsigned m_position : 18;
unsigned m_hasFastCheckableSelector : 1;
unsigned m_specificity : 24;
- unsigned m_hasMultipartSelector : 1;
unsigned m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash : 1;
unsigned m_containsUncommonAttributeSelector : 1;
unsigned m_linkMatchType : 2; // SelectorChecker::LinkMatchMask
@@ -185,7 +183,6 @@
private:
void addChildRules(const Vector<RefPtr<StyleRuleBase>>&, const MediaQueryEvaluator& medium, StyleResolver*, bool hasDocumentSecurityOrigin, AddRuleFlags);
- bool findBestRuleSetAndAdd(const CSSSelector*, RuleData&);
AtomRuleMap m_idRules;
AtomRuleMap m_classRules;
