| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
LGTM % minor comments
// TODO(crbug.com/481379161): Figure out a way to move this intoThree new TODOs with "see if this is possible" are "this will likely never get resolved" red flags to me, becoming a source of new dead code. Didn't the design go in-depth in what the plan was here?
,Can this go above the `iban_save_manager_` instantiation to keep it and `client_` together and avoid the dangling comma, or is the order important here?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
// TODO(crbug.com/481379161): Figure out a way to move this intoThree new TODOs with "see if this is possible" are "this will likely never get resolved" red flags to me, becoming a source of new dead code. Didn't the design go in-depth in what the plan was here?
I didn't realize that the details of how to do this would be more complicated and not no-ops than they were. I'd rather put that in a separate CL once the rest of the functions are moved.
Chatted offline with florian and he's aligned with this approach.
Can this go above the `iban_save_manager_` instantiation to keep it and `client_` together and avoid the dangling comma, or is the order important here?
This is fine since CreditCardSaveManager does not hold a reference to IbanSaveManager. Done
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hi Florian,
PTAL! Thanks
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hi Florian,
PTAL! Thanks
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
.GetCreditCardSaveManager()[This comment](https://source.chromium.org/chromium/chromium/src/+/main:components/autofill/core/browser/payments/credit_card_save_manager.h;l=42;drc=542eedeaf4606aec2003451a8365ac5a0269e33e) should be updated to PaymentsFormDataImporter :)
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks Florian + Olivia!
Allen - Can you PTAL at chrome/browser/ui/views/frame/browser_frame_view_browsertest.cc
Tommy - Can you PTAL at ios/chrome/browser/autofill/ui_bundled/autofill_app_interface.mm?
Thanks so much!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
[This comment](https://source.chromium.org/chromium/chromium/src/+/main:components/autofill/core/browser/payments/credit_card_save_manager.h;l=42;drc=542eedeaf4606aec2003451a8365ac5a0269e33e) should be updated to PaymentsFormDataImporter :)
Done
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
// save logic. Owned by PaymentsFormDataImporter.nitty nit: Remove one space
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
nitty nit: Remove one space
Done
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Move CCSM and credit_card_import_type_ into PaymentsFDI
This CL moves CreditCardSaveManager and credit_card_import_type_ into
PaymentsFormDataImporter.
This CL is a no-op.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |