Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(288)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 772103002: Collect content:attr(...)-features in RuleFeatureSet. (Closed)

Created:
6 years ago by andersr
Modified:
6 years ago
Reviewers:
rune
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, dglazkov+blink, ed+blinkwatch_opera.com, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Collect content:attr(...)-features in RuleFeatureSet. We currently store the attribute names seen in content:attr(...) during style application as a special case, and then afterwards ensure that we have invalidation sets for those attributes. This is bad, because it's yet another special thing we have to do when resolving a style. This patch moves the collection of this feature to RuleFeatureSet, along with the rest of the feature collection. R=rune@opera.com TEST=fast/css/invalidation/content-attr.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186954

Patch Set 1 #

Patch Set 2 : Rebase. #

Patch Set 3 : Added TCs. #

Total comments: 4

Patch Set 4 : Avoid findPropertyIndex when possible. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -35 lines) Patch
A LayoutTests/fast/css/invalidation/content-attr-style-addition.html View 1 2 3 1 chunk +15 lines, -0 lines 0 comments Download
A + LayoutTests/fast/css/invalidation/content-attr-style-addition-expected.html View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A LayoutTests/fast/css/invalidation/content-attr-style-mutation.html View 1 2 3 1 chunk +13 lines, -0 lines 0 comments Download
A + LayoutTests/fast/css/invalidation/content-attr-style-mutation-expected.html View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSSelector.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/RuleFeature.h View 1 2 3 3 chunks +4 lines, -6 lines 0 comments Download
M Source/core/css/RuleFeature.cpp View 1 2 3 4 chunks +40 lines, -10 lines 0 comments Download
M Source/core/css/resolver/StyleBuilderCustom.cpp View 1 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 1 4 chunks +0 lines, -12 lines 0 comments Download
M Source/core/css/resolver/StyleResolverState.h View 1 2 chunks +0 lines, -4 lines 0 comments Download

Messages

Total messages: 10 (1 generated)
andersr
6 years ago (2014-12-02 16:32:02 UTC) #1
rune
This requires the StyleResolver to be rebuilt for changes like: <style> div::before { content: "X"; ...
6 years ago (2014-12-02 20:26:55 UTC) #2
andersr
On 2014/12/02 20:26:55, rune wrote: > This requires the StyleResolver to be rebuilt for changes ...
6 years ago (2014-12-10 12:31:50 UTC) #3
rune
https://codereview.chromium.org/772103002/diff/40001/LayoutTests/fast/css/invalidation/content-attr-style-addition.html File LayoutTests/fast/css/invalidation/content-attr-style-addition.html (right): https://codereview.chromium.org/772103002/diff/40001/LayoutTests/fast/css/invalidation/content-attr-style-addition.html#newcode2 LayoutTests/fast/css/invalidation/content-attr-style-addition.html:2: <head> Skip <head> https://codereview.chromium.org/772103002/diff/40001/LayoutTests/fast/css/invalidation/content-attr-style-addition.html#newcode9 LayoutTests/fast/css/invalidation/content-attr-style-addition.html:9: addEventListener("load", function(){ "onload = ...
6 years ago (2014-12-10 13:03:55 UTC) #4
rune
On 2014/12/10 at 13:03:55, rune wrote: > Will add "number" twice. Well, not add, but ...
6 years ago (2014-12-10 13:05:02 UTC) #5
andersr
On 2014/12/10 13:03:55, rune wrote: > https://codereview.chromium.org/772103002/diff/40001/LayoutTests/fast/css/invalidation/content-attr-style-addition.html > File LayoutTests/fast/css/invalidation/content-attr-style-addition.html (right): > > https://codereview.chromium.org/772103002/diff/40001/LayoutTests/fast/css/invalidation/content-attr-style-addition.html#newcode2 > ...
6 years ago (2014-12-11 10:59:09 UTC) #6
rune
lgtm
6 years ago (2014-12-11 12:51:14 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/772103002/60001
6 years ago (2014-12-11 13:05:08 UTC) #9
commit-bot: I haz the power
6 years ago (2014-12-11 15:26:31 UTC) #10
Message was sent while issue was closed. 
Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=186954

Powered by Google App Engine
This is Rietveld 408576698