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

Issue 652223002: Support style invalidation for ::content. (Closed)

Created:
6 years, 2 months ago by rune
Modified:
6 years, 2 months ago
Reviewers:
chrishtr, esprehn
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, ed+blinkwatch_opera.com, rwlbuis, rune+blink, sof, webcomponents-bugzilla_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Support style invalidation for ::content. Start supporting style invalidation for selectors containing ::content, but still use SubtreeStyleChange on distributed children of the content element. If you have a selector ".class ::content .target", we will not invalidate the whole subtree for the element matching ".class", but do style invalidation for elements matching ".target". We will not cross the insertion point during invalidation at this point, but merely mark content elements found during style invalidation with SubtreeStyleChange. That may be done in a separate patch, but it requires the style invalidator to have invalidation sets scheduled for elements inside a shadow tree affect style invalidation of its host subtree. The addition of :host-context as boundary-crossing and insertion-point- crossing is in preparation for supporting style invalidation for the :host-context selector. This CL is split out of [1]. This CL relies on [2]. [1] https://codereview.chromium.org/639433002/ [2] https://codereview.chromium.org/637273003/ R=esprehn@chromium.org,chrishtr@chromium.org BUG=424123 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183875

Patch Set 1 #

Total comments: 4

Patch Set 2 : Rebase and split #

Patch Set 3 : Fixed review issue #

Patch Set 4 : Missing removal of PseudoContent case when moving. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+132 lines, -3 lines) Patch
A LayoutTests/fast/css/invalidation/targeted-class-content-pseudo.html View 1 chunk +66 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/invalidation/targeted-class-content-pseudo-expected.txt View 1 chunk +18 lines, -0 lines 0 comments Download
M Source/core/css/CSSSelector.h View 2 chunks +13 lines, -1 line 0 comments Download
M Source/core/css/RuleFeature.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/css/RuleFeature.cpp View 1 2 3 4 chunks +6 lines, -2 lines 0 comments Download
M Source/core/css/invalidation/DescendantInvalidationSet.h View 2 chunks +6 lines, -0 lines 0 comments Download
M Source/core/css/invalidation/DescendantInvalidationSet.cpp View 4 chunks +7 lines, -0 lines 0 comments Download
M Source/core/css/invalidation/StyleInvalidator.h View 5 chunks +6 lines, -0 lines 0 comments Download
M Source/core/css/invalidation/StyleInvalidator.cpp View 1 2 3 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 19 (5 generated)
rune
PTAL
6 years, 2 months ago (2014-10-14 12:18:49 UTC) #1
chrishtr
(still processing the rest, sorry for the delay) https://codereview.chromium.org/652223002/diff/1/Source/core/dom/shadow/InsertionPoint.cpp File Source/core/dom/shadow/InsertionPoint.cpp (right): https://codereview.chromium.org/652223002/diff/1/Source/core/dom/shadow/InsertionPoint.cpp#newcode123 Source/core/dom/shadow/InsertionPoint.cpp:123: if ...
6 years, 2 months ago (2014-10-16 02:02:51 UTC) #2
esprehn
You should be able to do this simpler by just making ::content tree boundary crossing ...
6 years, 2 months ago (2014-10-16 02:19:06 UTC) #3
rune
https://codereview.chromium.org/652223002/diff/1/Source/core/dom/shadow/InsertionPoint.cpp File Source/core/dom/shadow/InsertionPoint.cpp (right): https://codereview.chromium.org/652223002/diff/1/Source/core/dom/shadow/InsertionPoint.cpp#newcode123 Source/core/dom/shadow/InsertionPoint.cpp:123: if (change < Inherit && styleChangeType() < SubtreeStyleChange) On ...
6 years, 2 months ago (2014-10-16 07:23:32 UTC) #4
rune
On 2014/10/16 07:23:32, rune wrote: > https://codereview.chromium.org/652223002/diff/1/Source/core/dom/shadow/InsertionPoint.cpp > File Source/core/dom/shadow/InsertionPoint.cpp (right): > > https://codereview.chromium.org/652223002/diff/1/Source/core/dom/shadow/InsertionPoint.cpp#newcode123 > ...
6 years, 2 months ago (2014-10-16 10:58:40 UTC) #5
rune
On 2014/10/16 02:19:06, esprehn wrote: > You should be able to do this simpler by ...
6 years, 2 months ago (2014-10-16 12:18:24 UTC) #6
rune
https://codereview.chromium.org/652223002/diff/1/Source/core/css/invalidation/StyleInvalidator.cpp File Source/core/css/invalidation/StyleInvalidator.cpp (right): https://codereview.chromium.org/652223002/diff/1/Source/core/css/invalidation/StyleInvalidator.cpp#newcode183 Source/core/css/invalidation/StyleInvalidator.cpp:183: for (size_t i = 0; i < insertionPoint.size(); ++i) ...
6 years, 2 months ago (2014-10-16 12:18:36 UTC) #7
chrishtr
lgtm
6 years, 2 months ago (2014-10-16 21:36:58 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/652223002/40001
6 years, 2 months ago (2014-10-17 07:06:54 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: mac_gpu_retina_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_triggered_tests/builds/59666)
6 years, 2 months ago (2014-10-17 07:56:30 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/652223002/40001
6 years, 2 months ago (2014-10-17 08:00:19 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: mac_gpu_retina_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_triggered_tests/builds/59666)
6 years, 2 months ago (2014-10-17 08:04:26 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/652223002/60001
6 years, 2 months ago (2014-10-17 08:35:52 UTC) #18
commit-bot: I haz the power
6 years, 2 months ago (2014-10-17 10:29:44 UTC) #19
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as 183875

Powered by Google App Engine
This is Rietveld 408576698