Don't add rule features across ::content.

Don't add rule features across ::content.

When we see a ::content selector, we mark the invalidation sets for the
selectors left of ::content as insertion-point-crossing. For such
invalidation sets, we mark insertion points for subtree style recalc,
which means that we don't need to look at the selector features right of
::content selectors for invalidations.

For instance for:

.a ::content .b .c

The invalidation set for '.b' contains '.c', and the invalidation set for
'.a' contains '.c' and has the insertion-point-crossing flag set. Adding
'c' is however unnecessary since ::content already causes a subtree style
recalc. Also, this may cause unnecessary invalidations in '.a's scope if
there are in-scope '.c' descendants of '.a'.

This CL avoids adding invalidation set features like '.c' to the
invalidation set for '.a' as illustrated above. Now invalidation sets
may have the insertion-point-crossing flag set while otherwise being
empty, and they should not be considered empty as we need to traverse
and mark all insertion points for such sets.

Committed: https://crrev.com/5324c1bd08a36f5cb1a6149882cd6ff1725491aa
Cr-Commit-Position: refs/heads/master@{#368064}

[rune, 2015-12-22 22:13:29]

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

[commit-bot: I haz the power, 2015-12-22 22:14:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1544893003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1544893003/1

[rune, 2015-12-22 22:14:45]

The CQ bit was unchecked by rune@opera.com

[rune, 2015-12-22 22:28:09]

Not correct when insertion-point-crossing is set by :host-context

[rune, 2015-12-22 22:56:37]

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

[commit-bot: I haz the power, 2015-12-22 22:57:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1544893003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1544893003/20001

[commit-bot: I haz the power, 2015-12-23 00:12:52]

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

[commit-bot: I haz the power, 2015-12-23 00:12:53]

Dry run: This issue passed the CQ dry run.

[rune, 2016-01-05 08:46:45]

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

[rune, 2016-01-05 08:46:45]

ptal

[Eric Willigers, 2016-01-07 03:50:01]

lgtm with nit

https://codereview.chromium.org/1544893003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp (right):

https://codereview.chromium.org/1544893003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp:116: void
expectNoClassInvalidation(const AtomicString& className, InvalidationSetVector&
invalidationSets)
This isn't being used.

Perhaps call it from a test that has ".a ::content .b" only?

[rune, 2016-01-07 09:55:18]

https://codereview.chromium.org/1544893003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp (right):

https://codereview.chromium.org/1544893003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp:116: void
expectNoClassInvalidation(const AtomicString& className, InvalidationSetVector&
invalidationSets)
On 2016/01/07 03:50:00, Eric Willigers wrote:
> This isn't being used.
> 
> Perhaps call it from a test that has ".a ::content .b" only?

The reason I didn't use it was because it will crash when there is no class set.
I would need another access method in InvalidationSet to test this.

Removing this.

[rune, 2016-01-07 09:55:35]

The CQ bit was checked by rune@opera.com

[rune, 2016-01-07 09:55:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
ericwilligers@chromium.org
Link to the patchset: https://codereview.chromium.org/1544893003/#ps60001
(title: "Remove unused method")

[commit-bot: I haz the power, 2016-01-07 09:55:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1544893003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1544893003/60001

[commit-bot: I haz the power, 2016-01-07 13:08:56]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-07 13:09:45]

Description was changed from

==========
Don't add rule features across ::content.

When we see a ::content selector, we mark the invalidation sets for the
selectors left of ::content as insertion-point-crossing. For such
invalidation sets, we mark insertion points for subtree style recalc,
which means that we don't need to look at the selector features right of
::content selectors for invalidations.

For instance for:

.a ::content .b .c

The invalidation set for '.b' contains '.c', and the invalidation set for
'.a' contains '.c' and has the insertion-point-crossing flag set. Adding
'c' is however unnecessary since ::content already causes a subtree style
recalc. Also, this may cause unnecessary invalidations in '.a's scope if
there are in-scope '.c' descendants of '.a'.

This CL avoids adding invalidation set features like '.c' to the
invalidation set for '.a' as illustrated above. Now invalidation sets
may have the insertion-point-crossing flag set while otherwise being
empty, and they should not be considered empty as we need to traverse
and mark all insertion points for such sets.
==========

to

==========
Don't add rule features across ::content.

When we see a ::content selector, we mark the invalidation sets for the
selectors left of ::content as insertion-point-crossing. For such
invalidation sets, we mark insertion points for subtree style recalc,
which means that we don't need to look at the selector features right of
::content selectors for invalidations.

For instance for:

.a ::content .b .c

The invalidation set for '.b' contains '.c', and the invalidation set for
'.a' contains '.c' and has the insertion-point-crossing flag set. Adding
'c' is however unnecessary since ::content already causes a subtree style
recalc. Also, this may cause unnecessary invalidations in '.a's scope if
there are in-scope '.c' descendants of '.a'.

This CL avoids adding invalidation set features like '.c' to the
invalidation set for '.a' as illustrated above. Now invalidation sets
may have the insertion-point-crossing flag set while otherwise being
empty, and they should not be considered empty as we need to traverse
and mark all insertion points for such sets.

Committed: https://crrev.com/5324c1bd08a36f5cb1a6149882cd6ff1725491aa
Cr-Commit-Position: refs/heads/master@{#368064}
==========

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

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