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

Issue 383833002: All selectors in :-webkit-any must have an invalidation set feature. (Closed)

Created:
6 years, 5 months ago by rune
Modified:
6 years, 4 months ago
Reviewers:
chrishtr, esprehn
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, dglazkov+blink, ed+blinkwatch_opera.com, rwlbuis, rune+blink
Project:
blink
Visibility:
Public.

Description

All selectors in :-webkit-any must have an invalidation set feature. In order to avoid subtree style recalc for a :-webkit-any selector in the rightmost compound selector, all of its selectors must contain an invalidation set feature. In terms of invalidation sets, these selectors are all universal in the rightmost compound: .x :-webkit-any(*) .x :-webkit-any(.y, *) .x :-webkit-any(.y, :hover) The fix is to set foundIdent to true when all selectors in the selector list have found one. BUG=392852 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180566

Patch Set 1 #

Total comments: 2

Patch Set 2 : Removed condition from other patch #

Total comments: 2

Patch Set 3 : Documentation #

Total comments: 3

Patch Set 4 : More documentation #

Total comments: 3

Patch Set 5 : Rebased. #

Patch Set 6 : Avoid eventSender #

Unified diffs Side-by-side diffs Delta from patch set Stats (+84 lines, -5 lines) Patch
A LayoutTests/fast/css/invalidation/webkit-any-universal.html View 1 2 3 4 5 1 chunk +51 lines, -0 lines 0 comments Download
M Source/core/css/RuleFeature.cpp View 1 2 3 4 1 chunk +33 lines, -5 lines 0 comments Download

Messages

Total messages: 22 (0 generated)
rune
This CL is based on: https://codereview.chromium.org/365063005/
6 years, 5 months ago (2014-07-10 21:20:27 UTC) #1
rune
https://codereview.chromium.org/383833002/diff/1/Source/core/css/RuleFeature.cpp File Source/core/css/RuleFeature.cpp (right): https://codereview.chromium.org/383833002/diff/1/Source/core/css/RuleFeature.cpp#newcode138 Source/core/css/RuleFeature.cpp:138: if (hostMode == UseLocalStyleChange || component->pseudoType() == CSSSelector::PseudoNot) The ...
6 years, 5 months ago (2014-07-10 21:49:20 UTC) #2
rune
https://codereview.chromium.org/383833002/diff/1/Source/core/css/RuleFeature.cpp File Source/core/css/RuleFeature.cpp (right): https://codereview.chromium.org/383833002/diff/1/Source/core/css/RuleFeature.cpp#newcode138 Source/core/css/RuleFeature.cpp:138: if (hostMode == UseLocalStyleChange || component->pseudoType() == CSSSelector::PseudoNot) On ...
6 years, 5 months ago (2014-07-10 21:50:46 UTC) #3
chrishtr
https://codereview.chromium.org/383833002/diff/20001/Source/core/css/RuleFeature.cpp File Source/core/css/RuleFeature.cpp (right): https://codereview.chromium.org/383833002/diff/20001/Source/core/css/RuleFeature.cpp#newcode138 Source/core/css/RuleFeature.cpp:138: if (hostMode == UseLocalStyleChange) What does foundUniversal have to ...
6 years, 5 months ago (2014-07-10 23:26:49 UTC) #4
rune
https://codereview.chromium.org/383833002/diff/20001/Source/core/css/RuleFeature.cpp File Source/core/css/RuleFeature.cpp (right): https://codereview.chromium.org/383833002/diff/20001/Source/core/css/RuleFeature.cpp#newcode138 Source/core/css/RuleFeature.cpp:138: if (hostMode == UseLocalStyleChange) On 2014/07/10 23:26:49, chrishtr wrote: ...
6 years, 5 months ago (2014-07-11 08:45:14 UTC) #5
chrishtr
https://codereview.chromium.org/383833002/diff/30001/Source/core/css/RuleFeature.cpp File Source/core/css/RuleFeature.cpp (right): https://codereview.chromium.org/383833002/diff/30001/Source/core/css/RuleFeature.cpp#newcode148 Source/core/css/RuleFeature.cpp:148: return foundCombinator ? UseLocalStyleChange : UseSubtreeStyleChange; Please explain in ...
6 years, 5 months ago (2014-07-11 18:14:08 UTC) #6
rune
https://codereview.chromium.org/383833002/diff/30001/Source/core/css/RuleFeature.cpp File Source/core/css/RuleFeature.cpp (right): https://codereview.chromium.org/383833002/diff/30001/Source/core/css/RuleFeature.cpp#newcode148 Source/core/css/RuleFeature.cpp:148: return foundCombinator ? UseLocalStyleChange : UseSubtreeStyleChange; On 2014/07/11 18:14:08, ...
6 years, 5 months ago (2014-07-11 21:02:57 UTC) #7
rune
Added documentation, and a FIXME for investigating if UseSubtreeStyleChange is ever needed in rightmost compound. ...
6 years, 5 months ago (2014-07-11 21:49:37 UTC) #8
chrishtr
lgtm
6 years, 5 months ago (2014-07-12 00:06:38 UTC) #9
rune
@esprehn Needs owner review.
6 years, 4 months ago (2014-08-04 20:19:29 UTC) #10
rune
https://codereview.chromium.org/383833002/diff/50001/Source/core/css/RuleFeature.cpp File Source/core/css/RuleFeature.cpp (right): https://codereview.chromium.org/383833002/diff/50001/Source/core/css/RuleFeature.cpp#newcode153 Source/core/css/RuleFeature.cpp:153: // changing features when present in the rightmost compound ...
6 years, 4 months ago (2014-08-08 14:35:50 UTC) #11
rune
6 years, 4 months ago (2014-08-13 07:46:24 UTC) #12
esprehn
lgtm https://codereview.chromium.org/383833002/diff/50001/LayoutTests/fast/css/invalidation/webkit-any-universal.html File LayoutTests/fast/css/invalidation/webkit-any-universal.html (right): https://codereview.chromium.org/383833002/diff/50001/LayoutTests/fast/css/invalidation/webkit-any-universal.html#newcode43 LayoutTests/fast/css/invalidation/webkit-any-universal.html:43: assert_true(!!window.eventSender, "This test only works with eventSender present"); ...
6 years, 4 months ago (2014-08-16 07:55:10 UTC) #13
rune
The CQ bit was checked by rune@opera.com
6 years, 4 months ago (2014-08-19 13:14:19 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rune@opera.com/383833002/90001
6 years, 4 months ago (2014-08-19 13:14:29 UTC) #15
rune
https://codereview.chromium.org/383833002/diff/50001/LayoutTests/fast/css/invalidation/webkit-any-universal.html File LayoutTests/fast/css/invalidation/webkit-any-universal.html (right): https://codereview.chromium.org/383833002/diff/50001/LayoutTests/fast/css/invalidation/webkit-any-universal.html#newcode43 LayoutTests/fast/css/invalidation/webkit-any-universal.html:43: assert_true(!!window.eventSender, "This test only works with eventSender present"); On ...
6 years, 4 months ago (2014-08-19 13:14:30 UTC) #16
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_rel on tryserver.blink ...
6 years, 4 months ago (2014-08-19 14:14:30 UTC) #17
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-19 15:16:38 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23450)
6 years, 4 months ago (2014-08-19 15:16:40 UTC) #19
rune
The CQ bit was checked by rune@opera.com
6 years, 4 months ago (2014-08-19 15:22:45 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rune@opera.com/383833002/90001
6 years, 4 months ago (2014-08-19 15:23:22 UTC) #21
commit-bot: I haz the power
6 years, 4 months ago (2014-08-19 15:58:37 UTC) #22
Message was sent while issue was closed.
Committed patchset #6 (90001) as 180566

Powered by Google App Engine
This is Rietveld 408576698