[Webkit-unassigned] [Bug 76337] Cache CSSStyleSelector::Features in RuleSets

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 14 12:26:30 PST 2012


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





--- Comment #2 from Andreas Kling <kling at webkit.org>  2012-01-14 12:26:28 PST ---
(From update of attachment 122554)
View in context: https://bugs.webkit.org/attachment.cgi?id=122554&action=review

Dumping superficial comments since you have to reup anyway.

> Source/WebCore/ChangeLog:13
> +        This is 1-2% CPU time reduction (engadged, nytimes) due less time spent in feature collection.

Typo, engadget.

> Source/WebCore/ChangeLog:25
> +            Add a field far caching the features.

Typo, for.

> Source/WebCore/css/CSSStyleSelector.cpp:225
> +    void addRule(CSSStyleRule* , CSSSelector*);

Extra space after CSSStyleRule*.

> Source/WebCore/css/CSSStyleSelector.cpp:230
> +    

Extra whitespace.

> Source/WebCore/css/CSSStyleSelector.cpp:1950
> +    if (selector->m_match == CSSSelector::Id && !selector->value().isEmpty())

I suspect selector->value() can't be empty if selector->m_match == CSSSelector::Id.
No need to change that in this patch as you're just moving the code though.

> Source/WebCore/css/CSSStyleSelector.cpp:1980
> +                if (selector->isSiblingSelector())
> +                    foundSiblingSelector = true;

You could change this to:
if (!foundSiblingSelector && selector->isSiblingSelector())
To avoid wasting cycles after a first sibling selector is found.

> Source/WebCore/css/CSSStyleSelector.h:225
> +    struct RuleSelectorPair {
> +        RuleSelectorPair(CSSStyleRule* rule, CSSSelector* selector) : rule(rule), selector(selector) { }

Seems like we should either hide RuleSelectorPair() or make it initialize rule and selector as null pointers.

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



More information about the webkit-unassigned mailing list