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

Issue 1644543002: Moved element style recalc count and stats to StyleEngine. (Closed)

Created:
4 years, 11 months ago by rune
Modified:
4 years, 10 months ago
Reviewers:
dstockwell
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, eric.carlson_apple.com, feature-media-reviews_chromium.org, fs, gasubic, mlamouri+watch-blink_chromium.org, philipj_slow, rwlbuis, sof, nessy, vcarbune.chromium
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Moved element style recalc count and stats to StyleEngine. There are two element style recalc counters, one in Document, and one in StyleResolver. The one in StyleResolver includes pseudo elements in that count. We are unifying thesie counts keeping pseudo elements included which means that the element recalc count will be higher for tracing and the inspector, but unchanged for internals.updateStyleAndReturnAffectedElementCount(). Also the StyleResolverStats is moved from StyleResolver to StyleEngine. This means the stats will survive a clearResolver(). This change is motivated around item 6 and 9 in the design document for crbug.com/401359. R=dstockwell@chromium.org BUG=401359 Committed: https://crrev.com/d39fd545c08c6e83fe2de7e1134d0f75f10c0b25 Cr-Commit-Position: refs/heads/master@{#372045}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+103 lines, -121 lines) Patch
M third_party/WebKit/Source/core/css/AffectedByFocusTest.cpp View 4 chunks +12 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/css/DragUpdateTest.cpp View 3 chunks +9 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/core/css/ElementRuleCollector.cpp View 2 chunks +8 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp View 3 chunks +8 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleResolver.h View 3 chunks +0 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp View 9 chunks +11 lines, -27 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleResolverStats.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/Document.h View 2 chunks +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 6 chunks +11 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Element.cpp View 2 chunks +2 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.h View 6 chunks +19 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.cpp View 3 chunks +12 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp View 1 chunk +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/testing/Internals.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebDocumentTest.cpp View 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1644543002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1644543002/1
4 years, 11 months ago (2016-01-27 10:42:00 UTC) #2
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 11 months ago (2016-01-27 12:06:46 UTC) #4
rune
ptal
4 years, 10 months ago (2016-01-27 21:43:00 UTC) #5
dstockwell
lgtm I suppose we may want to rename StyleResolverStats in the future.
4 years, 10 months ago (2016-01-28 05:35:35 UTC) #6
rune
On 2016/01/28 05:35:35, dstockwell wrote: > lgtm > > I suppose we may want to ...
4 years, 10 months ago (2016-01-28 07:54:18 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1644543002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1644543002/1
4 years, 10 months ago (2016-01-28 07:54:37 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-01-28 08:00:51 UTC) #10
commit-bot: I haz the power
4 years, 10 months ago (2016-01-28 08:01:47 UTC) #12
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/d39fd545c08c6e83fe2de7e1134d0f75f10c0b25
Cr-Commit-Position: refs/heads/master@{#372045}

Powered by Google App Engine
This is Rietveld 408576698