yiwen qian (Gerrit)

unread,

Mar 2, 2026, 8:08:11 PM (3 days ago) Mar 2

to Qihui Zhao, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, sloboda...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Qihui Zhao

yiwen qian added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

yiwen qian . resolved

Hi Qihui, could you please review this CL? Thank you!

Open in Gerrit

Related details

Attention is currently required from:

Qihui Zhao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Qihui Zhao (Gerrit)

unread,

Mar 3, 2026, 12:13:42 PM (2 days ago) Mar 3

to yiwen qian, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, sloboda...@chromium.org, tmartino+tran...@chromium.org

Attention needed from yiwen qian

Qihui Zhao added 7 comments

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model.h

Line 150, Patchset 1 (Latest): virtual void OnUiUpdatedAndAcceptedForSaveAndFill(

Qihui Zhao . unresolved

Might be better to put this method around line62.
And per design, please rename this to `onUpdatedAndAcceptedForSaveAndFill`.

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model.mm

Line 64, Patchset 1 (Latest): save_card_delegate_->OnUiUpdatedAndAcceptedForSaveAndFill(details);

Qihui Zhao . unresolved

Can it be called by `save_card_delegate()`? I see other usages of this.

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model_unittest.mm

Line 217, Patchset 1 (Latest): details.card_number = u"987654321";

Qihui Zhao . unresolved

Please put a valid fake card number.

File ios/chrome/browser/autofill/ui_bundled/bottom_sheet/save_card_bottom_sheet_mediator.mm

Line 286, Patchset 1 (Latest): // Pass the details from the View Controller down to the Model

Qihui Zhao . unresolved

Add period.

File ios/chrome/browser/autofill/ui_bundled/bottom_sheet/save_card_bottom_sheet_mediator_unittest.mm

Line 598, Patchset 1 (Latest):// Tests that `onUpdatedAndAcceptedForSaveAndFill:` calls the corresponding

Qihui Zhao . unresolved

Remove.

Line 603, Patchset 1 (Latest): details.card_number = u"123456789";

Qihui Zhao . unresolved

Same here.

Line 603, Patchset 1 (Latest): details.card_number = u"123456789";

Qihui Zhao . unresolved

Can you cover all fields that we care about for our case?

Open in Gerrit

Related details

Attention is currently required from:

yiwen qian

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

yiwen qian (Gerrit)

unread,

Mar 3, 2026, 7:21:23 PM (2 days ago) Mar 3

to Qihui Zhao, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, sloboda...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Qihui Zhao

yiwen qian added 7 comments

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model.h

Line 150, Patchset 1: virtual void OnUiUpdatedAndAcceptedForSaveAndFill(

Qihui Zhao . resolved

Might be better to put this method around line62.
And per design, please rename this to `onUpdatedAndAcceptedForSaveAndFill`.

yiwen qian

Done

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model.mm

Line 64, Patchset 1: save_card_delegate_->OnUiUpdatedAndAcceptedForSaveAndFill(details);

Qihui Zhao . resolved

Can it be called by `save_card_delegate()`? I see other usages of this.

yiwen qian

Done

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model_unittest.mm

Line 217, Patchset 1: details.card_number = u"987654321";

Qihui Zhao . resolved

Please put a valid fake card number.

yiwen qian

Done

File ios/chrome/browser/autofill/ui_bundled/bottom_sheet/save_card_bottom_sheet_mediator.mm

Line 286, Patchset 1: // Pass the details from the View Controller down to the Model

Qihui Zhao . resolved

Add period.

yiwen qian

Done

File ios/chrome/browser/autofill/ui_bundled/bottom_sheet/save_card_bottom_sheet_mediator_unittest.mm

Line 598, Patchset 1:// Tests that `onUpdatedAndAcceptedForSaveAndFill:` calls the corresponding

Qihui Zhao . resolved

Remove.

yiwen qian

Done

Line 603, Patchset 1: details.card_number = u"123456789";

Qihui Zhao . resolved

Can you cover all fields that we care about for our case?

yiwen qian

Done

Line 603, Patchset 1: details.card_number = u"123456789";

Qihui Zhao . resolved

Same here.

yiwen qian

Done

Open in Gerrit

Related details

Attention is currently required from:

Qihui Zhao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Qihui Zhao (Gerrit)

unread,

Mar 4, 2026, 1:18:30 PM (yesterday) Mar 4

to yiwen qian, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, sloboda...@chromium.org, tmartino+tran...@chromium.org

Attention needed from yiwen qian

Qihui Zhao voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

yiwen qian

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

yiwen qian (Gerrit)

unread,

Mar 4, 2026, 1:30:06 PM (yesterday) Mar 4

to Tommy Martino, Qihui Zhao, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, sloboda...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Tommy Martino

yiwen qian added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

yiwen qian . resolved

Hi Tommy, could you please review this CL? Thank you!

Open in Gerrit

Related details

Attention is currently required from:

Tommy Martino

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Tommy Martino (Gerrit)

unread,

10:27 AM (5 hours ago) 10:27 AM

to yiwen qian, Qihui Zhao, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, sloboda...@chromium.org, tmartino+tran...@chromium.org

Attention needed from yiwen qian

Tommy Martino added 4 comments

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model.mm

Line 64, Patchset 2 (Latest): save_card_delegate()->OnUiUpdatedAndAcceptedForSaveAndFill(details);

Tommy Martino . unresolved

To avoid an unnecessary copy here, let's:

(1) change this method to accept `details` by value and not const reference

(2) `std::move(details)` on this line

File ios/chrome/browser/autofill/model/bottom_sheet/save_card_bottom_sheet_model_unittest.mm

Line 226, Patchset 2 (Latest):

      .WillOnce([&](const payments::PaymentsAutofillClient::
                        UserProvidedCardSaveAndFillDetails& passed_details) {
        EXPECT_EQ(passed_details.card_number, u"5555555555554444");
        EXPECT_EQ(passed_details.cardholder_name, u"John Doe");
        EXPECT_EQ(passed_details.expiration_date_month, u"12");
        EXPECT_EQ(passed_details.expiration_date_year, u"2030");
        EXPECT_EQ(passed_details.security_code, u"123");
        EXPECT_EQ(passed_details.nickname, u"My Test Card");

Tommy Martino . unresolved

Rather than using WillOnce to capture and examine the details, consider using a more specific compound matcher instead of `_` -- https://google.github.io/googletest/reference/matchers.html#member-matchers

File ios/chrome/browser/autofill/ui_bundled/bottom_sheet/save_card_bottom_sheet_mediator.mm

Line 287, Patchset 2 (Latest): _saveCardBottomSheetModel->OnUpdatedAndAcceptedForSaveAndFill(details);

Tommy Martino . unresolved

`std::move` here as well

File ios/chrome/browser/autofill/ui_bundled/bottom_sheet/save_card_bottom_sheet_mediator_unittest.mm

Line 612, Patchset 2 (Latest): .WillOnce([&](const autofill::payments::PaymentsAutofillClient::

Tommy Martino . unresolved

Same as above

Open in Gerrit

Related details

Attention is currently required from:

yiwen qian

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Tommy Martino (Gerrit)

unread,

10:27 AM (5 hours ago) 10:27 AM

to yiwen qian, Qihui Zhao, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, sloboda...@chromium.org, tmartino+tran...@chromium.org

Attention needed from yiwen qian

Tommy Martino voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

yiwen qian

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[ScanCard iOS] Add onUpdatedAndAcceptedForSaveAndFill to mutator [chromium/src : main]

yiwen qian (Gerrit)

yiwen qian added 1 comment

Related details

Qihui Zhao (Gerrit)

Qihui Zhao added 7 comments

Related details

yiwen qian (Gerrit)

yiwen qian added 7 comments

Related details

Qihui Zhao (Gerrit)

Qihui Zhao voted Code-Review+1

Related details

yiwen qian (Gerrit)

yiwen qian added 1 comment

Related details

Tommy Martino (Gerrit)

Tommy Martino added 4 comments

Related details

Tommy Martino (Gerrit)

Tommy Martino voted Code-Review+1

Related details