[cleanup] Simplify StyleDifference. [chromium/src : main]
0 views
Skip to first unread message
Ian Kilpatrick (Gerrit)
Feb 5, 2026, 5:46:01 AM (8 days ago) Feb 5
to Morten Stenshorne, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Morten Stenshorne
Ian Kilpatrick voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Morten Stenshorne
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Kilpatrick (Gerrit)
Feb 5, 2026, 6:01:18 AM (8 days ago) Feb 5
to Morten Stenshorne, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Morten Stenshorne
Ian Kilpatrick added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/core/style/style_difference_test.cc
Line 12, Patchset 7 (Parent):
TEST(StyleDifferenceTest, StreamOutputDefault) {Ian Kilpatrick . unresolved
Removed this test-suite as doesn't seem to provide much (if any) value for just debug logging.
Related details
Attention is currently required from:
- Morten Stenshorne
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Morten Stenshorne (Gerrit)
Feb 5, 2026, 6:31:45 AM (8 days ago) Feb 5
to Ian Kilpatrick, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Ian Kilpatrick
Morten Stenshorne added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 7 (Latest):
Morten Stenshorne . resolved
There are build failures. So I'll pause reviewing.
File third_party/blink/renderer/core/css/css_properties.json5
Line 386, Patchset 7 (Latest):
"transform",Morten Stenshorne . unresolved
Now there's transform-data, transform-property, and transform.
Does this naming make sense? Maybe transform-generic, for the sake of greppability, if nothing else.
Related details
Attention is currently required from:
- Ian Kilpatrick
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Morten Stenshorne (Gerrit)
Feb 5, 2026, 7:12:55 AM (8 days ago) Feb 5
to Ian Kilpatrick, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Ian Kilpatrick
Morten Stenshorne voted and added 2 comments![]( "Open in Gerrit")
Votes added by Morten Stenshorne
| Code-Review | +1 |
2 comments
Patchset-level comments
Morten Stenshorne . resolved
The rest looks fine!
File third_party/blink/renderer/core/css/css_properties.json5
Line 386, Patchset 7 (Latest):
"transform",Morten Stenshorne . unresolved
Now there's transform-data, transform-property, and transform.
Does this naming make sense? Maybe transform-generic, for the sake of greppability, if nothing else.
Morten Stenshorne
Or transform-common?
Related details
Attention is currently required from:
- Ian Kilpatrick
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Kilpatrick (Gerrit)
Feb 5, 2026, 7:20:34 AM (8 days ago) Feb 5
to Morten Stenshorne, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Morten Stenshorne
Ian Kilpatrick voted and added 3 comments![]( "Open in Gerrit")
Votes added by Ian Kilpatrick
| Commit-Queue | +1 |
3 comments
Patchset-level comments
File third_party/blink/renderer/core/css/css_properties.json5
Morten StenshorneNow there's transform-data, transform-property, and transform.
Does this naming make sense? Maybe transform-generic, for the sake of greppability, if nothing else.
Or transform-common?
Ian Kilpatrick
Ack. So the previously variant was needed, as we needed to track if *only* the transform property had changed. Switched to back to HEAD. See comment in computed_style.cc
File third_party/blink/renderer/core/style/computed_style.cc
Line 885, Patchset 8:
diff.transform_changed = true;Ian Kilpatrick . resolved
This was being used to indicate that *only* the transform property had changed (and nothing else). Rework to indicate that.
Related details
Attention is currently required from:
- Morten Stenshorne
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Morten Stenshorne (Gerrit)
Feb 5, 2026, 7:41:43 AM (8 days ago) Feb 5
to Ian Kilpatrick, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Ian Kilpatrick
Morten Stenshorne voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Ian Kilpatrick
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Kilpatrick (Gerrit)
Feb 5, 2026, 9:04:04 AM (8 days ago) Feb 5
to Morten Stenshorne, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Ian Kilpatrick voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ian Kilpatrick
| Commit-Queue | +2 |
1 comment
File third_party/blink/renderer/core/style/style_difference_test.cc
Line 12, Patchset 7 (Parent):
TEST(StyleDifferenceTest, StreamOutputDefault) {Ian Kilpatrick . resolved
Removed this test-suite as doesn't seem to provide much (if any) value for just debug logging.
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Feb 5, 2026, 9:41:43 AM (8 days ago) Feb 5
to Ian Kilpatrick, Morten Stenshorne, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
9 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/core/style/computed_style.cc
Insertions: 3, Deletions: 5.
@@ -847,11 +847,6 @@
if (field_diff & kFilterData) {
diff.filter_changed = true;
}
- if (field_diff & kHasTransform) {
- if (HasTransform() != other.HasTransform()) {
- diff.transform_changed = true;
- }
- }
if (field_diff & kMask) {
diff.mask_changed = true;
@@ -880,6 +875,9 @@
}
if (field_diff & kTransformOther) {
diff.transform_changed = true;
+ } else if ((field_diff & kHasTransform) &&
+ HasTransform() != other.HasTransform()) {
+ diff.transform_changed = true;
} else if (field_diff & kTransformProperty) {
diff.only_transform_property_changed = true;
diff.transform_changed = true;
```
Change information
Commit message:
[cleanup] Simplify StyleDifference.
If there was a choice to be made within StyleDifference, StyleDifference
did both.
For example:
- There is the "property_specific_differences_" bits, but single
properties were also split out into the bitfield, see:
"border_shape_changed_".
- "property_specific_differences_" wasn't only about single properties,
see: "kOtherTransformPropertyChanged".
- Using both "enum" and "enum class".
The setters/getters didn't provide much value for most things, just
expose the field directly for most things (except for paint/layout
invalidation which have a little bit more logic).
Other small cleanups include:
- Renaming NeedsPositionedMovementLayout to NeedsPositionedLayout.
- Adding "only_transform_property_changed" instead of
"OtherTransformPropertyChanged" which wasn't particularly clear
about what it was doing.
- Renaming "ScrollAnchorDisablingPropertyChanged" to
"disable_scroll_anchoring".
There should be no behaviour change.
Change-Id: Ibe1dd3570ac2a672866277d2224400b12404fd1f
Commit-Queue: Ian Kilpatrick <ikilp...@chromium.org>
Reviewed-by: Morten Stenshorne <mste...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1580073}
Files:
- M third_party/blink/renderer/core/layout/layout_block.cc
- M third_party/blink/renderer/core/layout/layout_block_flow_hot.cc
- M third_party/blink/renderer/core/layout/layout_box.cc
- M third_party/blink/renderer/core/layout/layout_box_model_object.cc
- M third_party/blink/renderer/core/layout/layout_inline.cc
- M third_party/blink/renderer/core/layout/layout_object.cc
- M third_party/blink/renderer/core/layout/layout_replaced.cc
- M third_party/blink/renderer/core/layout/layout_text.cc
- M third_party/blink/renderer/core/layout/svg/layout_svg_block.cc
- M third_party/blink/renderer/core/layout/svg/layout_svg_filter_primitive.cc
- M third_party/blink/renderer/core/layout/svg/layout_svg_model_object.cc
- M third_party/blink/renderer/core/layout/svg/layout_svg_resource_clipper.cc
- M third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.cc
- M third_party/blink/renderer/core/layout/svg/layout_svg_root.cc
- M third_party/blink/renderer/core/layout/svg/svg_resources.cc
- M third_party/blink/renderer/core/paint/paint_layer.cc
- M third_party/blink/renderer/core/style/build.gni
- M third_party/blink/renderer/core/style/computed_style.cc
- M third_party/blink/renderer/core/style/computed_style_test.cc
- M third_party/blink/renderer/core/style/style_difference.cc
- M third_party/blink/renderer/core/style/style_difference.h
- D third_party/blink/renderer/core/style/style_difference_test.cc
Change size: L
Delta: 22 files changed, 211 insertions(+), 409 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Morten Stenshorne
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Feb 5, 2026, 9:53:43 AM (8 days ago) Feb 5
to Chromium LUCI CQ, Ian Kilpatrick, Morten Stenshorne, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Stephen Chenney, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/speedometer3 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/12aee87b710000
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages