Do not add features to classes in rightmost compound selector.

Do not add features to classes in rightmost compound selector.

Features found in the rightmost compound selector should always have empty
invalidation sets. Their invalidation set should not contain itself or any
other features found in the same compound selector.

No need to look for descendants with classes 'a' or 'b' when changing 'a'
or 'b' on elements for the following selectors:

.a {}
.a.b {}
.x .a {}
.x .a.b {}

R=esprehn@chromium.org, chrishtr@chromium.org
BUG=345238
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167674

[chrishtr, 2014-02-20 18:56:30]

lgtm

[esprehn, 2014-02-21 04:33:46]

https://codereview.chromium.org/171513013/diff/1/Source/core/css/RuleFeature.cpp
File Source/core/css/RuleFeature.cpp (right):

https://codereview.chromium.org/171513013/diff/1/Source/core/css/RuleFeature....
Source/core/css/RuleFeature.cpp:120: continue;
I think you want to do this as two loops like before. The first loop is over the
right most compound selector.

[esprehn, 2014-02-21 04:38:54]

On 2014/02/21 04:33:46, esprehn wrote:
>
https://codereview.chromium.org/171513013/diff/1/Source/core/css/RuleFeature.cpp
> File Source/core/css/RuleFeature.cpp (right):
> 
>
https://codereview.chromium.org/171513013/diff/1/Source/core/css/RuleFeature....
> Source/core/css/RuleFeature.cpp:120: continue;
> I think you want to do this as two loops like before. The first loop is over
the
> right most compound selector.

See also my original design here:
https://codereview.chromium.org/129633003/#ps1

The second loop starts from the thing _after_ the right most selector. When
chrishtr changed my code he made it start from the beginning again which I think
is the issue.

You want to switch back to the way the loops were before and then between the
two loops do:

if (!selector) {
  for (class in classes)
    ensureDescendantInvalidationSet(class);
}

[esprehn, 2014-02-21 04:41:25]

On 2014/02/21 04:38:54, esprehn wrote:
> On 2014/02/21 04:33:46, esprehn wrote:
> > ...
> 
> if (!selector) {
>   for (class in classes)
>     ensureDescendantInvalidationSet(class);
> }

Err, you don't even need that, just add the ensure*() to the first loop when we
collect classes. :)

[rune, 2014-02-21 10:17:16]

https://codereview.chromium.org/171513013/diff/1/Source/core/css/RuleFeature.cpp
File Source/core/css/RuleFeature.cpp (right):

https://codereview.chromium.org/171513013/diff/1/Source/core/css/RuleFeature....
Source/core/css/RuleFeature.cpp:120: continue;
On 2014/02/21 04:33:46, esprehn wrote:
> I think you want to do this as two loops like before. The first loop is over
the
> right most compound selector.

Done that now. I did a break on !SubSelector instead of duplicating the code for
extract* and ensure*. I do a null check on the selector pointer now. If you
think it's safe enough to assume the leftmost selector will always be a
descendant combinator flag, I can change back to not checking and add an assert
instead.

[esprehn, 2014-02-24 05:55:17]

lgtm, thanks!

[rune, 2014-02-24 07:07:12]

The CQ bit was checked by rune@opera.com

[commit-bot: I haz the power, 2014-02-24 07:07:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/171513013/100001

[commit-bot: I haz the power, 2014-02-24 10:21:47]

Change committed as 167674