[iOS] Fix dangling raw_ptr to infobar delegate in password save modal [chromium/src : main]
0 views
Skip to first unread message
Karis Li (Gerrit)
4:26 AM (10 hours ago) 4:26 AM
to Chris Lu, Sylvain Defresne, Gauthier Ambard, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu, Gauthier Ambard and Sylvain Defresne
Karis Li added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 7 (Latest):
Karis Li . resolved
This change is ready for review,PTAL,thanks.
Related details
Attention is currently required from:
- Chris Lu
- Gauthier Ambard
- Sylvain Defresne
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
5:33 AM (9 hours ago) 5:33 AM
to Karis Li, Chris Lu, Sylvain Defresne, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu, Karis Li and Sylvain Defresne
Gauthier Ambard voted and added 1 comment![]( "Open in Gerrit")
Votes added by Gauthier Ambard
| Code-Review | +1 |
1 comment
File ios/chrome/browser/overlays/ui_bundled/infobar_modal/passwords/password_infobar_modal_overlay_mediator.mm
Line 59, Patchset 7 (Latest):
IOSChromeSavePasswordInfoBarDelegate* delegate = self.passwordDelegate;Gauthier Ambard . unresolved
Early return if the delegate is null.
Related details
Attention is currently required from:
- Chris Lu
- Karis Li
- Sylvain Defresne
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. |
Karis Li (Gerrit)
7:31 AM (7 hours ago) 7:31 AM
to Gauthier Ambard, Chris Lu, Sylvain Defresne, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu, Gauthier Ambard and Sylvain Defresne
Karis Li added 1 comment![]( "Open in Gerrit")
File ios/chrome/browser/overlays/ui_bundled/infobar_modal/passwords/password_infobar_modal_overlay_mediator.mm
Line 59, Patchset 7:
IOSChromeSavePasswordInfoBarDelegate* delegate = self.passwordDelegate;Gauthier Ambard . resolved
Early return if the delegate is null.
Attention is currently required from:
- Chris Lu
- Gauthier Ambard
- Sylvain Defresne
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sylvain Defresne (Gerrit)
8:32 AM (6 hours ago) 8:32 AM
to Karis Li, Gauthier Ambard, Chris Lu, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu, Gauthier Ambard and Karis Li
Sylvain Defresne voted and added 2 comments![]( "Open in Gerrit")
Votes added by Sylvain Defresne
| Code-Review | +1 |
2 comments
Patchset-level comments
File ios/chrome/browser/overlays/ui_bundled/infobar_modal/passwords/password_infobar_modal_overlay_mediator.mm
Line 55, Patchset 8 (Latest):
if (!_consumer || !self.config) {Sylvain Defresne . unresolved
You can remove this check (since it is now integrated into `-passwordDelegate`).
Related details
Attention is currently required from:
- Chris Lu
- Gauthier Ambard
- Karis Li
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. |
Gauthier Ambard (Gerrit)
8:50 AM (5 hours ago) 8:50 AM
to Karis Li, Sylvain Defresne, Chris Lu, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu, Karis Li and Sylvain Defresne
Gauthier Ambard voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Chris Lu
- Karis Li
- Sylvain Defresne
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. |
Karis Li (Gerrit)
8:51 AM (5 hours ago) 8:51 AM
to Gauthier Ambard, Sylvain Defresne, Chris Lu, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu and Sylvain Defresne
Karis Li added 1 comment![]( "Open in Gerrit")
File ios/chrome/browser/overlays/ui_bundled/infobar_modal/passwords/password_infobar_modal_overlay_mediator.mm
You can remove this check (since it is now integrated into `-passwordDelegate`).
Attention is currently required from:
- Chris Lu
- Sylvain Defresne
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sylvain Defresne (Gerrit)
9:52 AM (4 hours ago) 9:52 AM
to Karis Li, Gauthier Ambard, Chris Lu, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu and Karis Li
Sylvain Defresne voted and added 1 comment![]( "Open in Gerrit")
Votes added by Sylvain Defresne
| Code-Review | +1 |
Sylvain Defresne . resolved
lgtm
Attention is currently required from:
- Chris Lu
- Karis Li
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Karis Li (Gerrit)
9:52 AM (4 hours ago) 9:52 AM
to Sylvain Defresne, Gauthier Ambard, Chris Lu, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Attention needed from Chris Lu
Karis Li voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Chris Lu
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)
9:58 AM (4 hours ago) 9:58 AM
to Karis Li, Sylvain Defresne, Gauthier Ambard, Chris Lu, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[iOS] Fix dangling raw_ptr to infobar delegate in password save modal
The raw_ptr ivar `delegate_` was assigned once in setConsumer: but never
cleared. If InfoBarIOS is destroyed while the modal is still visible,
the pointer becomes dangling.
Replace it with a computed property that retrieves the delegate through
the config on each call, so it returns nil when the infobar has been
destroyed.
Bug: 446106570
Change-Id: I6b24f7107f07973f1f854e63793a85d30e6f48b3
Reviewed-by: Sylvain Defresne <sdef...@chromium.org>
Reviewed-by: Gauthier Ambard <gam...@chromium.org>
Commit-Queue: Karis Li <li...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1594655}
Files:
- M ios/chrome/browser/overlays/ui_bundled/infobar_modal/passwords/password_infobar_modal_overlay_mediator.mm
Change size: M
Delta: 1 file changed, 33 insertions(+), 22 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Sylvain Defresne, +1 by Gauthier Ambard
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sylvain Defresne (Gerrit)
10:41 AM (4 hours ago) 10:41 AM
to Chromium LUCI CQ, Karis Li, Gauthier Ambard, Chris Lu, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, gcasto+w...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, vasilii+watchlis...@chromium.org
Sylvain Defresne added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 10 (Latest):
Sylvain Defresne . resolved
I did look in more detail, and while this remove a dangling pointer annotation, `config` is a `DefaultInfobarOverlayRequestConfig*` which is a C++ pointer and thus can be dangling, but the `@property` does use `raw_ptr<T>`.
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