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

Issue 2528633003: Move MediaQueryResults to RuleFeatureSet. (Closed)

Created:
4 years ago by rune
Modified:
4 years ago
Reviewers:
meade_UTC10, nainar
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move MediaQueryResults to RuleFeatureSet. The existing code only cleared the query results on the StyleResolver when the StyleResolver was cleared. That meant we could end up in a situation where the result list was ever-growing. That wasn't a big issue in practice as the StyleResolver would be cleared quite often on stylesheet changes. However, that will change when the RuleSet based style invalidation is enabled. We move the media query results to RuleFeatureSet so that: - Results for @media rules are stored in RuleFeatureSet instead of RuleSet. - Results for media attributes are stored in the ScopedStyleResolver when added instead of appending them directly to StyleResolver. - Accumulated results for all scopes are stored in CSSGlobalRuleSet on StyleEngine instead of StyleResolver and are accumulated with other rule features in ScopedStyleResolver::collectFeaturesTo(). This CL introduces StyleEngine::ruleSetForSheet() for evaluating the media attribute of the stylesheet node and create the RuleSet if the media attribute matches. That way we are able to make the MediaQueryEvaluator private to StyleEngine. Also, this method is required when we start using ActiveStyleSheets. R=meade@chromium.org BUG=567021, 614026 Committed: https://crrev.com/4462ce68a6f0808d4f3d7e530b4920250d3675fd Cr-Commit-Position: refs/heads/master@{#434383}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Removed members. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+154 lines, -102 lines) Patch
M third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSStyleSheet.h View 3 chunks +10 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSStyleSheet.cpp View 2 chunks +12 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/RuleFeature.h View 3 chunks +16 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/RuleFeature.cpp View 3 chunks +8 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/RuleSet.h View 2 chunks +0 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/core/css/RuleSet.cpp View 3 chunks +6 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.h View 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.cpp View 5 chunks +27 lines, -17 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleResolver.h View 1 4 chunks +1 line, -14 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp View 4 chunks +1 line, -46 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.h View 3 chunks +9 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.cpp View 2 chunks +32 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngineTest.cpp View 1 chunk +24 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/FrameView.cpp View 1 chunk +3 lines, -4 lines 0 comments Download

Messages

Total messages: 29 (20 generated)
rune
https://codereview.chromium.org/2528633003/diff/1/third_party/WebKit/Source/core/css/resolver/StyleResolver.h File third_party/WebKit/Source/core/css/resolver/StyleResolver.h (right): https://codereview.chromium.org/2528633003/diff/1/third_party/WebKit/Source/core/css/resolver/StyleResolver.h#newcode278 third_party/WebKit/Source/core/css/resolver/StyleResolver.h:278: MediaQueryResultList m_deviceDependentMediaQueryResults; These should have been removed.
4 years ago (2016-11-23 13:37:42 UTC) #5
rune
https://codereview.chromium.org/2528633003/diff/1/third_party/WebKit/Source/core/css/resolver/StyleResolver.h File third_party/WebKit/Source/core/css/resolver/StyleResolver.h (right): https://codereview.chromium.org/2528633003/diff/1/third_party/WebKit/Source/core/css/resolver/StyleResolver.h#newcode278 third_party/WebKit/Source/core/css/resolver/StyleResolver.h:278: MediaQueryResultList m_deviceDependentMediaQueryResults; On 2016/11/23 13:37:42, rune wrote: > These ...
4 years ago (2016-11-23 13:46:54 UTC) #6
nainar
lgtm
4 years ago (2016-11-24 06:36:19 UTC) #16
meade_UTC10
lgtm
4 years ago (2016-11-24 06:36:27 UTC) #17
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/2528633003/20001
4 years ago (2016-11-24 09:54:47 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on ...
4 years ago (2016-11-24 11:55:41 UTC) #23
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/2528633003/20001
4 years ago (2016-11-24 18:19:30 UTC) #25
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years ago (2016-11-24 19:50:46 UTC) #27
commit-bot: I haz the power
4 years ago (2016-11-24 19:52:55 UTC) #29
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/4462ce68a6f0808d4f3d7e530b4920250d3675fd
Cr-Commit-Position: refs/heads/master@{#434383}

Powered by Google App Engine
This is Rietveld 408576698