[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