Refactored invalidation set extraction.

Refactored invalidation set extraction.

Refactor into smaller more descriptive methods.

Implemented InvalidationSetFeatures::add() and hasFeatures() to add
features conditionally for selector lists like in :-webkit-any() instead
of tracking non-feature compounds using a foundFeatures variable.

Committed: https://crrev.com/272daf817262c3964a6ac5f51f2cf48fd1efd430
Cr-Commit-Position: refs/heads/master@{#416912}

[rune, 2016-09-01 09:47:25]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 09:47:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rune, 2016-09-01 09:48:14]

The CQ bit was unchecked by rune@opera.com

[rune, 2016-09-01 12:00:12]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 12:00:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 13:22:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 13:22:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rune, 2016-09-02 10:06:03]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-02 10:06:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rune, 2016-09-02 10:10:26]

rune@opera.com changed reviewers:
+ ericwilligers@chromium.org, esprehn@chromium.org

[rune, 2016-09-02 10:10:26]

Tried to refactor the invalidation set extraction code into smaller more
descriptive methods, and got rid of the 
foundFeatures return value.

updateFeaturesFromCombinator is quite complex. It's both setting flags and
updating the siblingFeatures. It's possible the code should've been split for
handling the combinator left of the rightmost compound and the rest.

Do you think this is moving in the right direction?

[commit-bot: I haz the power, 2016-09-02 11:53:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-02 11:53:35]

Dry run: This issue passed the CQ dry run.

[Eric Willigers, 2016-09-03 02:57:57]

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/RuleFeature.cpp (right):

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeature.cpp:428: // rightmost compound
selector (extractInvalidationSetFeatures). Secondly,
extractInvalidationSetFeatures becomes
extractInvalidationSetFeaturesFromCompound

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeature.cpp:655:
compoundHasTagIdClassOrAttribute |=
simpleSelector->isIdClassOrAttributeSelector();
tag? Should the CSSSelector method also consider tag?

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/RuleFeature.h (right):

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeature.h:155: void add(const
InvalidationSetFeatures& other);
+1

We have discussed in the past reducing the invalidation set for
.... a#c.e:hover { property: value; }
from
.... { a, #c, .e, :hover }
to
.... { #c }

Building on your recent -webkit-any patch, having InvalidationSetFeatures::add
will be useful when we want to reduce the invalidation set for
.... :-webkit-any(a#c.e:hover, b.f) { property: value; }
from
.... { a, b, #c, .e, .f, :hover }
to
.... { #c .f }

[rune, 2016-09-05 08:16:20]

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/RuleFeature.cpp (right):

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeature.cpp:428: // rightmost compound
selector (extractInvalidationSetFeatures). Secondly,
On 2016/09/03 02:57:57, Eric Willigers wrote:
> extractInvalidationSetFeatures becomes
> extractInvalidationSetFeaturesFromCompound

Done.

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeature.cpp:655:
compoundHasTagIdClassOrAttribute |=
simpleSelector->isIdClassOrAttributeSelector();
On 2016/09/03 02:57:57, Eric Willigers wrote:
> tag? Should the CSSSelector method also consider tag?

Then we would need to have invalidation sets for tags. We haven't had that since
an element cannot change tag name, but it would make sense for invalidations for
sibling mutations. This CL shouldn't cover that, though.

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/RuleFeature.h (right):

https://codereview.chromium.org/2305593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeature.h:155: void add(const
InvalidationSetFeatures& other);
On 2016/09/03 02:57:57, Eric Willigers wrote:
> +1
> 
> We have discussed in the past reducing the invalidation set for
> .... a#c.e:hover { property: value; }
> from
> .... { a, #c, .e, :hover }
> to
> .... { #c }
> 
> Building on your recent -webkit-any patch, having InvalidationSetFeatures::add
> will be useful when we want to reduce the invalidation set for
> .... :-webkit-any(a#c.e:hover, b.f) { property: value; }
> from
> .... { a, b, #c, .e, .f, :hover }
> to
> .... { #c .f }

Yes. I initially wanted to land this optimization, but chrishtr@ and esprehn@
thought the current way of adding all features was easier to understand.

[Eric Willigers, 2016-09-06 15:01:01]

lgtm

[rune, 2016-09-06 15:18:27]

esprehn@: since you triggered this refactor, I'll hold back landing to see if
you have any comments.

[rune, 2016-09-07 08:34:54]

The CQ bit was checked by rune@opera.com

[rune, 2016-09-07 08:34:59]

The CQ bit was unchecked by rune@opera.com

[rune, 2016-09-07 08:44:35]

Description was changed from

==========
Refactored invalidation set extraction.

Tried to refactor into smaller more descriptive methods. Not sure how
much better this is.
==========

to

==========
Refactored invalidation set extraction.

Refactor into smaller more descriptive methods.

Implemented InvalidationSetFeatures::add() and hasFeatures() to add
features conditionally for selector lists like in :-webkit-any() instead
of tracking non-feature compounds using a foundFeatures variable.
==========

[rune, 2016-09-07 08:45:52]

The CQ bit was checked by rune@opera.com

[commit-bot: I haz the power, 2016-09-07 08:46:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 11:51:38]

Description was changed from

==========
Refactored invalidation set extraction.

Refactor into smaller more descriptive methods.

Implemented InvalidationSetFeatures::add() and hasFeatures() to add
features conditionally for selector lists like in :-webkit-any() instead
of tracking non-feature compounds using a foundFeatures variable.
==========

to

==========
Refactored invalidation set extraction.

Refactor into smaller more descriptive methods.

Implemented InvalidationSetFeatures::add() and hasFeatures() to add
features conditionally for selector lists like in :-webkit-any() instead
of tracking non-feature compounds using a foundFeatures variable.
==========

[commit-bot: I haz the power, 2016-09-07 11:51:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-07 11:53:07]

Description was changed from

==========
Refactored invalidation set extraction.

Refactor into smaller more descriptive methods.

Implemented InvalidationSetFeatures::add() and hasFeatures() to add
features conditionally for selector lists like in :-webkit-any() instead
of tracking non-feature compounds using a foundFeatures variable.
==========

to

==========
Refactored invalidation set extraction.

Refactor into smaller more descriptive methods.

Implemented InvalidationSetFeatures::add() and hasFeatures() to add
features conditionally for selector lists like in :-webkit-any() instead
of tracking non-feature compounds using a foundFeatures variable.

Committed: https://crrev.com/272daf817262c3964a6ac5f51f2cf48fd1efd430
Cr-Commit-Position: refs/heads/master@{#416912}
==========

[commit-bot: I haz the power, 2016-09-07 11:53:08]

Patchset 4 (id:??) landed as
https://crrev.com/272daf817262c3964a6ac5f51f2cf48fd1efd430
Cr-Commit-Position: refs/heads/master@{#416912}