[CCSG-2] Move parameters calculation from BAM into CCSG [chromium/src : main]

0 views
Skip to first unread message

Mikita Kuchyn (Gerrit)

unread,
10:05 AM (13 hours ago) 10:05 AM
to Jihad Hanna, Olivia Saul, Dominic Battre, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Jihad Hanna

Mikita Kuchyn added 1 comment

Patchset-level comments
File-level comment, Patchset 3 (Latest):
Mikita Kuchyn . resolved

Hi, PTAL

Open in Gerrit

Related details

Attention is currently required from:
  • Jihad Hanna
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
Gerrit-Change-Number: 7456884
Gerrit-PatchSet: 3
Gerrit-Owner: Mikita Kuchyn <kuc...@chromium.org>
Gerrit-Reviewer: Jihad Hanna <jihad...@google.com>
Gerrit-CC: Dominic Battre <bat...@chromium.org>
Gerrit-CC: Olivia Saul <os...@google.com>
Gerrit-Attention: Jihad Hanna <jihad...@google.com>
Gerrit-Comment-Date: Wed, 14 Jan 2026 15:04:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Jihad Hanna (Gerrit)

unread,
11:45 AM (11 hours ago) 11:45 AM
to Mikita Kuchyn, Chromium LUCI CQ, Olivia Saul, Dominic Battre, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Mikita Kuchyn

Jihad Hanna added 3 comments

Patchset-level comments
Jihad Hanna . unresolved

bots are unhappy

File components/autofill/core/browser/suggestions/payments/payments_suggestion_generator.cc
Line 346, Patchset 3 (Latest): std::ranges::move(GetCreditCardFooterSuggestions(
client, /*should_show_bnpl_suggestion=*/false,
ShouldShowScanCreditCard(
form_structure, autofill_trigger_field, client),
trigger_field.is_autofilled(), display_gpay_logo,
amount_extraction_status),
std::back_inserter(suggestions));
Jihad Hanna . unresolved

nit: Use `base::Extend()`?

File components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_unittest.cc
Line 2424, Patchset 3 (Latest): // Complete credit card form (passes FormStructure::IsCompleteCreditCardForm)
test::FormDescription form_description = {
.fields = {
{.role = FieldType::CREDIT_CARD_NUMBER, .value = u"411"},
{.role = FieldType::CREDIT_CARD_EXP_MONTH},
{.role = FieldType::CREDIT_CARD_EXP_4_DIGIT_YEAR},
{.role = FieldType::CREDIT_CARD_VERIFICATION_CODE},
{.role = FieldType::CREDIT_CARD_NAME_FULL},
}};
FormData form = test::GetFormData(form_description);
FormStructure form_structure(form);
for (size_t i = 0; i < form_structure.field_count(); ++i) {
form_structure.field(i)->set_heuristic_type(
HeuristicSource::kRegexes, form_description.fields[i].role);
}
Jihad Hanna . unresolved

Can you create a helper function `GetFormWithTypes()` that abstracts this?

Open in Gerrit

Related details

Attention is currently required from:
  • Mikita Kuchyn
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
    Gerrit-Change-Number: 7456884
    Gerrit-PatchSet: 3
    Gerrit-Owner: Mikita Kuchyn <kuc...@chromium.org>
    Gerrit-Reviewer: Jihad Hanna <jihad...@google.com>
    Gerrit-Reviewer: Mikita Kuchyn <kuc...@chromium.org>
    Gerrit-CC: Dominic Battre <bat...@chromium.org>
    Gerrit-CC: Olivia Saul <os...@google.com>
    Gerrit-Attention: Mikita Kuchyn <kuc...@chromium.org>
    Gerrit-Comment-Date: Wed, 14 Jan 2026 16:45:30 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mikita Kuchyn (Gerrit)

    unread,
    2:46 PM (8 hours ago) 2:46 PM
    to Chromium LUCI CQ, Jihad Hanna, Olivia Saul, Dominic Battre, chromium...@chromium.org, browser-comp...@chromium.org
    Attention needed from Jihad Hanna

    Mikita Kuchyn added 3 comments

    Patchset-level comments
    File-level comment, Patchset 3:
    Jihad Hanna . resolved

    bots are unhappy

    Mikita Kuchyn

    Should be cheered up by now (moved ShouldShowScanCreditCard tests from bam_unittests.cc to psg_unittests.cc)

    File components/autofill/core/browser/suggestions/payments/payments_suggestion_generator.cc
    Line 346, Patchset 3: std::ranges::move(GetCreditCardFooterSuggestions(

    client, /*should_show_bnpl_suggestion=*/false,
    ShouldShowScanCreditCard(
    form_structure, autofill_trigger_field, client),
    trigger_field.is_autofilled(), display_gpay_logo,
    amount_extraction_status),
    std::back_inserter(suggestions));
    Jihad Hanna . resolved

    nit: Use `base::Extend()`?

    Mikita Kuchyn

    Cool, didn't know about that one

    File components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_unittest.cc
    Line 2424, Patchset 3: // Complete credit card form (passes FormStructure::IsCompleteCreditCardForm)

    test::FormDescription form_description = {
    .fields = {
    {.role = FieldType::CREDIT_CARD_NUMBER, .value = u"411"},
    {.role = FieldType::CREDIT_CARD_EXP_MONTH},
    {.role = FieldType::CREDIT_CARD_EXP_4_DIGIT_YEAR},
    {.role = FieldType::CREDIT_CARD_VERIFICATION_CODE},
    {.role = FieldType::CREDIT_CARD_NAME_FULL},
    }};
    FormData form = test::GetFormData(form_description);
    FormStructure form_structure(form);
    for (size_t i = 0; i < form_structure.field_count(); ++i) {
    form_structure.field(i)->set_heuristic_type(
    HeuristicSource::kRegexes, form_description.fields[i].role);
    }
    Jihad Hanna . resolved

    Can you create a helper function `GetFormWithTypes()` that abstracts this?

    Mikita Kuchyn

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Jihad Hanna
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
      Gerrit-Change-Number: 7456884
      Gerrit-PatchSet: 6
      Gerrit-Owner: Mikita Kuchyn <kuc...@chromium.org>
      Gerrit-Reviewer: Jihad Hanna <jihad...@google.com>
      Gerrit-Reviewer: Mikita Kuchyn <kuc...@chromium.org>
      Gerrit-CC: Dominic Battre <bat...@chromium.org>
      Gerrit-CC: Olivia Saul <os...@google.com>
      Gerrit-Attention: Jihad Hanna <jihad...@google.com>
      Gerrit-Comment-Date: Wed, 14 Jan 2026 19:46:23 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Jihad Hanna <jihad...@google.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Jihad Hanna (Gerrit)

      unread,
      3:03 PM (8 hours ago) 3:03 PM
      to Mikita Kuchyn, Chromium LUCI CQ, Olivia Saul, Dominic Battre, chromium...@chromium.org, browser-comp...@chromium.org
      Attention needed from Mikita Kuchyn

      Jihad Hanna voted and added 2 comments

      Votes added by Jihad Hanna

      Code-Review+1

      2 comments

      File components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_unittest.cc
      Line 74, Patchset 6 (Latest):using ::std::make_unique;
      Jihad Hanna . unresolved

      We don't usually add using statements for `std`, please remove.

      Line 303, Patchset 6 (Latest): form_structure->field(i)->set_heuristic_type(
      Jihad Hanna . unresolved

      Why not just `SetTypeTo()`?

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Mikita Kuchyn
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
      Gerrit-Change-Number: 7456884
      Gerrit-PatchSet: 6
      Gerrit-Owner: Mikita Kuchyn <kuc...@chromium.org>
      Gerrit-Reviewer: Jihad Hanna <jihad...@google.com>
      Gerrit-Reviewer: Mikita Kuchyn <kuc...@chromium.org>
      Gerrit-CC: Dominic Battre <bat...@chromium.org>
      Gerrit-CC: Olivia Saul <os...@google.com>
      Gerrit-Attention: Mikita Kuchyn <kuc...@chromium.org>
      Gerrit-Comment-Date: Wed, 14 Jan 2026 20:03:32 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Mikita Kuchyn (Gerrit)

      unread,
      6:10 PM (5 hours ago) 6:10 PM
      to Jihad Hanna, Chromium LUCI CQ, Olivia Saul, Dominic Battre, chromium...@chromium.org, browser-comp...@chromium.org

      Mikita Kuchyn added 2 comments

      File components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_unittest.cc
      Line 74, Patchset 6:using ::std::make_unique;
      Jihad Hanna . resolved

      We don't usually add using statements for `std`, please remove.

      Mikita Kuchyn

      Done

      Line 303, Patchset 6: form_structure->field(i)->set_heuristic_type(
      Jihad Hanna . resolved

      Why not just `SetTypeTo()`?

      Mikita Kuchyn

      `psg.cc` works with `FieldType`, but `SetTypeTo()` works with `AutofillType`, so I defaulted to `set_heuristic_type()` to fill in the `FieldType`. I can see now that `SetTypeTo(AutofillType(FieldType))` is used throughout the codebase, so changed all occurences in tests to that.

      Open in Gerrit

      Related details

      Attention set is empty
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
        Gerrit-Change-Number: 7456884
        Gerrit-PatchSet: 7
        Gerrit-Owner: Mikita Kuchyn <kuc...@chromium.org>
        Gerrit-Reviewer: Jihad Hanna <jihad...@google.com>
        Gerrit-Reviewer: Mikita Kuchyn <kuc...@chromium.org>
        Gerrit-CC: Dominic Battre <bat...@chromium.org>
        Gerrit-CC: Olivia Saul <os...@google.com>
        Gerrit-Comment-Date: Wed, 14 Jan 2026 23:10:22 +0000
        satisfied_requirement
        open
        diffy

        Mikita Kuchyn (Gerrit)

        unread,
        6:10 PM (5 hours ago) 6:10 PM
        to Jihad Hanna, Chromium LUCI CQ, Olivia Saul, Dominic Battre, chromium...@chromium.org, browser-comp...@chromium.org

        Mikita Kuchyn voted Commit-Queue+2

        Commit-Queue+2
        Open in Gerrit

        Related details

        Attention set is empty
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
        Gerrit-Change-Number: 7456884
        Gerrit-PatchSet: 7
        Gerrit-Owner: Mikita Kuchyn <kuc...@chromium.org>
        Gerrit-Reviewer: Jihad Hanna <jihad...@google.com>
        Gerrit-Reviewer: Mikita Kuchyn <kuc...@chromium.org>
        Gerrit-CC: Dominic Battre <bat...@chromium.org>
        Gerrit-CC: Olivia Saul <os...@google.com>
        Gerrit-Comment-Date: Wed, 14 Jan 2026 23:10:30 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        open
        diffy

        Chromium LUCI CQ (Gerrit)

        unread,
        7:38 PM (4 hours ago) 7:38 PM
        to Mikita Kuchyn, Jihad Hanna, Olivia Saul, Dominic Battre, chromium...@chromium.org, browser-comp...@chromium.org

        Chromium LUCI CQ submitted the change with unreviewed changes

        Unreviewed changes

        6 is the latest approved patch-set.
        The change was submitted with unreviewed changes in the following files:

        ```
        The name of the file: components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_unittest.cc
        Insertions: 20, Deletions: 20.

        @@ -71,7 +71,6 @@
         namespace {
         
         using ::gfx::test::AreImagesEqual;
        -using ::std::make_unique;
         using ::testing::_;
         using ::testing::AllOf;
         using ::testing::ElementsAre;
        @@ -300,8 +299,8 @@

             FormData form = test::GetFormData(form_description);
             auto form_structure = std::make_unique<FormStructure>(form);
             for (size_t i = 0; i < form_structure->field_count(); ++i) {
        -      form_structure->field(i)->set_heuristic_type(
        -          HeuristicSource::kRegexes, form_description.fields[i].role);
        +      form_structure->field(i)->SetTypeTo(
        +          AutofillType(form_description.fields[i].role), std::nullopt);
             }
         
             AutofillField* trigger_autofill_field = form_structure->field(0);
        @@ -4229,9 +4228,9 @@
           FormData form;
           FormFieldData trigger_field;
           AutofillField trigger_autofill_field(trigger_field);
        -  trigger_autofill_field.set_heuristic_type(
        -      HeuristicSource::kRegexes,
        -      FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE);
        +  trigger_autofill_field.SetTypeTo(
        +      AutofillType(FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE),
        +      AutofillPredictionSource::kHeuristics);
           std::vector<Suggestion> suggestions = GetSuggestionsForCreditCards(
               form, FormStructure(form), trigger_field, trigger_autofill_field,
               autofill_client(),
        @@ -4273,8 +4272,9 @@
         
           FormFieldData trigger_field;
           AutofillField trigger_autofill_field(trigger_field);
        -  trigger_autofill_field.set_heuristic_type(
        -      HeuristicSource::kRegexes, FieldType::CREDIT_CARD_VERIFICATION_CODE);
        +  trigger_autofill_field.SetTypeTo(
        +      AutofillType(FieldType::CREDIT_CARD_VERIFICATION_CODE),
        +      AutofillPredictionSource::kHeuristics);
         
           std::vector<Suggestion> suggestions = GetSuggestionsForCreditCards(
               form, FormStructure(form), trigger_field, trigger_autofill_field,
        @@ -4305,9 +4305,9 @@
           FormData form;
           FormFieldData trigger_field;
           AutofillField trigger_autofill_field(trigger_field);
        -  trigger_autofill_field.set_heuristic_type(
        -      HeuristicSource::kRegexes,
        -      FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE);
        +  trigger_autofill_field.SetTypeTo(
        +      AutofillType(FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE),
        +      AutofillPredictionSource::kHeuristics);
           std::vector<Suggestion> suggestions = GetSuggestionsForCreditCards(
               form, FormStructure(form), trigger_field, trigger_autofill_field,
               autofill_client(),
        @@ -4325,9 +4325,9 @@
           FormData form;
           FormFieldData trigger_field;
           AutofillField trigger_autofill_field(trigger_field);
        -  trigger_autofill_field.set_heuristic_type(
        -      HeuristicSource::kRegexes,
        -      FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE);
        +  trigger_autofill_field.SetTypeTo(
        +      AutofillType(FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE),
        +      AutofillPredictionSource::kHeuristics);
           std::vector<Suggestion> suggestions = GetSuggestionsForCreditCards(
               form, FormStructure(form), trigger_field, trigger_autofill_field,
               autofill_client(),
        @@ -4351,9 +4351,9 @@
           FormData form;
           FormFieldData trigger_field;
           AutofillField trigger_autofill_field(trigger_field);
        -  trigger_autofill_field.set_heuristic_type(
        -      HeuristicSource::kRegexes,
        -      FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE);
        +  trigger_autofill_field.SetTypeTo(
        +      AutofillType(FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE),
        +      AutofillPredictionSource::kHeuristics);
           std::vector<Suggestion> suggestions = GetSuggestionsForCreditCards(
               form, FormStructure(form), trigger_field, trigger_autofill_field,
               autofill_client(),
        @@ -4387,9 +4387,9 @@
           FormFieldData trigger_field;
           trigger_field.set_origin(virtual_card_usage_data.merchant_origin());
           AutofillField trigger_autofill_field(trigger_field);
        -  trigger_autofill_field.set_heuristic_type(
        -      HeuristicSource::kRegexes,
        -      FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE);
        +  trigger_autofill_field.SetTypeTo(
        +      AutofillType(FieldType::CREDIT_CARD_STANDALONE_VERIFICATION_CODE),
        +      AutofillPredictionSource::kHeuristics);
         
           std::vector<Suggestion> suggestions = GetSuggestionsForCreditCards(
               form, FormStructure(form), trigger_field, trigger_autofill_field,
        ```

        Change information

        Commit message:
        [CCSG-2] Move parameters calculation from BAM into CCSG

        Move `should_show_scan_credit_card` and `is_complete_form` calculation
        into `CreditCardSuggestionGenerator class, so the logic is incapsulated
        inside of the class.
        Bug: 409962888
        Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
        Reviewed-by: Jihad Hanna <jihad...@google.com>
        Commit-Queue: Mikita Kuchyn <kuc...@chromium.org>
        Cr-Commit-Position: refs/heads/main@{#1569433}
        Files:
        • M components/autofill/core/browser/foundations/browser_autofill_manager.cc
        • M components/autofill/core/browser/foundations/browser_autofill_manager.h
        • M components/autofill/core/browser/foundations/browser_autofill_manager_test_api.h
        • M components/autofill/core/browser/foundations/browser_autofill_manager_unittest.cc
        • M components/autofill/core/browser/suggestions/payments/credit_card_suggestion_generator.cc
        • M components/autofill/core/browser/suggestions/payments/credit_card_suggestion_generator.h
        • M components/autofill/core/browser/suggestions/payments/payments_suggestion_generator.cc
        • M components/autofill/core/browser/suggestions/payments/payments_suggestion_generator.h
        • M components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_unittest.cc
        • M components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_util.cc
        • M components/autofill/core/browser/suggestions/payments/payments_suggestion_generator_util.h
        Change size: L
        Delta: 11 files changed, 351 insertions(+), 328 deletions(-)
        Branch: refs/heads/main
        Submit Requirements:
        • requirement satisfiedCode-Review: +1 by Jihad Hanna
        Open in Gerrit
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: merged
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I61eb119b7833666f7d1a1754fac21c0dc7bdc0e5
        Gerrit-Change-Number: 7456884
        Gerrit-PatchSet: 8
        Gerrit-Owner: Mikita Kuchyn <kuc...@chromium.org>
        Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
        Gerrit-Reviewer: Jihad Hanna <jihad...@google.com>
        Gerrit-Reviewer: Mikita Kuchyn <kuc...@chromium.org>
        open
        diffy
        satisfied_requirement
        Reply all
        Reply to author
        Forward
        0 new messages