Skip to content
Permalink
Browse files
Style sharing: Allow sharing between elements with classes not refere…
…nced by any selectors.

<http://webkit.org/b/103925>

Reviewed by Antti Koivisto.

When looking for elements that can share style, instead of blindly rejecting candidates with
a different "class" attribute, check if it's actually referenced by any of the tracked style rules.
It's surprisingly common for web pages to have elements with classes to which no rules apply,
mediawiki content is especially good at this.

~2100 new sharing "hits" on <https://en.wikipedia.org/wiki/Steve_Jobs>.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::canShareStyleWithElement):

    Don't bail early if the two elements have different return values for hasClass().

(WebCore::StyleResolver::classNamesAffectedByRules):

    Helper function that returns whether a SpaceSplitString contains a class name referenced
    by any style rules.

(WebCore::StyleResolver::sharingCandidateHasIdenticalStyleAffectingAttributes):

    Make this a member function since we need access to m_features.classesInRules.

* css/StyleResolver.h:
(StyleResolver):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::locateSharedStyle):

    Cache whether the element we're resolving style for has a "class" attribute referenced by style rules.

Canonical link: https://commits.webkit.org/122197@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@136615 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Andreas Kling committed Dec 5, 2012
1 parent 4d7bc53 commit 9f396cfd811d6de0532a1c02a030c92ecfda8d3d
Showing with 73 additions and 17 deletions.
  1. +35 −0 Source/WebCore/ChangeLog
  2. +34 −17 Source/WebCore/css/StyleResolver.cpp
  3. +4 −0 Source/WebCore/css/StyleResolver.h
@@ -1,3 +1,38 @@
2012-12-04 Andreas Kling <akling@apple.com>

Style sharing: Allow sharing between elements with classes not referenced by any selectors.
<http://webkit.org/b/103925>

Reviewed by Antti Koivisto.

When looking for elements that can share style, instead of blindly rejecting candidates with
a different "class" attribute, check if it's actually referenced by any of the tracked style rules.
It's surprisingly common for web pages to have elements with classes to which no rules apply,
mediawiki content is especially good at this.

~2100 new sharing "hits" on <https://en.wikipedia.org/wiki/Steve_Jobs>.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::canShareStyleWithElement):

Don't bail early if the two elements have different return values for hasClass().

(WebCore::StyleResolver::classNamesAffectedByRules):

Helper function that returns whether a SpaceSplitString contains a class name referenced
by any style rules.

(WebCore::StyleResolver::sharingCandidateHasIdenticalStyleAffectingAttributes):

Make this a member function since we need access to m_features.classesInRules.

* css/StyleResolver.h:
(StyleResolver):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::locateSharedStyle):

Cache whether the element we're resolving style for has a "class" attribute referenced by style rules.

2012-12-04 Silvia Pfeiffer <silviapf@chromium.org>

Refactor Media Control Elements to remove code duplication.
@@ -961,6 +961,15 @@ void StyleResolver::matchAllRules(MatchResult& result, bool includeSMILPropertie
#endif
}

bool StyleResolver::classNamesAffectedByRules(const SpaceSplitString& classNames) const
{
for (unsigned i = 0; i < classNames.size(); ++i) {
if (m_features.classesInRules.contains(classNames[i].impl()))
return true;
}
return false;
}

inline void StyleResolver::initElement(Element* e)
{
if (m_element != e) {
@@ -1134,32 +1143,38 @@ static inline bool elementHasDirectionAuto(Element* element)
return element->isHTMLElement() && toHTMLElement(element)->hasDirectionAuto();
}

static inline bool haveIdenticalStyleAffectingAttributes(StyledElement* a, StyledElement* b)
bool StyleResolver::sharingCandidateHasIdenticalStyleAffectingAttributes(StyledElement* sharingCandidate) const
{
if (a->attributeData() == b->attributeData())
if (m_element->attributeData() == sharingCandidate->attributeData())
return true;
if (a->fastGetAttribute(XMLNames::langAttr) != b->fastGetAttribute(XMLNames::langAttr))
if (m_element->fastGetAttribute(XMLNames::langAttr) != sharingCandidate->fastGetAttribute(XMLNames::langAttr))
return false;
if (a->fastGetAttribute(langAttr) != b->fastGetAttribute(langAttr))
if (m_element->fastGetAttribute(langAttr) != sharingCandidate->fastGetAttribute(langAttr))
return false;
if (a->hasClass()) {

if (!m_elementAffectedByClassRules) {
if (sharingCandidate->hasClass() && classNamesAffectedByRules(sharingCandidate->classNames()))
return false;
} else if (sharingCandidate->hasClass()) {
#if ENABLE(SVG)
// SVG elements require a (slow!) getAttribute comparision because "class" is an animatable attribute for SVG.
if (a->isSVGElement()) {
if (a->getAttribute(classAttr) != b->getAttribute(classAttr))
if (m_element->isSVGElement()) {
if (m_element->getAttribute(classAttr) != sharingCandidate->getAttribute(classAttr))
return false;
} else
} else {
#endif
if (a->attributeData()->classNames() != b->attributeData()->classNames())
return false;
}
if (m_element->classNames() != sharingCandidate->classNames())
return false;
}
} else
return false;

if (a->presentationAttributeStyle() != b->presentationAttributeStyle())
if (m_styledElement->presentationAttributeStyle() != sharingCandidate->presentationAttributeStyle())
return false;

#if ENABLE(PROGRESS_ELEMENT)
if (a->hasTagName(progressTag)) {
if (static_cast<HTMLProgressElement*>(a)->isDeterminate() != static_cast<HTMLProgressElement*>(b)->isDeterminate())
if (m_element->hasTagName(progressTag)) {
if (static_cast<HTMLProgressElement*>(m_element)->isDeterminate() != static_cast<HTMLProgressElement*>(sharingCandidate)->isDeterminate())
return false;
}
#endif
@@ -1177,8 +1192,6 @@ bool StyleResolver::canShareStyleWithElement(StyledElement* element) const
return false;
if (element->tagQName() != m_element->tagQName())
return false;
if (element->hasClass() != m_element->hasClass())
return false;
if (element->inlineStyle())
return false;
if (element->needsStyleRecalc())
@@ -1199,7 +1212,7 @@ bool StyleResolver::canShareStyleWithElement(StyledElement* element) const
return false;
if (element == element->document()->cssTarget())
return false;
if (!haveIdenticalStyleAffectingAttributes(element, m_styledElement))
if (!sharingCandidateHasIdenticalStyleAffectingAttributes(element))
return false;
if (element->additionalPresentationAttributeStyle() != m_styledElement->additionalPresentationAttributeStyle())
return false;
@@ -1293,6 +1306,10 @@ RenderStyle* StyleResolver::locateSharedStyle()
if (elementHasDirectionAuto(m_element))
return 0;

// Cache whether m_element is affected by any known class selectors.
// FIXME: This shouldn't be a member variable. The style sharing code could be factored out of StyleResolver.
m_elementAffectedByClassRules = m_element && m_element->hasClass() && classNamesAffectedByRules(m_element->classNames());

// Check previous siblings and their cousins.
unsigned count = 0;
unsigned visitedNodeCount = 0;
@@ -457,6 +457,9 @@ class StyleResolver {
// the last reference to a style declaration are garbage collected.
void sweepMatchedPropertiesCache(Timer<StyleResolver>*);

bool classNamesAffectedByRules(const SpaceSplitString&) const;
bool sharingCandidateHasIdenticalStyleAffectingAttributes(StyledElement*) const;

unsigned m_matchedPropertiesCacheAdditionsSinceLastSweep;

typedef HashMap<unsigned, MatchedPropertiesCacheItem> MatchedPropertiesCache;
@@ -488,6 +491,7 @@ class StyleResolver {
StyledElement* m_styledElement;
RenderRegion* m_regionForStyling;
EInsideLink m_elementLinkState;
bool m_elementAffectedByClassRules;
ContainerNode* m_parentNode;
CSSValue* m_lineHeightValue;
bool m_fontDirty;

0 comments on commit 9f396cf

Please sign in to comment.