CSS: Avoid invalidating style when only descendants need updating

CSS: Avoid style recalc when only descendants need update

When we have a style rule of the form
.a > .b { ... }
and class 'a' is added to, or removed from, an element, we
have been forcing the style of the updated element to
become recalculated, just in case there was also a style
rule of the form
.a { ... }
or
.c > .a { ... }

We introduce a 'invalidatesSelf' bit in InvalidationSet,
to indicate if a given class/id/tag/pseudo-type (class
'a' in this example) ever appears in a rightmost position.


Discussed in Sibling Invalidation design doc https://goo.gl/z0Z9gn

BUG=532315
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=202449

[Eric Willigers, 2015-09-16 04:00:38]

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

[Eric Willigers, 2015-09-16 04:00:39]

This proposal can be accepted/rejected independently from sibling invalidation
CL 1317533002.

[Timothy Loh, 2015-09-16 04:30:49]

timloh@chromium.org changed reviewers:
+ timloh@chromium.org

[Timothy Loh, 2015-09-16 04:30:49]

lgtm, but wait for opinions from esprehn/rune

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
File Source/core/css/invalidation/InvalidationSet.h (right):

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
Source/core/css/invalidation/InvalidationSet.h:108: // Indicates the element
itself is invalid.
Maybe "must be invalidated" for consistency with other comments

[esprehn, 2015-09-16 04:34:04]

Your patch description is truncated.

[Eric Willigers, 2015-09-16 04:47:00]

On 2015/09/16 04:34:04, esprehn wrote:
> Your patch description is truncated.

Updated.

[rune, 2015-09-16 08:44:33]

lgtm with nit

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
File Source/core/css/invalidation/InvalidationSet.cpp (right):

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
Source/core/css/invalidation/InvalidationSet.cpp:103: setAppliesDirectly();
The existing code did an early return on wholeSubtreeInvalid() before copying
any other bits. Did you put this before on purpose? If not, you can move it down
where the other bits are copied.

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
File Source/core/css/invalidation/InvalidationSet.h (right):

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
Source/core/css/invalidation/InvalidationSet.h:109: unsigned m_appliesDirectly :
1;
I think there should be a better name, preferably something that includes
"invalidate", but I cannot come up with one that I'm satisfied with.

The other bits are also inconsistently named. We can fix later.

[esprehn, 2015-09-16 08:53:54]

I think I'd prefer something with "self" or "root". Like invslidatesSelf or
invalidateRoot.

[Eric Willigers, 2015-09-16 13:30:42]

On 2015/09/16 08:53:54, esprehn wrote:
> I think I'd prefer something with "self" or "root". Like invslidatesSelf or
> invalidateRoot.

Renamed to invalidatesSelf.

[Eric Willigers, 2015-09-16 13:32:09]

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
File Source/core/css/invalidation/InvalidationSet.cpp (right):

https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
Source/core/css/invalidation/InvalidationSet.cpp:103: setAppliesDirectly();
On 2015/09/16 08:44:32, rune wrote:
> The existing code did an early return on wholeSubtreeInvalid() before copying
> any other bits. Did you put this before on purpose? If not, you can move it
down
> where the other bits are copied.

Moved.  The original positioning, and the original naming, was motivated by
sibling invalidation: I was hoping to avoid allocating a sibling descendant
invalidation set if only the sibling itself is being invalidated. Not important:
it is clearer to instead have a sibling descendant invalidation set that is
empty and has 'invalidatesSelf'.

[rune, 2015-09-17 07:56:27]

On 2015/09/16 13:32:09, Eric Willigers wrote:
>
https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
> File Source/core/css/invalidation/InvalidationSet.cpp (right):
> 
>
https://codereview.chromium.org/1349543004/diff/1/Source/core/css/invalidatio...
> Source/core/css/invalidation/InvalidationSet.cpp:103: setAppliesDirectly();
> On 2015/09/16 08:44:32, rune wrote:
> > The existing code did an early return on wholeSubtreeInvalid() before
copying
> > any other bits. Did you put this before on purpose? If not, you can move it
> down
> > where the other bits are copied.
> 
> Moved.  The original positioning, and the original naming, was motivated by
> sibling invalidation: I was hoping to avoid allocating a sibling descendant
> invalidation set if only the sibling itself is being invalidated. Not
important:
> it is clearer to instead have a sibling descendant invalidation set that is
> empty and has 'invalidatesSelf'.

I think you may land this now.

[Eric Willigers, 2015-09-17 10:52:34]

The CQ bit was checked by ericwilligers@chromium.org

[Eric Willigers, 2015-09-17 10:52:34]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org,
rune@opera.com
Link to the patchset: https://codereview.chromium.org/1349543004/#ps20001
(title: "invalidatesSelf")

[commit-bot: I haz the power, 2015-09-17 10:52:46]

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

[commit-bot: I haz the power, 2015-09-17 12:02:20]

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

[commit-bot: I haz the power, 2015-09-17 12:02:21]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Eric Willigers, 2015-09-17 14:04:47]

The CQ bit was checked by ericwilligers@chromium.org

[commit-bot: I haz the power, 2015-09-17 14:05:00]

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

[commit-bot: I haz the power, 2015-09-17 14:09:56]

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