[Toolbar] Fix strict weak ordering violation in actions_sorting_function [chromium/src : main]
0 views
Skip to first unread message
Tim Eulitz (Gerrit)
Apr 19, 2026, 9:08:12 PM (11 days ago) Apr 19
to Antonio Sartori, Caroline Rising, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Antonio Sartori and Caroline Rising
Tim Eulitz added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Tim Eulitz . resolved
A bit of context for this fix: We ran into a crash on startup while running a local debug build of CEF. It turns out this particular lambda violated the strict weak ordering requirement for std::sort (comparing an item to itself returned true). Release builds seem to silently survive it but libc++ debug assertions catch it and intentionally crash. Adding this equivalence check to return false for identical IDs fixes it.
Related details
Attention is currently required from:
- Antonio Sartori
- Caroline Rising
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Eshwar Stalin (Gerrit)
Apr 20, 2026, 7:01:41 PM (10 days ago) Apr 20
to Tim Eulitz, David Yeung, Caroline Rising, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Caroline Rising, David Yeung and Tim Eulitz
Eshwar Stalin voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Caroline Rising
- David Yeung
- Tim Eulitz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Yeung (Gerrit)
Apr 20, 2026, 9:34:16 PM (10 days ago) Apr 20
to Tim Eulitz, Eshwar Stalin, Caroline Rising, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Caroline Rising and Tim Eulitz
David Yeung voted and added 1 comment![]( "Open in Gerrit")
Votes added by David Yeung
| Code-Review | +1 |
1 comment
File chrome/browser/ui/views/toolbar/toolbar_controller.cc
Line 614, Patchset 2 (Latest):
for (int ordered_pinned_action_id : ordered_pinned_action_ids) {David Yeung . resolved
Unrelated to the CL but having a for loop within a sort seems like code smell.
Related details
Attention is currently required from:
- Caroline Rising
- Tim Eulitz
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tim Eulitz (Gerrit)
Apr 21, 2026, 5:00:47 AM (10 days ago) Apr 21
to David Yeung, Eshwar Stalin, Caroline Rising, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Caroline Rising
Tim Eulitz voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Caroline Rising
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tim Eulitz (Gerrit)
Apr 21, 2026, 5:48:01 AM (10 days ago) Apr 21
to David Yeung, Eshwar Stalin, Caroline Rising, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Caroline Rising
Tim Eulitz added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Tim Eulitz . resolved
Thanks for the approvals! Since I don't have committer access, could one of you please set CQ+2 to submit this? @est...@chromium.org @day...@chromium.org
David Yeung (Gerrit)
Apr 21, 2026, 10:04:41 AM (9 days ago) Apr 21
to Tim Eulitz, Eshwar Stalin, Caroline Rising, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Caroline Rising and Tim Eulitz
David Yeung voted Commit-Queue+2![]( "Open in Gerrit")
Attention is currently required from:
- Caroline Rising
- Tim Eulitz
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)
Apr 21, 2026, 10:55:56 AM (9 days ago) Apr 21
to Tim Eulitz, David Yeung, Eshwar Stalin, Caroline Rising, chromium...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[Toolbar] Fix strict weak ordering violation in actions_sorting_function
The lambda actions_sorting_function in toolbar_controller.cc
violated the irreflexivity requirement of strict weak ordering
by returning true when comparing identical ActionIds. This
caused deterministic crashes in libc++ debug builds due to
std::sort assertion failures.
This CL adds an equality check to ensure identical elements
return false, satisfying the comparator requirements.
Bug: None
Change-Id: I1898eb3062f3121cea79a711895a77b7274557e1
Commit-Queue: David Yeung <day...@chromium.org>
Reviewed-by: David Yeung <day...@chromium.org>
Reviewed-by: Eshwar Stalin <est...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1618198}
Files:
- M AUTHORS
- M chrome/browser/ui/views/toolbar/toolbar_controller.cc
Change size: XS
Delta: 2 files changed, 4 insertions(+), 0 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by David Yeung, +1 by Eshwar Stalin
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages