Added additional `this` tracking in MenuController:OnMousePressed(). [chromium/src : main]
0 views
Skip to first unread message
Allen Bauer (Gerrit)
12:23 PM (5 hours ago) 12:23 PM
to David Yeung, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, sky+...@chromium.org, roblia...@chromium.org
Attention needed from David Yeung
Allen Bauer voted and added 1 comment![]( "Open in Gerrit")
Votes added by Allen Bauer
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- David Yeung
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)
12:56 PM (4 hours ago) 12:56 PM
to Allen Bauer, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, sky+...@chromium.org, roblia...@chromium.org
Attention needed from Allen Bauer
David Yeung voted and added 2 comments![]( "Open in Gerrit")
Votes added by David Yeung
| Code-Review | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 2 (Latest):
David Yeung . resolved
lgtm w/comments about how to scale this fix.
File ui/views/controls/menu/menu_controller.cc
Line 912, Patchset 2 (Latest):
SetHotTrackedButton(button);David Yeung . unresolved
nit. I'm worried about how we're going to handle this without weak ptr checks. SetHotTrackedButton is called 22 times in this class. This seems like we'd need to add weakptr checks everywhere.
Related details
Attention is currently required from:
- Allen Bauer
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. |
Allen Bauer (Gerrit)
1:13 PM (4 hours ago) 1:13 PM
to David Yeung, Chromium LUCI CQ, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, sky+...@chromium.org, roblia...@chromium.org
Allen Bauer voted and added 1 comment![]( "Open in Gerrit")
Votes added by Allen Bauer
| Auto-Submit | +1 |
| Commit-Queue | +2 |
1 comment
File ui/views/controls/menu/menu_controller.cc
Line 912, Patchset 2 (Latest):
SetHotTrackedButton(button);David Yeung . resolved
nit. I'm worried about how we're going to handle this without weak ptr checks. SetHotTrackedButton is called 22 times in this class. This seems like we'd need to add weakptr checks everywhere.
Allen Bauer
That is certainly a broader concern, for which we don't have an easy answer. The immediate concern is to take care of the S0 reports. I don't think this one is easily down-gradable. Feel free to comment on the bug if you think the severity is not S0/S1.
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. |
Chromium LUCI CQ (Gerrit)
3:35 PM (1 hour ago) 3:35 PM
to Allen Bauer, David Yeung, chromium...@chromium.org, android-bu...@system.gserviceaccount.com, sky+...@chromium.org, roblia...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Added additional `this` tracking in MenuController:OnMousePressed().
Guards against a11y tools signaling the menu to close during the
notification of the newly hot-tracked button.
Change-Id: Ida8121655c53bc1c4f59c0aaf9a25226ee73c40a
Bug: 516962178
Auto-Submit: Allen Bauer <kyl...@chromium.org>
Commit-Queue: Allen Bauer <kyl...@chromium.org>
Reviewed-by: David Yeung <day...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1637875}
Files:
- M ui/views/controls/menu/menu_controller.cc
Change size: XS
Delta: 1 file changed, 5 insertions(+), 1 deletion(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by David Yeung
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages