Check negative margin sizes for min-content while computing indent position [chromium/src : main]
0 views
Skip to first unread message
Nishaanth Shriram M S (Gerrit)
Mar 26, 2026, 8:15:57 AMMar 26
to Fredrik Söderquist, Kent Tamura, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Kent Tamura
Nishaanth Shriram M S voted![]( "Open in Gerrit")
| Auto-Submit | +1 |
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Fredrik Söderquist
- Kent Tamura
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fredrik Söderquist (Gerrit)
Mar 26, 2026, 8:38:48 AMMar 26
to Nishaanth Shriram M S, Koji Ishii, Kent Tamura, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Kent Tamura, Koji Ishii and Nishaanth Shriram M S
Fredrik Söderquist added 1 comment![]( "Open in Gerrit")
Commit Message
Line 9, Patchset 4 (Latest):
Bug: Using negative margins can offset the indent position which isn't checked in debug builds
Change-Id: Id954d9463407ef4f858005026692398fdd91b296
Failing DCHECK:
DCHECK_EQ(should_break_at_first_opportunity,
is_at_mid_word ||
(has_cloned_box_decorations_ &&
cloned_box_decorations_initial_size_ > LayoutUnit()));
How to produce the issue: In debug/component build and visit accounts.zoho.comFredrik Söderquist . unresolved
This commit message looks supremely weird. Let's keep the `Change-Id` as the last line. I'd also suggest actually filing a bug. otherwise drop the `Bug:` directive.
Related details
Attention is currently required from:
- Kent Tamura
- Koji Ishii
- Nishaanth Shriram M S
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. |
Kent Tamura (Gerrit)
Mar 26, 2026, 7:51:21 PMMar 26
to Nishaanth Shriram M S, Koji Ishii, Fredrik Söderquist, Kent Tamura, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Koji Ishii and Nishaanth Shriram M S
Kent Tamura added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
Nishaanth Shriram M S (Gerrit)
Mar 27, 2026, 1:07:25 AMMar 27
to Koji Ishii, Fredrik Söderquist, Kent Tamura, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Koji Ishii
Nishaanth Shriram M S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Auto-Submit | +1 |
1 comment
Commit Message
Line 9, Patchset 4:
Bug: Using negative margins can offset the indent position which isn't checked in debug builds
Change-Id: Id954d9463407ef4f858005026692398fdd91b296
Failing DCHECK:
DCHECK_EQ(should_break_at_first_opportunity,
is_at_mid_word ||
(has_cloned_box_decorations_ &&
cloned_box_decorations_initial_size_ > LayoutUnit()));
How to produce the issue: In debug/component build and visit accounts.zoho.comFredrik Söderquist . resolved
This commit message looks supremely weird. Let's keep the `Change-Id` as the last line. I'd also suggest actually filing a bug. otherwise drop the `Bug:` directive.
Nishaanth Shriram M S
Sorry about that, I edited the message in the end.
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
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. |
Nishaanth Shriram M S (Gerrit)
Mar 27, 2026, 1:09:26 AMMar 27
to Koji Ishii, Fredrik Söderquist, Kent Tamura, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist, Kent Tamura and Koji Ishii
Nishaanth Shriram M S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Auto-Submit | +1 |
| Commit-Queue | +2 |
1 comment
Patchset-level comments
File-level comment, Patchset 4:
Kent Tamura . resolved
Nishaanth Shriram M SPlease add a test.
Added it. Thanks! Do you normally run tests with EXPENSIVE_DCHECKS on?
Related details
Attention is currently required from:
- Fredrik Söderquist
- Kent Tamura
- Koji Ishii
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nishaanth Shriram M S (Gerrit)
Mar 27, 2026, 3:56:37 AMMar 27
to Kent Tamura, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Koji Ishii
Nishaanth Shriram M S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Auto-Submit | +1 |
1 comment
Commit Message
Line 9, Patchset 4:
Bug: Using negative margins can offset the indent position which isn't checked in debug builds
Change-Id: Id954d9463407ef4f858005026692398fdd91b296
Failing DCHECK:
DCHECK_EQ(should_break_at_first_opportunity,
is_at_mid_word ||
(has_cloned_box_decorations_ &&
cloned_box_decorations_initial_size_ > LayoutUnit()));
How to produce the issue: In debug/component build and visit accounts.zoho.comFredrik Söderquist . resolved
Nishaanth Shriram M SThis commit message looks supremely weird. Let's keep the `Change-Id` as the last line. I'd also suggest actually filing a bug. otherwise drop the `Bug:` directive.
Sorry about that, I edited the message in the end.
Nishaanth Shriram M S
I've filed a bug and linked it. https://issues.chromium.org/issues/496807859
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Koji Ishii (Gerrit)
Mar 27, 2026, 7:08:44 AMMar 27
to Nishaanth Shriram M S, Kent Tamura, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Nishaanth Shriram M S
Koji Ishii added 4 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 10 (Latest):
Koji Ishii . resolved
Thanks for finding this issue, filing it, and trying to fix. Please see inline:
Commit Message
Line 9, Patchset 10 (Latest):
Bug: https://issues.chromium.org/issues/496807859Koji Ishii . unresolved
Can you change this line to:
```
Bug: 496807859
```
File third_party/blink/renderer/core/layout/inline/line_breaker.cc
Line 1846, Patchset 10 (Latest):
} else if (line_info->Results().size() > 1) [[unlikely]] {Koji Ishii . unresolved
Hmm...this is quite a common case, no? We will run into this code path for:
```html
<span><span>text</span></span>
```
or
```html
text<span>text</span>
```
correct?
While fixing the failure is great, I'm afraid of loosing the performance gain by this fast code path in too common cases.
Can you try to find a way to limit `return false` in more rare cases, or other ways to fix this?
Related details
Attention is currently required from:
- Fredrik Söderquist
- Nishaanth Shriram M S
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. |
Nishaanth Shriram M S (Gerrit)
Mar 27, 2026, 8:14:07 AMMar 27
to Kent Tamura, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Koji Ishii
Nishaanth Shriram M S voted and added 3 comments![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Auto-Submit | +1 |
3 comments
Commit Message
Can you change this line to:
```
Bug: 496807859
```
Nishaanth Shriram M S
Done
Please remove this blank line.
Nishaanth Shriram M S
Done
File third_party/blink/renderer/core/layout/inline/line_breaker.cc
Line 1846, Patchset 10:
} else if (line_info->Results().size() > 1) [[unlikely]] {Koji Ishii . unresolved
Hmm...this is quite a common case, no? We will run into this code path for:
```html
<span><span>text</span></span>
```
or
```html
text<span>text</span>
```
correct?While fixing the failure is great, I'm afraid of loosing the performance gain by this fast code path in too common cases.
Can you try to find a way to limit `return false` in more rare cases, or other ways to fix this?
Nishaanth Shriram M S
Oh yes. Guess didn't think of its occurrance without the margin values. Then I guess the DCHECK it self must be corrected in that case. What do you think?
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
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. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Mar 27, 2026, 8:29:19 AMMar 27
to Nishaanth Shriram M S, Kent Tamura, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist, Koji Ishii and Nishaanth Shriram M S
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/12cd907d090000
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
- Nishaanth Shriram M S
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. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Mar 27, 2026, 8:50:37 AMMar 27
to Nishaanth Shriram M S, Kent Tamura, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist, Koji Ishii and Nishaanth Shriram M S
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
😿 Job linux-perf/blink_perf.layout failed.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/12143b57090000
Nishaanth Shriram M S (Gerrit)
Mar 30, 2026, 1:07:36 AMMar 30
to chrom...@appspot.gserviceaccount.com, Kent Tamura, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Koji Ishii
Nishaanth Shriram M S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Auto-Submit | +1 |
1 comment
File third_party/blink/renderer/core/layout/inline/line_breaker.cc
Line 1846, Patchset 10:
} else if (line_info->Results().size() > 1) [[unlikely]] {Koji Ishii . resolved
Nishaanth Shriram M SHmm...this is quite a common case, no? We will run into this code path for:
```html
<span><span>text</span></span>
```
or
```html
text<span>text</span>
```
correct?While fixing the failure is great, I'm afraid of loosing the performance gain by this fast code path in too common cases.
Can you try to find a way to limit `return false` in more rare cases, or other ways to fix this?
Oh yes. Guess didn't think of its occurrance without the margin values. Then I guess the DCHECK it self must be corrected in that case. What do you think?
Nishaanth Shriram M S
I have updated the DCHECK to account for the right width. Can you check?
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nishaanth Shriram M S (Gerrit)
Apr 1, 2026, 5:13:11 AM (12 days ago) Apr 1
to chrom...@appspot.gserviceaccount.com, Kent Tamura, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Koji Ishii
Nishaanth Shriram M S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Auto-Submit | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 12 (Latest):
Nishaanth Shriram M S . resolved
HI @ko...@chromium.org @tk...@chromium.org @f...@opera.com Any update on this Pull request?
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nishaanth Shriram M S (Gerrit)
Apr 6, 2026, 2:19:23 AM (7 days ago) Apr 6
to Kent Tamura, chrom...@appspot.gserviceaccount.com, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Koji Ishii
Nishaanth Shriram M S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Auto-Submit | +1 |
Nishaanth Shriram M S . resolved
hi @tk...@chromium.orgCan you review this CL?
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nishaanth Shriram M S (Gerrit)
Apr 6, 2026, 2:19:43 AM (7 days ago) Apr 6
to Kent Tamura, chrom...@appspot.gserviceaccount.com, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Koji Ishii
Nishaanth Shriram M S added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Nishaanth Shriram M S . resolved
hi @tk...@chromium.orgCan you review this CL?
Nishaanth Shriram M S
Nishaanth Shriram M S (Gerrit)
Apr 7, 2026, 1:14:50 AM (6 days ago) Apr 7
to Kent Tamura, chrom...@appspot.gserviceaccount.com, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist, Koji Ishii and Nishaanth Shriram M S
Nishaanth Shriram M S voted Commit-Queue+2![]( "Open in Gerrit")
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
- Nishaanth Shriram M S
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kent Tamura (Gerrit)
Apr 7, 2026, 1:43:58 AM (6 days ago) Apr 7
to Nishaanth Shriram M S, Kent Tamura, chrom...@appspot.gserviceaccount.com, Koji Ishii, Fredrik Söderquist, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist, Koji Ishii and Nishaanth Shriram M S
Kent Tamura added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Nishaanth Shriram M S . resolved
Nishaanth Shriram M Shi @tk...@chromium.orgCan you review this CL?
Kent Tamura
I'll leave the review of this to @ko...@chromium.org .
Related details
Attention is currently required from:
- Fredrik Söderquist
- Koji Ishii
- Nishaanth Shriram M S
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Koji Ishii (Gerrit)
12:53 AM (7 hours ago) 12:53 AM
to Nishaanth Shriram M S, Kent Tamura, chrom...@appspot.gserviceaccount.com, Fredrik Söderquist, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist and Nishaanth Shriram M S
Koji Ishii voted and added 1 comment![]( "Open in Gerrit")
Votes added by Koji Ishii
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Fredrik Söderquist
- Nishaanth Shriram M S
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nishaanth Shriram M S (Gerrit)
5:49 AM (3 hours ago) 5:49 AM
to Koji Ishii, Kent Tamura, chrom...@appspot.gserviceaccount.com, Fredrik Söderquist, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist
Nishaanth Shriram M S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nishaanth Shriram M S
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Nishaanth Shriram M S . resolved
Related details
Attention is currently required from:
- Fredrik Söderquist
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fredrik Söderquist (Gerrit)
5:51 AM (2 hours ago) 5:51 AM
to Nishaanth Shriram M S, Koji Ishii, Kent Tamura, chrom...@appspot.gserviceaccount.com, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Nishaanth Shriram M S
Fredrik Söderquist voted and added 1 comment![]( "Open in Gerrit")
Votes added by Fredrik Söderquist
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Attention is currently required from:
- Nishaanth Shriram M S
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
6:27 AM (2 hours ago) 6:27 AM
to Nishaanth Shriram M S, Fredrik Söderquist, Koji Ishii, Kent Tamura, chrom...@appspot.gserviceaccount.com, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Nishaanth Shriram M S
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/59140.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Related details
Attention is currently required from:
- Nishaanth Shriram M S
Submit Requirements:
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)
6:46 AM (2 hours ago) 6:46 AM
to Nishaanth Shriram M S, Blink W3C Test Autoroller, Fredrik Söderquist, Koji Ishii, Kent Tamura, chrom...@appspot.gserviceaccount.com, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Check negative margin sizes for min-content while computing indent position
Bug: 496807859
Change-Id: Id954d9463407ef4f858005026692398fdd91b296
Reviewed-by: Fredrik Söderquist <f...@opera.com>
Reviewed-by: Koji Ishii <ko...@chromium.org>
Commit-Queue: Fredrik Söderquist <f...@opera.com>
Auto-Submit: Nishaanth Shriram M S <nishaanth...@gmail.com>
Cr-Commit-Position: refs/heads/main@{#1613611}
Files:
- M third_party/blink/renderer/core/layout/inline/line_breaker.cc
- A third_party/blink/web_tests/external/wpt/css/css-sizing/min-content-negative-margin-inline-crash.html
Change size: S
Delta: 2 files changed, 37 insertions(+), 3 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Fredrik Söderquist, +1 by Koji Ishii
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages