Don't cache nth-indices of pseudo elements.

Don't cache nth-indices of pseudo elements.

A pseudo element is not found in its parent's child list. That caused an
assert while trying to cache the nth-index for a ::before element while
matching style for the ::before element's scrollbar pseudo elements.
Instead, skip the caching part and traverse siblings to fall back to the
behavior we had before the NthIndexCache.

Trying to get custom scrollbar style for pseudo elements with scrolling
overflow will not work, but never did. It's not currently possible to
match ::before::-webkit-scrollbar, and a universal ::-webkit-scrollbar
will not match scrollbars in pseudo elements like ::before, either.

R=esprehn@chromium.org
BUG=481495
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194728

[rune, 2015-04-29 10:43:22]

PTAL

[esprehn, 2015-04-30 01:03:52]

lgtm, but it'd be nicer to move the condition to a separate one.

https://codereview.chromium.org/1108783002/diff/1/Source/core/dom/NthIndexCac...
File Source/core/dom/NthIndexCache.h (right):

https://codereview.chromium.org/1108783002/diff/1/Source/core/dom/NthIndexCac...
Source/core/dom/NthIndexCache.h:63: for (Element* sibling = &element; sibling;
sibling = ElementTraversal::previousSibling(*sibling), index++) {
Why go through this loop for pseudo elements? I know it just aborts right away
but that's not obvious.

Could we do:

if (element.isPseudoElement())
  return 0;

before the if (!m_count) statement instead?

[rune, 2015-04-30 08:22:01]

On 2015/04/30 01:03:52, esprehn wrote:
> lgtm, but it'd be nicer to move the condition to a separate one.
> 
>
https://codereview.chromium.org/1108783002/diff/1/Source/core/dom/NthIndexCac...
> File Source/core/dom/NthIndexCache.h (right):
> 
>
https://codereview.chromium.org/1108783002/diff/1/Source/core/dom/NthIndexCac...
> Source/core/dom/NthIndexCache.h:63: for (Element* sibling = &element; sibling;
> sibling = ElementTraversal::previousSibling(*sibling), index++) {
> Why go through this loop for pseudo elements? I know it just aborts right away
> but that's not obvious.
> 
> Could we do:
> 
> if (element.isPseudoElement())
>   return 0;
> 
> before the if (!m_count) statement instead?

Done. Returned 1, though, because the nth-indices are 1-based here. I had to do
the same for nthLastIndex, since it uses m_count which is not valid for pseudo
elements.

[rune, 2015-04-30 08:22:23]

The CQ bit was checked by rune@opera.com

[rune, 2015-04-30 08:22:23]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1108783002/#ps20001
(title: "Early return for pseudo elements in NthIndexCache.")

[commit-bot: I haz the power, 2015-04-30 08:22:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1108783002/20001

[commit-bot: I haz the power, 2015-04-30 11:08:28]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194728