New issue
Advanced search Search tips

Issue 915223 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Pull-up Autocomplete History Manager to be a KeyedService

Project Member Reported by seblalancette@chromium.org, Dec 14

Issue description

We don't need one AutocompleteHistoryManager per frame; one per browser context would be sufficient.

How to test:
Validate in the following clients:
- Desktop chrome,
- iOS chrome,
- iOS WebView,
- Android WebView

Things to validate:
- Autocomplete works as expected (can retrieve old input, can save new input),
- Autocomplete in Incognito doesn't let you save new input.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecaff4ddd80cde0e9663a65f58989714c9d070e5

commit ecaff4ddd80cde0e9663a65f58989714c9d070e5
Author: Sebastien Lalancette <seblalancette@chromium.org>
Date: Mon Dec 17 22:38:06 2018

Pulled-up AutocompleteHistoryManager to be a KeyedService.

Autocomplete suggestions are scoped under a Profile, but the
AutocompleteHistoryManager was living under a Frame context. Pulling it up to
live at the BrowserContext level makes it represent the reality better. It will
also allow us to run "one-time" initialization, such as logging metrics for
local data once on start-up.

To keep in mind:
- Since we don't want to load all autocomplete entries into memory,
  autocomplete suggestions are always fetched asynchronously.

Before:
- AutocompleteHistoryManager lived in AutofillManager,
- AutofillManager gave an instance of ExternalDelegate to
  AutocompleteHistoryManager,
- Whenever AutocompleteHistoryManager asynchronously received suggestions, it
  forwarded them to the given ExternalDelegate.
- All three instances are within the context of a Frame.

After:
- AutocompleteHistoryManager lives as a KeyedService, and is passed as a ctor
  parameter to AutofillManager.
- AutofillManager implements the AutocompleteHistoryManager::SuggestionsHandler
  interface.
- Whenever an AutofillManager instance wants to fetch autocomplete suggestions,
  it passes itself as an observer of the query.
- AutocompleteHistoryManager keeps a map of DB query IDs (unique) to observers,
  and updates them upon receiving the suggestions.
- When AutofillManager is invoked after receiving the suggestions, it forwards
  them to its ExternalDelegate instance, allowing per Frame rendering.

Bug:  915223 
Bug:  907902 
Change-Id: I9e3d257269e4918a6ce29bcbbb2ec961afccaab1
Reviewed-on: https://chromium-review.googlesource.com/c/1351434
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617267}
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/android_webview/browser/aw_autofill_client.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/android_webview/browser/aw_autofill_client.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/android_webview/browser/aw_browser_context.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/android_webview/browser/aw_browser_context.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/chrome/browser/BUILD.gn
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/chrome/browser/autofill/autocomplete_history_manager_factory.cc
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/chrome/browser/autofill/autocomplete_history_manager_factory.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/chrome/browser/ui/autofill/chrome_autofill_client.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/chrome/browser/ui/autofill/chrome_autofill_client.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/BUILD.gn
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autocomplete_history_manager.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autocomplete_history_manager.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autocomplete_history_manager_unittest.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autofill_assistant_unittest.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autofill_client.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autofill_external_delegate_unittest.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autofill_manager.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autofill_manager_unittest.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/credit_card_save_manager_unittest.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/local_card_migration_manager_unittest.cc
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/mock_autocomplete_history_manager.cc
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/mock_autocomplete_history_manager.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/test_autofill_client.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/test_autofill_client.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/test_autofill_manager.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/autofill/core/browser/test_autofill_manager.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/components/password_manager/core/browser/password_form_manager_unittest.cc
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/chrome/browser/autofill/BUILD.gn
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/chrome/browser/autofill/autocomplete_history_manager_factory.cc
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/chrome/browser/autofill/autocomplete_history_manager_factory.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.mm
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/web_view/BUILD.gn
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/web_view/internal/autofill/cwv_autofill_controller.mm
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/web_view/internal/autofill/web_view_autocomplete_history_manager_factory.cc
[add] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/web_view/internal/autofill/web_view_autocomplete_history_manager_factory.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/web_view/internal/autofill/web_view_autofill_client_ios.h
[modify] https://crrev.com/ecaff4ddd80cde0e9663a65f58989714c9d070e5/ios/web_view/internal/autofill/web_view_autofill_client_ios.mm

Status: Fixed (was: Started)
Cc: satyavat...@chromium.org michaelbai@chromium.org
RE: Adding to my test guidelines:

How to test:
Validate in the following clients:
- Desktop chrome,
- iOS chrome,
- iOS WebView,
- Android WebView

Things to validate:
- Autocomplete works as expected (can retrieve old input, can save new input),
- Autocomplete in Incognito doesn't let you save new input.

How to validate:
- Navigate to this website: https://rsolomakhin.github.io/autofill/
- Input data into box "Basic form key/value Autocomplete (unstructured data)"
- Data in pop-up showing at that location means it was saved as autocomplete data.
- Use the "Submit" button to try and save an input.
Components: Mobile>WebView
satyavathir@, this patch changed WebView autocomplete which hasn't been touched for a long time, please add it to your test plan, this only work in Android N and before.
Webview side verifed on 
1. Samsung A7/LMY55X and 2. Samsung S2/NRD90M 

With webview version : 72.0.3626.28 
dneelamegam@ you need to verify it in m73, this patch just landed.
Verified/Fix on Sony Xperia Z3+/7.1.1 / Webview : 73.0.3664.2

Sign in to add a comment