Refactor RuleFeatureSet to simplify iteration over CSS Selectors when collecting data.

Refactor RuleFeatureSet to simplify iteration over CSS Selectors when collecting data.

Make all callers use the public methods rather than poking data structures. Fix a
bug in descendant selector set matching along the way (was marking whole-tree
recalc needed for all class names, as opposed to the ones in the particular
rule).

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

[chrishtr, 2014-01-23 00:12:37]

Refactor RuleFeatureSet to simplify iteration over CSS Selectors when collecting
data.

Also change all call sites to use the public apis of RuleFeatureSet.

Fixes a bug in descendant invalidation set computation - only want to loop over
classes in the current rule.

https://codereview.chromium.org/143653010/diff/1/LayoutTests/fast/dom/shadow/...
File LayoutTests/fast/dom/shadow/ancestor-pseudo-class-expected.txt (right):

https://codereview.chromium.org/143653010/diff/1/LayoutTests/fast/dom/shadow/...
LayoutTests/fast/dom/shadow/ancestor-pseudo-class-expected.txt:35: PASS
backgroundColorOf('host') is "rgba(0, 0, 0, 0)"
I pinged tasak@ about this, crrev.com/138023007

[esprehn, 2014-01-23 01:13:28]

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

https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:136: m_metadata.add(metadata);
Doesn't this mean we need to do more hash table iterations and copying?

https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
File Source/core/css/RuleFeature.h (right):

https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.h:83: // Marks the given attribute name as
"appearing in a selector". Used for select:
select: ? I'm not sure what that is.

[chrishtr, 2014-01-23 01:20:56]

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

https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.cpp:136: m_metadata.add(metadata);
On 2014/01/23 01:13:29, esprehn wrote:
> Doesn't this mean we need to do more hash table iterations and copying?

Yes. It'll have to do twice as many operations as before. However, I will soon
be able to remove the redundant ones, since a descendant invalidation set will
have the same information about affected classe, ids and attrs, so we can get
rid of these hash sets.

https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
File Source/core/css/RuleFeature.h (right):

https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
Source/core/css/RuleFeature.h:83: // Marks the given attribute name as
"appearing in a selector". Used for select:
On 2014/01/23 01:13:29, esprehn wrote:
> select: ? I'm not sure what that is.

Fixed comment.

[esprehn, 2014-01-23 01:33:50]

On 2014/01/23 01:20:56, Chris Harrelson wrote:
>
https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
> File Source/core/css/RuleFeature.cpp (right):
> 
>
https://codereview.chromium.org/143653010/diff/50001/Source/core/css/RuleFeat...
> Source/core/css/RuleFeature.cpp:136: m_metadata.add(metadata);
> On 2014/01/23 01:13:29, esprehn wrote:
> > Doesn't this mean we need to do more hash table iterations and copying?
> 
> Yes. It'll have to do twice as many operations as before. However, I will soon
> be able to remove the redundant ones, since a descendant invalidation set will
> have the same information about affected classe, ids and attrs, so we can get
> rid of these hash sets.
> 


Okay, watch the perf bots.

[esprehn, 2014-01-23 01:33:58]

On 2014/01/23 01:33:50, esprehn wrote:
> ...
> 
> Okay, watch the perf bots.

lgtm

[commit-bot: I haz the power, 2014-01-23 01:36:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/chrishtr@chromium.org/143653010/120001

[commit-bot: I haz the power, 2014-01-23 02:28:08]

Retried try job too often on mac_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[commit-bot: I haz the power, 2014-01-23 22:36:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/chrishtr@chromium.org/143653010/380001

[commit-bot: I haz the power, 2014-01-24 01:25:26]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2014-01-24 17:50:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/chrishtr@chromium.org/143653010/500002

[commit-bot: I haz the power, 2014-01-24 18:55:19]

Change committed as 165771