[HybridInAutofill] Allow footer entry point for all dropdowns [chromium/src : main]
0 views
Skip to first unread message
Friedrich Hauser (Gerrit)
Jan 15, 2026, 11:28:47 AM (2 days ago) Jan 15
to Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Christoph Schwering
Friedrich Hauser added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Friedrich Hauser . resolved
Hi Chris,
Would you please take a look at this bug fix?
Related details
Attention is currently required from:
- Christoph Schwering
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christoph Schwering (Gerrit)
Jan 15, 2026, 4:41:04 PM (2 days ago) Jan 15
to Friedrich Hauser, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Friedrich Hauser
Christoph Schwering added 1 comment![]( "Open in Gerrit")
File components/autofill/core/browser/foundations/browser_autofill_manager.cc
Line 1415, Patchset 2 (Latest):
&BrowserAutofillManager::GenerateFooter, weak_ptr_factory_.GetWeakPtr(),Christoph Schwering . unresolved
I know this is nitpicky, but since I find the chain of suggestion generation functions (OnAskForValuesToFillImpl() --> phase 1 --> phase 2 --> phase 3 --> OnGenerateSuggestionsComplete()) intimidating, I wonder if one of the following would be an improvement:
- Option 1: Call GenerateFooter() from OnGenerateSuggestionsComplete().
I assume your reasoning behind making it a separate step is that the suggestions are not complete before the footer is added. That makes sense to me, but I wonder if the chain of suggestion generation functions is more digestible if we cheat and just do this in OnGenerateSuggestionsComplete().
- Option 2: OnGenerateSuggestionsComplete() currently calls ReorderWebauthnFallbackToFooter(). If we want to make GenerateFooter() a separate step in the chain, shouldn't that step include ReorderWebauthnFallbackToFooter()?
No strong preference between the two.
Related details
Attention is currently required from:
- Friedrich Hauser
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. |
Friedrich Hauser (Gerrit)
Jan 16, 2026, 4:53:34 AM (yesterday) Jan 16
to Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Christoph Schwering
Friedrich Hauser added 1 comment![]( "Open in Gerrit")
File components/autofill/core/browser/foundations/browser_autofill_manager.cc
Line 1415, Patchset 2 (Latest):
&BrowserAutofillManager::GenerateFooter, weak_ptr_factory_.GetWeakPtr(),Christoph Schwering . resolved
I know this is nitpicky, but since I find the chain of suggestion generation functions (OnAskForValuesToFillImpl() --> phase 1 --> phase 2 --> phase 3 --> OnGenerateSuggestionsComplete()) intimidating, I wonder if one of the following would be an improvement:
- Option 1: Call GenerateFooter() from OnGenerateSuggestionsComplete().
I assume your reasoning behind making it a separate step is that the suggestions are not complete before the footer is added. That makes sense to me, but I wonder if the chain of suggestion generation functions is more digestible if we cheat and just do this in OnGenerateSuggestionsComplete().
- Option 2: OnGenerateSuggestionsComplete() currently calls ReorderWebauthnFallbackToFooter(). If we want to make GenerateFooter() a separate step in the chain, shouldn't that step include ReorderWebauthnFallbackToFooter()?No strong preference between the two.
Friedrich Hauser
I technically agree with you. My first draft just added the Footer at the correct position with no reordering at all. The issue is that with kAutofillNewSuggestionGeneration, there isn't one but two ways to generate suggestions and this Footer is no exception:
```
(OnAskForValuesToFillImpl()
|
| <with kAutofillNewSuggestionGeneration>
|-------------------------------------------|
V |
GenerateSuggestionsAndMaybeShowUIPhase1 (uses PasskeySuggestionGenerator)
| |
V V
GenerateSuggestionsAndMaybeShowUIPhase2 OnSuggestionDataFetched
| |
V V
GenerateSuggestionsAndMaybeShowUIPhase3 OnIndividualSuggestionsGenerated
| |
V V
OnGenerateSuggestionsComplete())
```
And since kAutofillNewSuggestionGeneration is arguably the modern way and eventual default behavior, I don't think we can just cheat the option into OnGenerateSuggestionsComplete(). Instead, I'd expect to eventually move the ordering to OnIndividualSuggestionsGenerated where suggestions are merged. For now, adding the footer and placing it correctly need to be two (three) separate steps though.
Related details
Attention is currently required from:
- Christoph Schwering
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christoph Schwering (Gerrit)
Jan 16, 2026, 5:44:13 AM (yesterday) Jan 16
to Friedrich Hauser, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Friedrich Hauser
Christoph Schwering voted and added 1 comment![]( "Open in Gerrit")
Votes added by Christoph Schwering
| Code-Review | +1 |
1 comment
File components/autofill/core/browser/foundations/browser_autofill_manager.cc
Line 1415, Patchset 2 (Latest):
&BrowserAutofillManager::GenerateFooter, weak_ptr_factory_.GetWeakPtr(),Christoph Schwering . resolved
Friedrich HauserI know this is nitpicky, but since I find the chain of suggestion generation functions (OnAskForValuesToFillImpl() --> phase 1 --> phase 2 --> phase 3 --> OnGenerateSuggestionsComplete()) intimidating, I wonder if one of the following would be an improvement:
- Option 1: Call GenerateFooter() from OnGenerateSuggestionsComplete().
I assume your reasoning behind making it a separate step is that the suggestions are not complete before the footer is added. That makes sense to me, but I wonder if the chain of suggestion generation functions is more digestible if we cheat and just do this in OnGenerateSuggestionsComplete().
- Option 2: OnGenerateSuggestionsComplete() currently calls ReorderWebauthnFallbackToFooter(). If we want to make GenerateFooter() a separate step in the chain, shouldn't that step include ReorderWebauthnFallbackToFooter()?No strong preference between the two.
I technically agree with you. My first draft just added the Footer at the correct position with no reordering at all. The issue is that with kAutofillNewSuggestionGeneration, there isn't one but two ways to generate suggestions and this Footer is no exception:
```
(OnAskForValuesToFillImpl()
|
| <with kAutofillNewSuggestionGeneration>
|-------------------------------------------|
V |
GenerateSuggestionsAndMaybeShowUIPhase1 (uses PasskeySuggestionGenerator)
| |
V V
GenerateSuggestionsAndMaybeShowUIPhase2 OnSuggestionDataFetched
| |
V V
GenerateSuggestionsAndMaybeShowUIPhase3 OnIndividualSuggestionsGenerated
| |
V V
OnGenerateSuggestionsComplete())
```And since kAutofillNewSuggestionGeneration is arguably the modern way and eventual default behavior, I don't think we can just cheat the option into OnGenerateSuggestionsComplete(). Instead, I'd expect to eventually move the ordering to OnIndividualSuggestionsGenerated where suggestions are merged. For now, adding the footer and placing it correctly need to be two (three) separate steps though.
Attention is currently required from:
- Friedrich Hauser
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Friedrich Hauser (Gerrit)
Jan 16, 2026, 5:45:16 AM (yesterday) Jan 16
to Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Friedrich Hauser voted and added 1 comment![]( "Open in Gerrit")
Votes added by Friedrich Hauser
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Friedrich Hauser . resolved
Thanks for the swift review!
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. |
Chromium LUCI CQ (Gerrit)
Jan 16, 2026, 5:48:03 AM (yesterday) Jan 16
to Friedrich Hauser, chromium...@chromium.org, browser-comp...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[HybridInAutofill] Allow footer entry point for all dropdowns
Before this change, the new entry point would suppress suggestions that
require no other suggestions to be present, most notably: Autocomplete.
This change ensures the footer is always added in a last step which
allows to show Autocomplete suggestions again even if the hybrid entry
point must be shown.
Tested with and without kAutofillNewSuggestionGeneration enabled.
Screenshot: http://screen/B6vaDk8cKN9KdPB.png
Feature: AutofillReintroduceHybridPasskeyDropdownItem
Bug: 399131928, 399124614
Change-Id: I35d35646860f7857b2c3e2c7edeee63087da4c34
Fixed: 476010275
Reviewed-by: Christoph Schwering <schw...@google.com>
Commit-Queue: Friedrich Hauser <fried...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1570276}
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_unittest.cc
Change size: M
Delta: 3 files changed, 93 insertions(+), 24 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Christoph Schwering
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chrome Crash (Prod) (Gerrit)
5:33 AM (5 hours ago) 5:33 AM
to Chromium LUCI CQ, Friedrich Hauser, chromium...@chromium.org, browser-comp...@chromium.org
Chrome Crash (Prod) has created a revert of this change![]( "Open in Gerrit")
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