[webkit-reviews] review granted: [Bug 126199] Use the Selector Code Generator for resolving style : [Attachment 221004] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 13 08:42:26 PST 2014


Ryosuke Niwa <rniwa at webkit.org> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 126199: Use the Selector Code Generator for resolving style
https://bugs.webkit.org/show_bug.cgi?id=126199

Attachment 221004: Patch
https://bugs.webkit.org/attachment.cgi?id=221004&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=221004&action=review


> Source/WebCore/css/ElementRuleCollector.cpp:312
> +	   JSC::VM* vm = document().scriptExecutionContext()->vm();

We should be using a reference instead in all these places (probably should be
done in a separate patch though).

> Source/WebCore/css/ElementRuleCollector.cpp:321
> +	       SelectorCompiler::SimpleSelectorChecker selectorChecker =
SelectorCompiler::simpleSelectorCheckerFunction(compiledSelectorChecker,
ruleData.compilationStatus);

Do we really need this temporary variable?  Also, isn't this the case where
auto will be useful?
Since this is a function pointer into a JIT'ed code, type doesn't give us any
extra information anyway.

> Source/WebCore/css/RuleSet.h:84
> +#if ENABLE(CSS_SELECTOR_JIT)
> +    mutable SelectorCompilationStatus compilationStatus;
> +    mutable JSC::MacroAssemblerCodeRef compiledSelectorCodeRef;
> +#endif // ENABLE(CSS_SELECTOR_JIT)
> +

These should really be private and have accessors instead.


More information about the webkit-reviews mailing list