Chaining multiple :nth-child() does not work properly
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Sep 2014 18:49:12 +0000 (18:49 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Sep 2014 18:49:12 +0000 (18:49 +0000)
commitffa02a91f688fcabbf7b734af57e1db210f8e233
treee94f8ae093583659af1534af63d2dbac6f56c8aa
parent9588e43067760dcddad63bb5f8deab1987c904aa
Chaining multiple :nth-child() does not work properly
https://bugs.webkit.org/show_bug.cgi?id=137032

Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-09-27
Reviewed by Gavin Barraclough.

Source/WebCore:

When multiple :nth-child() are chained, the evaluation of each "An+B" could depend on
the execution of the previous "An+B". The reason is that the register holding the position
of the current element could be modified by the evaluation of "An+B".

There are two cases in which the register was used as the destination of an operation:
1) When A and B are positive, the counter would be the destination of "counter - B".
2) When A is not 1 or 2, the modulo operation was not preserving the input register.

For (1), we a copy of the counter in that case of generateElementIsNthChild().

For (2), we also preserve a copy of the input if it is used by the operation. In this case,
if the input register is one of the argument we need for idiv, we preserve it on the stack
or in a register depending on what is available.

This increases the register requirements by 2 in the worst case on x86. The extra registers
can push generateElementIsNthChild() above the 4 available registers. To accomodate for that,
minimumRegisterRequirements() reserve more registers on x86.

The extra register pressure has strictly no effect on performance, x86_64 has 9 registers
available without pushing anything. The extra allocation is only necessary for debugging.

Tests: fast/selectors/nth-child-basics.html
       fast/selectors/nth-child-chained.html
       fast/selectors/nth-child-of-basics-2.html
       fast/selectors/nth-child-of-chained.html

* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::minimumRegisterRequirements):
(WebCore::SelectorCompiler::SelectorCodeGenerator::modulo):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsNthChild):

LayoutTests:

* fast/selectors/nth-child-chained-expected.txt: Added.
* fast/selectors/nth-child-chained.html: Added.
* fast/selectors/nth-child-of-chained-expected.txt: Added.
* fast/selectors/nth-child-of-chained.html: Added.
Those new tests target specifically the register reuse bug fixed by the patch.

* fast/selectors/nth-child-basics-expected.txt: Added.
* fast/selectors/nth-child-basics.html: Added.
* fast/selectors/nth-child-of-basics-2-expected.txt: Added.
* fast/selectors/nth-child-of-basics-2.html: Added.
Those tests add coverage for the examples used by http://nthmaster.com. This is to increase
the general test coverage.

I added nth-child-of-basics-2.html instead of extending nth-child-of-basics.html because
of the speed issue in debug without CSS JIT (otherwise the test can timeout).

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174035 268f45cc-cd09-0410-ab3c-d52691b4dbfc
LayoutTests/ChangeLog
LayoutTests/fast/selectors/nth-child-basics-expected.txt [new file with mode: 0644]
LayoutTests/fast/selectors/nth-child-basics.html [new file with mode: 0644]
LayoutTests/fast/selectors/nth-child-chained-expected.txt [new file with mode: 0644]
LayoutTests/fast/selectors/nth-child-chained.html [new file with mode: 0644]
LayoutTests/fast/selectors/nth-child-of-basics-2-expected.txt [new file with mode: 0644]
LayoutTests/fast/selectors/nth-child-of-basics-2.html [new file with mode: 0644]
LayoutTests/fast/selectors/nth-child-of-chained-expected.txt [new file with mode: 0644]
LayoutTests/fast/selectors/nth-child-of-chained.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/cssjit/SelectorCompiler.cpp