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

Issue 2557533005: Collect active stylesheets and and apply asynchronously. (Closed)

Created:
4 years ago by rune
Modified:
4 years ago
Reviewers:
meade_UTC10, esprehn, sashab
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-style_chromium.org, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, dglazkov+blink, eae+blinkwatch, Eric Willigers, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, rjwright, rwlbuis, shans, sof
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland: Collect active stylesheets and and apply asynchronously. This CL enables asynchronously updating the lists of active stylesheets applying any style changes using rule set invalidations. This means we more often avoid full style recalcs when we add or remove stylesheets from the document as well as when the evaluation of media queries change. In general, we now alway compare new and old stylesheets by comparing their rulesets and schedule style invalidations for removed and added rulesets. When media queries changes, we used to give completely in and recalculate all style once we discovered a media query changed its evaluation. With this patch, we clear rule sets for sheets which contain media queries which means we will invalidate rules for the sets before and after the query change. This can be further refined by only clearing rule sets when the sheets has a media query which actually did change evaluation, and also just schedule invalidations for rules which are inside @media rules. TreeScopeStyleSheetCollectionTest.cpp is removed as it is replaced by ActiveStyleSheetsTest.cpp which landed earlier. updateActiveStyle() has been added a few places where ensureStyleResolver() previously caused active stylesheets to be up-to- date. ensureStyleResolver() is now merely a method which creates the StyleResolver if necessary and returns it. There are some cleanups and code removal which needs to be done after this CL, but I have left those out to make this CL as small as possible. For instance resolverChanged(), which synchronously updated the active stylesheets, has an empty implementation instead of including a lot of removals in this CL. The code for lazy-appending stylesheets in StyleResolver is still there, but not in use. R=meade@chromium.org BUG=567021 Committed: https://crrev.com/9fb5b60edfb769134733009f9447bad3eaf347b0 Committed: https://crrev.com/90d4ea3d543f0031769b3aacac2d3e084b95fb7d Cr-Original-Commit-Position: refs/heads/master@{#438148} Cr-Commit-Position: refs/heads/master@{#439092}

Patch Set 1 #

Patch Set 2 : Rebased #

Patch Set 3 : [Mac] Missing style recalc for device scale change #

Total comments: 7

Patch Set 4 : Rebased #

Patch Set 5 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+142 lines, -588 lines) Patch
M third_party/WebKit/LayoutTests/animations/add-keyframes-in-shadow-recalc.html View 1 chunk +1 line, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/modify-stylesheet-minimal-recalc-style.html View 2 chunks +2 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/modify-stylesheet-minimal-recalc-style-expected.txt View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/BUILD.gn View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/FontFaceSet.cpp View 1 2 3 4 2 chunks +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/css/StyleSheetContents.cpp View 1 2 3 4 2 chunks +9 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 2 3 4 2 chunks +7 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DocumentStyleSheetCollection.h View 3 chunks +3 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DocumentStyleSheetCollection.cpp View 2 chunks +21 lines, -49 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DocumentStyleSheetCollector.h View 2 chunks +2 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DocumentStyleSheetCollector.cpp View 1 2 3 4 2 chunks +3 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ShadowTreeStyleSheetCollection.h View 3 chunks +2 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ShadowTreeStyleSheetCollection.cpp View 2 chunks +8 lines, -36 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.h View 1 2 3 8 chunks +6 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.cpp View 1 2 3 4 9 chunks +26 lines, -87 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngineTest.cpp View 1 2 3 2 chunks +2 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleSheetCollection.h View 4 chunks +4 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleSheetCollection.cpp View 1 2 3 4 2 chunks +4 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.h View 3 chunks +5 lines, -37 lines 0 comments Download
M third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.cpp View 1 2 chunks +19 lines, -127 lines 0 comments Download
D third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollectionTest.cpp View 1 chunk +0 lines, -152 lines 0 comments Download
M third_party/WebKit/Source/core/frame/FrameView.cpp View 1 2 3 4 3 chunks +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/frame/LocalFrame.cpp View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/inspector/InspectorCSSAgent.cpp View 1 2 3 1 chunk +3 lines, -6 lines 0 comments Download

Messages

Total messages: 53 (33 generated)
rune
This is the CL that enables async stylesheet update. After landing various other pieces it's ...
4 years ago (2016-12-08 23:48:38 UTC) #13
rune
This is the CL that enables async stylesheet update. After landing various other pieces it's ...
4 years ago (2016-12-08 23:48:39 UTC) #14
esprehn
This looks amazing, but all your bots are red? Btw I'm curious what cases we ...
4 years ago (2016-12-09 01:36:21 UTC) #17
meade_UTC10
On 2016/12/09 01:36:21, esprehn wrote: > This looks amazing, but all your bots are red? ...
4 years ago (2016-12-09 02:45:34 UTC) #18
rune
https://codereview.chromium.org/2557533005/diff/40001/third_party/WebKit/Source/core/css/StyleSheetContents.cpp File third_party/WebKit/Source/core/css/StyleSheetContents.cpp (right): https://codereview.chromium.org/2557533005/diff/40001/third_party/WebKit/Source/core/css/StyleSheetContents.cpp#newcode628 third_party/WebKit/Source/core/css/StyleSheetContents.cpp:628: document->styleEngine().setNeedsActiveStyleUpdate(node->treeScope()); On 2016/12/09 01:36:21, esprehn wrote: > Should this ...
4 years ago (2016-12-09 08:55:50 UTC) #23
meade_UTC10
I'm OOO this week. Sasha, would you mind taking a look? You're already on the ...
4 years ago (2016-12-12 09:50:37 UTC) #24
rune
On 2016/12/12 09:50:37, Eddy (OOO until 19 Dec) wrote: > I'm OOO this week. Sasha, ...
4 years ago (2016-12-12 10:02:41 UTC) #25
esprehn
lgtm
4 years ago (2016-12-12 22:24:30 UTC) #26
rune
On 2016/12/12 22:24:30, esprehn wrote: > lgtm thanks! after running some blink performance tests locally ...
4 years ago (2016-12-13 08:20:23 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2557533005/40001
4 years ago (2016-12-13 08:22:43 UTC) #29
commit-bot: I haz the power
Failed to apply patch for third_party/WebKit/Source/core/BUILD.gn: While running git apply --index -p1; error: patch failed: ...
4 years ago (2016-12-13 10:03:01 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2557533005/60001
4 years ago (2016-12-13 10:11:33 UTC) #34
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years ago (2016-12-13 13:03:49 UTC) #37
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/9fb5b60edfb769134733009f9447bad3eaf347b0 Cr-Commit-Position: refs/heads/master@{#438148}
4 years ago (2016-12-13 13:05:41 UTC) #39
paulmeyer
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2572473006/ by paulmeyer@chromium.org. ...
4 years ago (2016-12-13 20:52:31 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2557533005/60001
4 years ago (2016-12-16 10:01:56 UTC) #43
commit-bot: I haz the power
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/124212) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
4 years ago (2016-12-16 10:03:43 UTC) #45
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2557533005/80001
4 years ago (2016-12-16 10:43:14 UTC) #48
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years ago (2016-12-16 12:23:00 UTC) #51
commit-bot: I haz the power
4 years ago (2016-12-16 12:25:21 UTC) #53
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/90d4ea3d543f0031769b3aacac2d3e084b95fb7d
Cr-Commit-Position: refs/heads/master@{#439092}

Powered by Google App Engine
This is Rietveld 408576698