Single code path for updating invalidation sets.

Single code path for updating invalidation sets.

Allow all selectors to update invalidation sets using updateInvalidationSets
and never fall back to a separate wholeSubtree loop in
collectFeaturesFromSelector. The method for finding the InvalidationSetMode
is dropped completely.

With this change we are now at a point were a whole subtree invalidation is
only necessary for these cases:

1. There are no recognized features in the rightmost compound selector

 - A universal selector: ".a *"
 - Only contains pseudo classes or elements that are not considered
   features:

   ".a ::before", ".a :first-child", etc.

2. The invalidation set component is immediately left of an adjacent
   combinator:

   ".a + .b", ".a ~ .b"

3. The selector has a ::first-letter or ::first-line pseudo

   These are currently depending on style recalc or layout below the
   element they are pseudo elements for.

3. There is a :host-context() in the rightmost compound selector

   :host-context sub-selectors match ascendants of the :host element for
   which we are resolving style, so ":host-context(.a)" is effectively the
   same as ".a :host" which falls under no recognized features in the
   rightmost compound in point 1.

Points 1, 3, and 4 are now handled through
InvalidationSetFeatures::forceSubtree set as a result of feature extraction.
Point 2 is handled by updating InvalidationSetFeatures::adjacent as we
progress past selector combinators.

RuleFeaturesSet::collectFeaturesFromSelector is now back to how it was
before we introduced invalidation sets (without the feature hash sets).
Now, it only counts adjacent combinators and sets a flag when finding
::first-line, so it is possible to squash it into the same method as
updateInvalidationSets() to have a single pass over the selector when
finding rule features (if we want to do that).

The lowered expected node count in the :host-context test is due to the
fact that we only fall back to subtree invalidation when present in the
rightmost compound. ::first-letter/::first-line are always in the rightmost
compound, and :host-context() doesn't need subtree invalidation when in
other compounds. ":host-context(.a) .b" is effectively ".a :host .b".

R=chrishtr@chromium.org
BUG=335247
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185354

[rune, 2014-11-13 16:43:16]

Failure in last try-job doesn't look like my fault. Ready for review.

[chrishtr, 2014-11-13 23:01:11]

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
File Source/core/css/RuleFeature.cpp (right):

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:171: || pseudo == CSSSelector::PseudoHostContext
This one is sort of unrelated to the main thrust of this CL, right?

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:214: if (selector.match() == CSSSelector::Tag &&
selector.tagQName().localName() != starAtom)
Why this change?

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:226: return true;
Is this code reachable?

[rune, 2014-11-14 00:04:54]

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
File Source/core/css/RuleFeature.cpp (right):

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:171: || pseudo == CSSSelector::PseudoHostContext
On 2014/11/13 23:01:11, chrishtr wrote:
> This one is sort of unrelated to the main thrust of this CL, right?

This is what's described in the last section of the CL description about not
requiring subtree invalidation for :host-context presence in other compound
selectors than the rightmost one. Introducing a single pass through the
selector, which is the main thing with this CL, makes it hard if there are
selectors in the non-rightmost-compound that can trigger subtree recalcs because
we then would've already started to add features to invalidation sets before
discovering that we should've set wholeSubtree instead. It's possible to do the
allow :host-context part first. That would mean modifying the
InvalidationSetMode code to do so before introducing this CL which will just
nuke that again.

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:214: if (selector.match() == CSSSelector::Tag &&
selector.tagQName().localName() != starAtom)
On 2014/11/13 23:01:11, chrishtr wrote:
> Why this change?

It's to get the return value correct, and avoid using "*" as a tag name.

The universal selector is not a feature like a tag name is. Alternatively, I
could've done:

if (selector.match() == CSSSelector::Tag) {
    features.tagName = selector.tagQName().localName();
    return features.tagName != starAtom;
}

to not change the behavior, but before this change, and with the alternative
above, we would unnecessarily add "*" as a tag name to the invalidation set hash
map for tag names. This should be an improvement.

https://codereview.chromium.org/726503003/diff/40001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:226: return true;
On 2014/11/13 23:01:11, chrishtr wrote:
> Is this code reachable?

Yes, it's the return value for all but the last else.

[chrishtr, 2014-11-14 01:07:14]

lgtm

[rune, 2014-11-14 06:43:55]

The CQ bit was checked by rune@opera.com

[commit-bot: I haz the power, 2014-11-14 06:44:06]

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

[commit-bot: I haz the power, 2014-11-14 08:23:41]

Committed patchset #3 (id:40001) as 185354