Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2493003003:
Introduce CSS parser mode for distinguishing static/dynamic profile (Closed)
|
Created:
4 years, 1 month ago by kochi Modified:
4 years, 1 month ago CC:
chromium-reviews, blink-reviews-css, sof, eae+blinkwatch, blink-reviews-dom_chromium.org, dglazkov+blink, apavlov+blink_chromium.org, darktears, blink-reviews, rwlbuis, hayato, rune Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionIntroduce CSS parser mode for distinguishing static/dynamic profile
Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1]
to selectively enable or disable CSS selectors depending on the
performance characteristics that selectors require.
'>>>' (aka /deep/) is planned to be enabled under the experimental
flag in static profile [2]. In the future :has() pseudo class will
also be enabled in static profile.
Implementation for '>>>' combinator is done in later CLs.
[1] https://drafts.csswg.org/selectors/#profiles
[2] https://github.com/w3c/webcomponents/issues/78
BUG=633007
Committed: https://crrev.com/528e38c9ff62f9eee3ced46ba11f4450e2fd4616
Cr-Commit-Position: refs/heads/master@{#432133}
Patch Set 1 #Patch Set 2 : clean up #
Total comments: 4
Patch Set 3 : Address review comments #
Messages
Total messages: 29 (19 generated)
The CQ bit was checked by kochi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Description was changed from ========== Introduce CSS parser mode for distinguishing static/dynamic profile BUG= ========== to ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class also enabled in static profile as well. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ==========
Description was changed from ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class also enabled in static profile as well. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ========== to ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class will also be enabled in static profile. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ==========
The CQ bit was checked by kochi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class will also be enabled in static profile. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ========== to ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class will also be enabled in static profile. Implementation for '>>>' combinator is done in later CLs. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ==========
kochi@chromium.org changed reviewers: + timloh@chromium.org
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
rune@opera.com changed reviewers: + rune@opera.com
For consistency, I think the parseSelector calls in SelectorQueryTest should use the static profile. apart from that, lgtm, but wait from timloh@ to have a look.
lgtm https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/parser/CSSParserMode.cpp (right): https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/parser/CSSParserMode.cpp:78: CSSParserContext::CSSParserContext(const CSSParserContext& other, Technically we should update the copy ctor and operator== below (even though it shouldn't affect behaviour). https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/parser/CSSParserMode.h (right): https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/parser/CSSParserMode.h:89: enum Profile { DynamicProfile, StaticProfile }; SelectorProfile?
Thanks for reviews! Addressed all comments. https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/parser/CSSParserMode.cpp (right): https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/parser/CSSParserMode.cpp:78: CSSParserContext::CSSParserContext(const CSSParserContext& other, On 2016/11/15 02:27:07, Timothy Loh wrote: > Technically we should update the copy ctor and operator== below (even though it > shouldn't affect behaviour). Good catch, done. https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/parser/CSSParserMode.h (right): https://codereview.chromium.org/2493003003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/parser/CSSParserMode.h:89: enum Profile { DynamicProfile, StaticProfile }; On 2016/11/15 02:27:07, Timothy Loh wrote: > SelectorProfile? Done. SelectorProfile sounds better and clearer.
The CQ bit was checked by kochi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org, rune@opera.com Link to the patchset: https://codereview.chromium.org/2493003003/#ps40001 (title: "Address review comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
The CQ bit was checked by kochi@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class will also be enabled in static profile. Implementation for '>>>' combinator is done in later CLs. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ========== to ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class will also be enabled in static profile. Implementation for '>>>' combinator is done in later CLs. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class will also be enabled in static profile. Implementation for '>>>' combinator is done in later CLs. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 ========== to ========== Introduce CSS parser mode for distinguishing static/dynamic profile Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1] to selectively enable or disable CSS selectors depending on the performance characteristics that selectors require. '>>>' (aka /deep/) is planned to be enabled under the experimental flag in static profile [2]. In the future :has() pseudo class will also be enabled in static profile. Implementation for '>>>' combinator is done in later CLs. [1] https://drafts.csswg.org/selectors/#profiles [2] https://github.com/w3c/webcomponents/issues/78 BUG=633007 Committed: https://crrev.com/528e38c9ff62f9eee3ced46ba11f4450e2fd4616 Cr-Commit-Position: refs/heads/master@{#432133} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/528e38c9ff62f9eee3ced46ba11f4450e2fd4616 Cr-Commit-Position: refs/heads/master@{#432133} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
