[Webkit-unassigned] [Bug 52370] Style sharing optimization no longer works on major web sites

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52370





--- Comment #8 from Simon Fraser (smfr) <simon.fraser at apple.com>  2011-01-14 14:39:43 PST ---
(From update of attachment 78990)
View in context: https://bugs.webkit.org/attachment.cgi?id=78990&action=review

> Source/WebCore/ChangeLog:25
> +            MatML default style sheet has sibling rules. Extract them and also

MathML

> Source/WebCore/ChangeLog:38
> +            Track sibling rules and ids used in the stylesheets to allow much more fine grained rejection of cases

fine-grained

> Source/WebCore/ChangeLog:51
> +            MatML default style sheet has sibling rules which don't get noticed by the normal document

MathML

> Source/WebCore/css/CSSStyleSelector.cpp:424
> +    void collectIdsAndSiblingRulesFromList(HashSet<AtomicStringImpl*>& ids, OwnPtr<CSSRuleSet>& siblingRules, CSSRuleDataList* rules);

Can CSSRuleDataList* be const CSSRuleDataList*? It's hard to see what the "out" params are otherwise. Maybe put 'rules' first?

> Source/WebCore/css/CSSStyleSelector.cpp:574
> +    // This is used in the style sharing code to check for rules that prevent sharing.

Not clear what "This" refers to.

> Source/WebCore/css/CSSStyleSelector.cpp:1127
> +            goto returnResult;

Ugh, goto. Can the code be factored with a new method or function to avoid this?

> Source/WebCore/css/CSSStyleSelector.cpp:3011
> +    return relation == CSSSelector::DirectAdjacent
> +        || relation == CSSSelector::IndirectAdjacent
> +        || type == CSSSelector::PseudoEmpty
> +        || type == CSSSelector::PseudoFirstChild
> +        || type == CSSSelector::PseudoLastChild
> +        || type == CSSSelector::PseudoOnlyChild
> +        || type == CSSSelector::PseudoOnlyOfType
> +        || type == CSSSelector::PseudoNthChild
> +        || type == CSSSelector::PseudoNthOfType
> +        || type == CSSSelector::PseudoNthLastChild
> +        || type == CSSSelector::PseudoNthLastOfType;
> +}

Style rules are to put || at the end of the previous line.

> Source/WebCore/css/CSSStyleSelector.cpp:3053
> +    AtomRuleMap::iterator end = m_idRules.end();
> +    for (AtomRuleMap::iterator it = m_idRules.begin(); it != end; ++it)
> +    end = m_classRules.end();
> +        collectIdsAndSiblingRulesFromList(ids, siblingRules, it->second);
> +    for (AtomRuleMap::iterator it = m_tagRules.begin(); it != end; ++it)
> +    for (AtomRuleMap::iterator it = m_pseudoRules.begin(); it != end; ++it)
> +    if (m_universalRules)
> +        collectIdsAndSiblingRulesFromList(ids, siblingRules, m_universalRules.get());
> +}
>  

Should be able to use const_iterators here. Can the whole method be const?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.