Pipe through SuggestionMetadata to AtMemory [chromium/src : main]

0 views
Skip to first unread message

Jan Keitel (Gerrit)

unread,
Jul 7, 2026, 9:37:29 AM (yesterday) Jul 7
to Sophie Chang, Rahim Mammadli, Ireneusz Szulc, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Christoph Schwering

Jan Keitel added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Jan Keitel . resolved

Chris, would you PTAL?

Open in Gerrit

Related details

Attention is currently required from:
  • Christoph Schwering
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement 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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
Gerrit-Change-Number: 8052191
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Keitel <jke...@google.com>
Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
Gerrit-Reviewer: Jan Keitel <jke...@google.com>
Gerrit-CC: Ireneusz Szulc <ireneu...@google.com>
Gerrit-CC: Rahim Mammadli <rah...@google.com>
Gerrit-CC: Sophie Chang <sophi...@chromium.org>
Gerrit-Attention: Christoph Schwering <schw...@google.com>
Gerrit-Comment-Date: Tue, 07 Jul 2026 13:37:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Christoph Schwering (Gerrit)

unread,
Jul 7, 2026, 10:40:51 PM (11 hours ago) Jul 7
to Jan Keitel, Sophie Chang, Rahim Mammadli, Ireneusz Szulc, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Jan Keitel

Christoph Schwering added 1 comment

File components/autofill/core/browser/ui/autofill_external_delegate.cc
Line 605, Patchset 3 (Latest): !parent_suggestion_metadata) {
Christoph Schwering . unresolved

Why? I don't understand it intuitivel.

Isn't it true because of the early return above?

Open in Gerrit

Related details

Attention is currently required from:
  • Jan Keitel
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement 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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
    Gerrit-Change-Number: 8052191
    Gerrit-PatchSet: 3
    Gerrit-Owner: Jan Keitel <jke...@google.com>
    Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
    Gerrit-Reviewer: Jan Keitel <jke...@google.com>
    Gerrit-CC: Ireneusz Szulc <ireneu...@google.com>
    Gerrit-CC: Rahim Mammadli <rah...@google.com>
    Gerrit-CC: Sophie Chang <sophi...@chromium.org>
    Gerrit-Attention: Jan Keitel <jke...@google.com>
    Gerrit-Comment-Date: Wed, 08 Jul 2026 02:40:33 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Jan Keitel (Gerrit)

    unread,
    3:00 AM (7 hours ago) 3:00 AM
    to Sophie Chang, Rahim Mammadli, Ireneusz Szulc, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
    Attention needed from Christoph Schwering

    Jan Keitel added 1 comment

    File components/autofill/core/browser/ui/autofill_external_delegate.cc
    Line 605, Patchset 3: !parent_suggestion_metadata) {
    Christoph Schwering . resolved

    Why? I don't understand it intuitivel.

    Isn't it true because of the early return above?

    Jan Keitel

    Please look again. 😊

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Christoph Schwering
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement 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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
      Gerrit-Change-Number: 8052191
      Gerrit-PatchSet: 4
      Gerrit-Owner: Jan Keitel <jke...@google.com>
      Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
      Gerrit-Reviewer: Jan Keitel <jke...@google.com>
      Gerrit-CC: Ireneusz Szulc <ireneu...@google.com>
      Gerrit-CC: Rahim Mammadli <rah...@google.com>
      Gerrit-CC: Sophie Chang <sophi...@chromium.org>
      Gerrit-Attention: Christoph Schwering <schw...@google.com>
      Gerrit-Comment-Date: Wed, 08 Jul 2026 06:59:45 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Christoph Schwering <schw...@google.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Christoph Schwering (Gerrit)

      unread,
      7:00 AM (3 hours ago) 7:00 AM
      to Jan Keitel, Sophie Chang, Rahim Mammadli, Ireneusz Szulc, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
      Attention needed from Jan Keitel

      Christoph Schwering voted and added 1 comment

      Votes added by Christoph Schwering

      Code-Review+1

      1 comment

      File components/autofill/core/browser/ui/autofill_external_delegate.cc
      Line 605, Patchset 3: !parent_suggestion_metadata) {
      Christoph Schwering . unresolved

      Why? I don't understand it intuitivel.

      Isn't it true because of the early return above?

      Jan Keitel

      Please look again. 😊

      Christoph Schwering

      Why do we want to do the `else` branch even if `is_subopop`?

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Jan Keitel
      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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
        Gerrit-Change-Number: 8052191
        Gerrit-PatchSet: 6
        Gerrit-Owner: Jan Keitel <jke...@google.com>
        Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
        Gerrit-Reviewer: Jan Keitel <jke...@google.com>
        Gerrit-CC: Ireneusz Szulc <ireneu...@google.com>
        Gerrit-CC: Rahim Mammadli <rah...@google.com>
        Gerrit-CC: Sophie Chang <sophi...@chromium.org>
        Gerrit-Attention: Jan Keitel <jke...@google.com>
        Gerrit-Comment-Date: Wed, 08 Jul 2026 11:00:31 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        Comment-In-Reply-To: Jan Keitel <jke...@google.com>
        Comment-In-Reply-To: Christoph Schwering <schw...@google.com>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Jan Keitel (Gerrit)

        unread,
        7:06 AM (3 hours ago) 7:06 AM
        to Sophie Chang, Rahim Mammadli, Ireneusz Szulc, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
        Attention needed from Christoph Schwering

        Jan Keitel added 1 comment

        File components/autofill/core/browser/ui/autofill_external_delegate.cc
        Line 605, Patchset 3: !parent_suggestion_metadata) {
        Christoph Schwering . unresolved

        Why? I don't understand it intuitivel.

        Isn't it true because of the early return above?

        Jan Keitel

        Please look again. 😊

        Christoph Schwering

        Why do we want to do the `else` branch even if `is_subopop`?

        Jan Keitel

        We don't, but autocomplete entries never appear in sub-popups so it doesn't matter whether you include it. I could also add an `if (!parent_suggestion_metadata)` around the entire block. Preferences?

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Christoph Schwering
        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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
        Gerrit-Change-Number: 8052191
        Gerrit-PatchSet: 6
        Gerrit-Owner: Jan Keitel <jke...@google.com>
        Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
        Gerrit-Reviewer: Jan Keitel <jke...@google.com>
        Gerrit-CC: Ireneusz Szulc <ireneu...@google.com>
        Gerrit-CC: Rahim Mammadli <rah...@google.com>
        Gerrit-CC: Sophie Chang <sophi...@chromium.org>
        Gerrit-Attention: Christoph Schwering <schw...@google.com>
        Gerrit-Comment-Date: Wed, 08 Jul 2026 11:06:22 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Christoph Schwering (Gerrit)

        unread,
        7:16 AM (3 hours ago) 7:16 AM
        to Jan Keitel, Sophie Chang, Rahim Mammadli, Ireneusz Szulc, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
        Attention needed from Jan Keitel

        Christoph Schwering added 1 comment

        File components/autofill/core/browser/ui/autofill_external_delegate.cc
        Line 605, Patchset 3: !parent_suggestion_metadata) {
        Christoph Schwering . unresolved

        Why? I don't understand it intuitivel.

        Isn't it true because of the early return above?

        Jan Keitel

        Please look again. 😊

        Christoph Schwering

        Why do we want to do the `else` branch even if `is_subopop`?

        Jan Keitel

        We don't, but autocomplete entries never appear in sub-popups so it doesn't matter whether you include it. I could also add an `if (!parent_suggestion_metadata)` around the entire block. Preferences?

        Christoph Schwering

        I thought so, but I'd rather put it around the entire block.

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Jan Keitel
        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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
        Gerrit-Change-Number: 8052191
        Gerrit-PatchSet: 6
        Gerrit-Owner: Jan Keitel <jke...@google.com>
        Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
        Gerrit-Reviewer: Jan Keitel <jke...@google.com>
        Gerrit-CC: Ireneusz Szulc <ireneu...@google.com>
        Gerrit-CC: Rahim Mammadli <rah...@google.com>
        Gerrit-CC: Sophie Chang <sophi...@chromium.org>
        Gerrit-Attention: Jan Keitel <jke...@google.com>
        Gerrit-Comment-Date: Wed, 08 Jul 2026 11:16:04 +0000
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Jan Keitel (Gerrit)

        unread,
        7:28 AM (2 hours ago) 7:28 AM
        to Sophie Chang, Rahim Mammadli, Ireneusz Szulc, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org

        Jan Keitel voted and added 2 comments

        Votes added by Jan Keitel

        Commit-Queue+2

        2 comments

        Patchset-level comments
        File-level comment, Patchset 7 (Latest):
        Jan Keitel . resolved

        Thanks!

        File components/autofill/core/browser/ui/autofill_external_delegate.cc
        Line 605, Patchset 3: !parent_suggestion_metadata) {
        Christoph Schwering . resolved

        Why? I don't understand it intuitivel.

        Isn't it true because of the early return above?

        Jan Keitel

        Please look again. 😊

        Christoph Schwering

        Why do we want to do the `else` branch even if `is_subopop`?

        Jan Keitel

        We don't, but autocomplete entries never appear in sub-popups so it doesn't matter whether you include it. I could also add an `if (!parent_suggestion_metadata)` around the entire block. Preferences?

        Christoph Schwering

        I thought so, but I'd rather put it around the entire block.

        Jan Keitel

        Done

        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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
          Gerrit-Change-Number: 8052191
          Gerrit-PatchSet: 7
          Gerrit-Owner: Jan Keitel <jke...@google.com>
          Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
          Gerrit-Reviewer: Jan Keitel <jke...@google.com>
          Gerrit-CC: Ireneusz Szulc <ireneu...@google.com>
          Gerrit-CC: Rahim Mammadli <rah...@google.com>
          Gerrit-CC: Sophie Chang <sophi...@chromium.org>
          Gerrit-Comment-Date: Wed, 08 Jul 2026 11:28:13 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: Yes
          satisfied_requirement
          open
          diffy

          Chromium LUCI CQ (Gerrit)

          unread,
          8:23 AM (2 hours ago) 8:23 AM
          to Jan Keitel, Sophie Chang, Rahim Mammadli, Ireneusz Szulc, 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/ui/autofill_external_delegate.cc
          Insertions: 12, Deletions: 11.

          The diff is too large to show. Please review the diff.
          ```

          Change information

          Commit message:
          Pipe through SuggestionMetadata to AtMemory

          This CL connects the SuggestionMetadata received in AED to
          AtMemoryMetricsRecorder.

          The CL is intended to be a no-op.
          Bug: 532065749
          Change-Id: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
          Reviewed-by: Christoph Schwering <schw...@google.com>
          Commit-Queue: Jan Keitel <jke...@google.com>
          Cr-Commit-Position: refs/heads/main@{#1658663}
          Files:
          • M chrome/browser/touch_to_fill/autofill/android/touch_to_fill_payment_method_delegate_android_impl.cc
          • M components/autofill/core/browser/at_memory/at_memory_manager.cc
          • M components/autofill/core/browser/at_memory/at_memory_manager.h
          • M components/autofill/core/browser/at_memory/at_memory_manager_unittest.cc
          • M components/autofill/core/browser/at_memory/at_memory_metrics_recorder.cc
          • M components/autofill/core/browser/at_memory/at_memory_metrics_recorder.h
          • M components/autofill/core/browser/at_memory/at_memory_metrics_recorder_unittest.cc
          • M components/autofill/core/browser/data_manager/addresses/account_name_email_strike_manager_unittest.cc
          • 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_unittest.cc
          • M components/autofill/core/browser/metrics/autofill_metrics_test_base.h
          • M components/autofill/core/browser/metrics/form_events/address_form_event_logger_unittest.cc
          • M components/autofill/core/browser/ui/autofill_external_delegate.cc
          Change size: L
          Delta: 14 files changed, 199 insertions(+), 90 deletions(-)
          Branch: refs/heads/main
          Submit Requirements:
          • requirement satisfiedCode-Review: +1 by Christoph Schwering
          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: I3a8bd595d0cd1c7d509ab1995dde2d0387a8b73c
          Gerrit-Change-Number: 8052191
          Gerrit-PatchSet: 8
          Gerrit-Owner: Jan Keitel <jke...@google.com>
          Gerrit-Reviewer: Christoph Schwering <schw...@google.com>
          Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
          Gerrit-Reviewer: Jan Keitel <jke...@google.com>
          open
          diffy
          satisfied_requirement
          Reply all
          Reply to author
          Forward
          0 new messages