Skip to content

Commit

Permalink
[Autofill] Allow silent profile updates for importing partial profiles
Browse files Browse the repository at this point in the history 
Currently, on form submission, a profile is imported when it satisfies
the import requirements such as having a valid country and having
complete address information etc.
This change lifts those requirements allowing for silent updates
provided that they contain any structured information related to name or
address.

Bug: 1234768
Change-Id: I452f5e58c9a6a6f405a7ce5ddf2f1ba60310fe05
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3067339
Commit-Queue: Vidhan Jain <vidhanj@google.com>
Reviewed-by: Matthias Körber <koerber@google.com>
Cr-Commit-Position: refs/heads/master@{#911021}
  • Loading branch information
Vidhan authored and Chromium LUCI CQ committed Aug 11, 2021
1 parent 52b5b63 commit 91ec1c4
Show file tree
Hide file tree
Showing 13 changed files with 632 additions and 39 deletions.
14 changes: 10 additions & 4 deletions components/autofill/core/browser/address_profile_save_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,26 @@ AddressProfileSaveManager::~AddressProfileSaveManager() = default;
void AddressProfileSaveManager::ImportProfileFromForm(
const AutofillProfile& observed_profile,
const std::string& app_locale,
const GURL& url) {
const GURL& url,
bool allow_only_silent_updates) {
// Without a personal data manager, profile storage is not possible.
if (!personal_data_manager_)
return;

// If the explicit save prompts are not enabled, revert back to the legacy
// behavior and directly import the observed profile without recording any
// additional metrics.
// additional metrics. However, if only silent updates are allowed, proceed
// with the profile import process.
if (!base::FeatureList::IsEnabled(
features::kAutofillAddressProfileSavePrompt)) {
features::kAutofillAddressProfileSavePrompt) &&
!allow_only_silent_updates) {
personal_data_manager_->SaveImportedProfile(observed_profile);
return;
}

auto process_ptr = std::make_unique<ProfileImportProcess>(
observed_profile, app_locale, url, personal_data_manager_);
observed_profile, app_locale, url, personal_data_manager_,
allow_only_silent_updates);

MaybeOfferSavePrompt(std::move(process_ptr));
}
Expand All @@ -52,9 +56,11 @@ void AddressProfileSaveManager::MaybeOfferSavePrompt(
// process without initiating a user prompt
case AutofillProfileImportType::kDuplicateImport:
case AutofillProfileImportType::kSilentUpdate:
case AutofillProfileImportType::kSilentUpdateForIncompleteProfile:
case AutofillProfileImportType::kSuppressedNewProfile:
case AutofillProfileImportType::kSuppressedConfirmableMergeAndSilentUpdate:
case AutofillProfileImportType::kSuppressedConfirmableMerge:
case AutofillProfileImportType::kUnusableIncompleteProfile:
import_process->AcceptWithoutPrompt();
FinalizeProfileImport(std::move(import_process));
return;
Expand Down
6 changes: 5 additions & 1 deletion components/autofill/core/browser/address_profile_save_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ class AddressProfileSaveManager {
// change of an existing profile that must be confirmed by the user, a UI
// prompt will be initiated. At the end of the process, metrics will be
// recorded.
// |allow_only_silent_updates| allows only for silent updates of profiles
// that have either a structured name or address or both but do not fulfill
// the import requirements.
void ImportProfileFromForm(const AutofillProfile& profile,
const std::string& app_locale,
const GURL& url);
const GURL& url,
bool allow_only_silent_updates);

protected:
// Initiates showing the prompt to the user.
Expand Down
227 changes: 224 additions & 3 deletions components/autofill/core/browser/address_profile_save_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ struct ImportScenarioTestCase {
bool new_profiles_suppresssed_for_domain;
std::vector<std::string> blocked_guids_for_updates;
absl::optional<AutofillProfile> profile_to_be_added_while_waiting;
bool allow_only_silent_updates = false;
};

class AddressProfileSaveManagerTest : public testing::Test {
Expand Down Expand Up @@ -225,8 +226,9 @@ void AddressProfileSaveManagerTest::TestImportScenario(
mock_personal_data_manager_.SetProfiles(&test_scenario.existing_profiles);

// Initiate the profile import.
save_manager.ImportProfileFromForm(test_scenario.observed_profile, "en-US",
url);
save_manager.ImportProfileFromForm(
test_scenario.observed_profile, "en-US", url,
/*allow_only_silent_updates=*/test_scenario.allow_only_silent_updates);

// Assert that there is a finished import process on record.
ASSERT_NE(save_manager.last_import(), nullptr);
Expand Down Expand Up @@ -972,7 +974,226 @@ TEST_F(AddressProfileSaveManagerTest, SaveProfileWhenNoSavePrompt) {
AutofillProfile test_profile = test::GetFullProfile();
EXPECT_CALL(mock_personal_data_manager_, SaveImportedProfile(test_profile));
save_manager.ImportProfileFromForm(test_profile, "en_US",
GURL("https://www.noprompt.com"));
GURL("https://www.noprompt.com"),
/*allow_only_silent_updates=*/false);
}

// Tests that a new profile is not imported when only silent updates are
// allowed.
TEST_F(AddressProfileSaveManagerTest, SilentlyUpdateProfile_SaveNewProfile) {
AutofillProfile observed_profile = test::StandardProfile();

ImportScenarioTestCase test_scenario{
.existing_profiles = {},
.observed_profile = observed_profile,
.is_prompt_expected = false,
.user_decision = UserDecision::kUserNotAsked,
.expected_import_type =
AutofillProfileImportType::kUnusableIncompleteProfile,
.is_profile_change_expected = false,
.expected_final_profiles = {},
.allow_only_silent_updates = true};

TestImportScenario(test_scenario);
}

// Test that the observation of quasi identical profile that has a different
// structure in the name will result in a silent update when only silent updates
// are allowed.
TEST_F(AddressProfileSaveManagerTest,
SilentlyUpdateProfile_WithIncompleteProfile) {
AutofillProfile observed_profile = test::StandardProfile();
AutofillProfile updateable_profile = test::UpdateableStandardProfile();
AutofillProfile final_profile = observed_profile;
test::CopyGUID(updateable_profile, &final_profile);

ImportScenarioTestCase test_scenario{
.existing_profiles = {updateable_profile},
.observed_profile = observed_profile,
.is_prompt_expected = false,
.expected_import_type =
AutofillProfileImportType::kSilentUpdateForIncompleteProfile,
.is_profile_change_expected = true,
.expected_final_profiles = {final_profile},
.allow_only_silent_updates = true};
TestImportScenario(test_scenario);
}

// Tests that the profile's structured name information is silently updated when
// an updated profile is observed with no settings visible difference.
// Silent Update is enabled for the test.
TEST_F(AddressProfileSaveManagerTest,
SilentlyUpdateProfile_UpdateStructuredName) {
AutofillProfile updateable_profile;
test::SetProfileTestValues(
&updateable_profile,
{{NAME_FULL, "AAA BBB CCC", VerificationStatus::kObserved},
{NAME_FIRST, "AAA", VerificationStatus::kParsed},
{NAME_MIDDLE, "BBB", VerificationStatus::kParsed},
{NAME_LAST, "CCC", VerificationStatus::kParsed},
{ADDRESS_HOME_STREET_ADDRESS, "119 Some Avenue",
VerificationStatus::kObserved},
{ADDRESS_HOME_COUNTRY, "US", VerificationStatus::kObserved},
{ADDRESS_HOME_STATE, "CA", VerificationStatus::kObserved},
{ADDRESS_HOME_ZIP, "99666", VerificationStatus::kObserved},
{ADDRESS_HOME_CITY, "Los Angeles", VerificationStatus::kObserved}});

AutofillProfile observed_profile;
test::SetProfileTestValues(
&observed_profile,
{{NAME_FULL, "AAA BBB CCC", VerificationStatus::kObserved},
{NAME_FIRST, "AAA", VerificationStatus::kParsed},
{NAME_MIDDLE, "", VerificationStatus::kParsed},
{NAME_LAST, "BBB CCC", VerificationStatus::kParsed},
{ADDRESS_HOME_STREET_ADDRESS, "119 Some Avenue",
VerificationStatus::kObserved},
{ADDRESS_HOME_COUNTRY, "US", VerificationStatus::kObserved},
{ADDRESS_HOME_STATE, "CA", VerificationStatus::kObserved},
{ADDRESS_HOME_ZIP, "99666", VerificationStatus::kObserved},
{ADDRESS_HOME_CITY, "Los Angeles", VerificationStatus::kObserved}});

AutofillProfile final_profile = observed_profile;
test::CopyGUID(updateable_profile, &final_profile);

ImportScenarioTestCase test_scenario{
.existing_profiles = {updateable_profile},
.observed_profile = observed_profile,
.is_prompt_expected = false,
.expected_import_type =
AutofillProfileImportType::kSilentUpdateForIncompleteProfile,
.is_profile_change_expected = true,
.expected_final_profiles = {final_profile},
.allow_only_silent_updates = true};
TestImportScenario(test_scenario);
}

// Tests that the profile's structured name information is silently updated when
// an updated profile with no address data is observed with no settings visible
// difference.
// Silent Update is enabled for the test.
TEST_F(AddressProfileSaveManagerTest,
SilentlyUpdateProfile_UpdateStructuredNameWithIncompleteProfile) {
AutofillProfile updateable_profile;
test::SetProfileTestValues(
&updateable_profile,
{{NAME_FULL, "AAA BBB CCC", VerificationStatus::kObserved},
{NAME_FIRST, "AAA", VerificationStatus::kParsed},
{NAME_MIDDLE, "BBB", VerificationStatus::kParsed},
{NAME_LAST, "CCC", VerificationStatus::kParsed},
{ADDRESS_HOME_STREET_ADDRESS, "119 Some Avenue",
VerificationStatus::kObserved},
{ADDRESS_HOME_COUNTRY, "US", VerificationStatus::kObserved},
{ADDRESS_HOME_STATE, "CA", VerificationStatus::kObserved},
{ADDRESS_HOME_ZIP, "99666", VerificationStatus::kObserved},
{ADDRESS_HOME_CITY, "Los Angeles", VerificationStatus::kObserved}});

AutofillProfile observed_profile;
test::SetProfileTestValues(
&observed_profile,
{{NAME_FULL, "AAA BBB CCC", VerificationStatus::kObserved},
{NAME_FIRST, "AAA", VerificationStatus::kParsed},
{NAME_MIDDLE, "", VerificationStatus::kParsed},
{NAME_LAST, "BBB CCC", VerificationStatus::kParsed}});

AutofillProfile final_profile;
test::SetProfileTestValues(
&final_profile,
{{NAME_FULL, "AAA BBB CCC", VerificationStatus::kObserved},
{NAME_FIRST, "AAA", VerificationStatus::kParsed},
{NAME_MIDDLE, "", VerificationStatus::kParsed},
{NAME_LAST, "BBB CCC", VerificationStatus::kParsed},
{ADDRESS_HOME_STREET_ADDRESS, "119 Some Avenue",
VerificationStatus::kObserved},
{ADDRESS_HOME_COUNTRY, "US", VerificationStatus::kObserved},
{ADDRESS_HOME_STATE, "CA", VerificationStatus::kObserved},
{ADDRESS_HOME_ZIP, "99666", VerificationStatus::kObserved},
{ADDRESS_HOME_CITY, "Los Angeles", VerificationStatus::kObserved}});
test::CopyGUID(updateable_profile, &final_profile);

ImportScenarioTestCase test_scenario{
.existing_profiles = {updateable_profile},
.observed_profile = observed_profile,
.is_prompt_expected = false,
.expected_import_type =
AutofillProfileImportType::kSilentUpdateForIncompleteProfile,
.is_profile_change_expected = true,
.expected_final_profiles = {final_profile},
.allow_only_silent_updates = true};
TestImportScenario(test_scenario);
}

// Test that the observation of quasi identical profile that has a different
// structure in the name will result in a silent update even though the domain
// is blocked for new profile imports when silent updates are enforced.
TEST_F(AddressProfileSaveManagerTest, SilentlyUpdateProfile_OnBlockedDomain) {
AutofillProfile observed_profile = test::StandardProfile();
AutofillProfile updateable_profile = test::UpdateableStandardProfile();
AutofillProfile final_profile = observed_profile;
test::CopyGUID(updateable_profile, &final_profile);

ImportScenarioTestCase test_scenario{
.existing_profiles = {updateable_profile},
.observed_profile = observed_profile,
.is_prompt_expected = false,
.expected_import_type =
AutofillProfileImportType::kSilentUpdateForIncompleteProfile,
.is_profile_change_expected = true,
.expected_final_profiles = {final_profile},
.new_profiles_suppresssed_for_domain = true,
.allow_only_silent_updates = true};
TestImportScenario(test_scenario);
}

// Test a mixed scenario in which a duplicate profile already exists, but
// another profile is mergeable with the observed profile and yet another
// profile can be silently updated. Only silent updates are allowed for this
// test.
TEST_F(AddressProfileSaveManagerTest,
SilentlyUpdateProfile_UserConfirmableMergeAndUpdateAndDuplicate) {
AutofillProfile observed_profile = test::StandardProfile();
AutofillProfile existing_duplicate = test::StandardProfile();
AutofillProfile updateable_profile = test::UpdateableStandardProfile();
AutofillProfile mergeable_profile = test::SubsetOfStandardProfile();

// The updateable profile should have the same value as the observed profile.
AutofillProfile updated_profile = observed_profile;
// However, the GUIDs must be maintained.
test::CopyGUID(updateable_profile, &updated_profile);

ImportScenarioTestCase test_scenario{
.existing_profiles = {existing_duplicate, mergeable_profile,
updateable_profile},
.observed_profile = observed_profile,
.is_prompt_expected = false,
.expected_import_type =
AutofillProfileImportType::kSilentUpdateForIncompleteProfile,
.is_profile_change_expected = true,
.expected_final_profiles = {existing_duplicate, mergeable_profile,
updated_profile},
.allow_only_silent_updates = true};

TestImportScenario(test_scenario);
}

// Tests that the mergeable profiles are not merged when only silent updates
// are allowed.
TEST_F(AddressProfileSaveManagerTest,
SilentlyUpdateProfile_UserConfirmableMergeNotAllowed) {
AutofillProfile observed_profile = test::StandardProfile();
AutofillProfile mergeable_profile = test::SubsetOfStandardProfile();

ImportScenarioTestCase test_scenario{
.existing_profiles = {mergeable_profile},
.observed_profile = observed_profile,
.is_prompt_expected = false,
.user_decision = UserDecision::kUserNotAsked,
.expected_import_type =
AutofillProfileImportType::kUnusableIncompleteProfile,
.is_profile_change_expected = true,
.expected_final_profiles = {mergeable_profile},
.allow_only_silent_updates = true};

TestImportScenario(test_scenario);
}

} // namespace
Expand Down
32 changes: 24 additions & 8 deletions components/autofill/core/browser/autofill_profile_import_process.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ ProfileImportProcess::ProfileImportProcess(
const AutofillProfile& observed_profile,
const std::string& app_locale,
const GURL& form_source_url,
const PersonalDataManager* personal_data_manager)
const PersonalDataManager* personal_data_manager,
bool allow_only_silent_updates)
: import_id_(GetImportId()),
observed_profile_(observed_profile),
app_locale_(app_locale),
form_source_url_(form_source_url),
personal_data_manager_(personal_data_manager) {
personal_data_manager_(personal_data_manager),
allow_only_silent_updates_(allow_only_silent_updates) {
DetermineProfileImportType();
}

Expand Down Expand Up @@ -105,6 +107,11 @@ void ProfileImportProcess::DetermineProfileImportType() {
// user confirmation.
if (AutofillProfileComparator::ProfilesHaveDifferentSettingsVisibleValues(
*existing_profile, merged_profile)) {
if (allow_only_silent_updates_) {
++number_of_unchanged_profiles;
continue;
}

// Determine if the existing profile is blocked for updates.
// If the personal data manager is not available the profile is considered
// as not blocked.
Expand Down Expand Up @@ -139,13 +146,17 @@ void ProfileImportProcess::DetermineProfileImportType() {
// If the profile is not mergeable with an existing profile, the import
// corresponds to a new profile.
if (!is_mergeable_with_existing_profile) {
// There should be no import candidate yet.
DCHECK(!import_candidate_.has_value());
if (new_profiles_suppressed_for_domain_) {
import_type_ = AutofillProfileImportType::kSuppressedNewProfile;
if (!allow_only_silent_updates_) {
// There should be no import candidate yet.
DCHECK(!import_candidate_.has_value());
if (new_profiles_suppressed_for_domain_) {
import_type_ = AutofillProfileImportType::kSuppressedNewProfile;
} else {
import_type_ = AutofillProfileImportType::kNewProfile;
import_candidate_ = observed_profile();
}
} else {
import_type_ = AutofillProfileImportType::kNewProfile;
import_candidate_ = observed_profile();
import_type_ = AutofillProfileImportType::kUnusableIncompleteProfile;
}
} else {
bool silent_updates_present = updated_profiles_.size() > 0;
Expand All @@ -161,6 +172,11 @@ void ProfileImportProcess::DetermineProfileImportType() {
? AutofillProfileImportType::
kSuppressedConfirmableMergeAndSilentUpdate
: AutofillProfileImportType::kSuppressedConfirmableMerge;
} else if (allow_only_silent_updates_) {
import_type_ =
silent_updates_present
? AutofillProfileImportType::kSilentUpdateForIncompleteProfile
: AutofillProfileImportType::kUnusableIncompleteProfile;
} else {
import_type_ = silent_updates_present
? AutofillProfileImportType::kSilentUpdate
Expand Down
Loading

0 comments on commit 91ec1c4

Please sign in to comment.