[webkit-reviews] review denied: [Bug 132319] CSS JIT: optimize direct / indirect adjacent's traversal backtracking : [Attachment 230473] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 30 20:30:13 PDT 2014
Benjamin Poulain <benjamin at webkit.org> has denied Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 132319: CSS JIT: optimize direct / indirect adjacent's traversal
backtracking
https://bugs.webkit.org/show_bug.cgi?id=132319
Attachment 230473: Patch
https://bugs.webkit.org/attachment.cgi?id=230473&action=review
------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=230473&action=review
The patch looks great.
The comments you have added on bugzilla are very useful. You should put those
explanation in the changelog too.
r- because I think the test needs to be extended. The patch looks correct to me
otherwise.
> Source/WebCore/cssjit/SelectorCompiler.cpp:610
> +static inline BacktrackingAction solveTraversalBacktrackingAction(const
SelectorFragment& fragment, bool hasDescendantRelationOnTheRight)
This is only for adjacent backtracking action, let's name this
solveAdjacentTraversalBacktrackingAction() for clarity.
> LayoutTests/fast/selectors/backtracking-adjacent.html:6
> +/* default */
You can remove this comment.
> LayoutTests/fast/selectors/backtracking-adjacent.html:113
> +description('The backtracking direct adjacent combinator with descendant
tail cases');
Hum, I am confused here, none of the cases tested needs a descendant tail.
I think what would make a good test here is cases of adjacent chain inside a
descendant chain where a descendant tail would have been needed before your
backtracking optimizations.
More information about the webkit-reviews
mailing list