Commit 77709ee6 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Chromium LUCI CQ

[Autofill] Open the Address Profile Save Prompt when feature is enabled

This CL checks the kAutofillAddressProfileSavePrompt feature in
AddressProfileSaveManager::SaveProfile()

If the feature is disabled, it invokes the personal data manager
directly. (No behavioral change)

If the feature is enabled, it invokes the autofill client method that
opens the save prompt.

Bug: 1135178
Change-Id: I36fa5ec8d45d88a27d66d89c17f12f09f7bcc2a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2631500
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarMatthias Körber <koerber@google.com>
Cr-Commit-Position: refs/heads/master@{#845131}
parent 87a0f761
...@@ -6,20 +6,53 @@ ...@@ -6,20 +6,53 @@
#include "components/autofill/core/browser/data_model/autofill_profile.h" #include "components/autofill/core/browser/data_model/autofill_profile.h"
#include "components/autofill/core/browser/personal_data_manager.h" #include "components/autofill/core/browser/personal_data_manager.h"
#include "components/autofill/core/common/autofill_features.h"
namespace autofill { namespace autofill {
AddressProfileSaveManager::AddressProfileSaveManager( AddressProfileSaveManager::AddressProfileSaveManager(
AutofillClient* client,
PersonalDataManager* personal_data_manager) PersonalDataManager* personal_data_manager)
: personal_data_manager_(personal_data_manager) {} : client_(client), personal_data_manager_(personal_data_manager) {}
AddressProfileSaveManager::~AddressProfileSaveManager() = default; AddressProfileSaveManager::~AddressProfileSaveManager() = default;
std::string AddressProfileSaveManager::SaveProfile( bool AddressProfileSaveManager::SaveProfile(const AutofillProfile& profile) {
if (!personal_data_manager_)
return false;
if (base::FeatureList::IsEnabled(
features::kAutofillAddressProfileSavePrompt)) {
client_->ConfirmSaveAddressProfile(
profile,
base::BindOnce(&AddressProfileSaveManager::SaveProfilePromptCallback,
weak_ptr_factory_.GetWeakPtr()));
// TODO(crbug.com/1135178): The semantics of the return value are that the
// profile has been saved. At this point, we cannot tell if the profile is
// going to be saved or not since it depends on the user action. Revisit
// once the intended behavior of save prompts is final.
return true;
}
return SaveProfileInternal(profile);
}
bool AddressProfileSaveManager::SaveProfileInternal(
const AutofillProfile& profile) { const AutofillProfile& profile) {
return personal_data_manager_ return !personal_data_manager_->SaveImportedProfile(profile).empty();
? personal_data_manager_->SaveImportedProfile(profile) }
: std::string();
void AddressProfileSaveManager::SaveProfilePromptCallback(
AutofillClient::SaveAddressProfileOfferUserDecision user_decision,
AutofillProfile profile) {
switch (user_decision) {
case AutofillClient::SaveAddressProfileOfferUserDecision::kAccepted:
case AutofillClient::SaveAddressProfileOfferUserDecision::kEdited:
personal_data_manager_->SaveImportedProfile(profile);
break;
case AutofillClient::SaveAddressProfileOfferUserDecision::kDeclined:
case AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored:
break;
}
} }
} // namespace autofill } // namespace autofill
...@@ -7,6 +7,9 @@ ...@@ -7,6 +7,9 @@
#include <string> #include <string>
#include "base/memory/weak_ptr.h"
#include "components/autofill/core/browser/autofill_client.h"
namespace autofill { namespace autofill {
class AutofillProfile; class AutofillProfile;
...@@ -16,22 +19,32 @@ class PersonalDataManager; ...@@ -16,22 +19,32 @@ class PersonalDataManager;
// FormDataImporter. // FormDataImporter.
class AddressProfileSaveManager { class AddressProfileSaveManager {
public: public:
explicit AddressProfileSaveManager( // The parameters should outlive the AddressProfileSaveManager.
PersonalDataManager* personal_data_manager); AddressProfileSaveManager(AutofillClient* client,
PersonalDataManager* personal_data_manager);
AddressProfileSaveManager(const AddressProfileSaveManager&) = delete; AddressProfileSaveManager(const AddressProfileSaveManager&) = delete;
AddressProfileSaveManager& operator=(const AddressProfileSaveManager&) = AddressProfileSaveManager& operator=(const AddressProfileSaveManager&) =
delete; delete;
virtual ~AddressProfileSaveManager(); virtual ~AddressProfileSaveManager();
// Saves `imported_profile` using the `personal_data_manager_`. Returns the // Saves `profile` using the `personal_data_manager_`. Returns true
// guid of the new or updated profile, or the empty string if no profile was // if the profile was saved or updated, and false if no profile was
// saved. // saved.
std::string SaveProfile(const AutofillProfile& imported_profile); bool SaveProfile(const AutofillProfile& profile);
private: private:
void SaveProfilePromptCallback(
AutofillClient::SaveAddressProfileOfferUserDecision user_decision,
AutofillProfile profile);
bool SaveProfileInternal(const AutofillProfile& profile);
AutofillClient* const client_;
// The personal data manager, used to save and load personal data to/from the // The personal data manager, used to save and load personal data to/from the
// web database. // web database.
PersonalDataManager* const personal_data_manager_; PersonalDataManager* const personal_data_manager_;
base::WeakPtrFactory<AddressProfileSaveManager> weak_ptr_factory_{this};
}; };
} // namespace autofill } // namespace autofill
......
...@@ -4,8 +4,12 @@ ...@@ -4,8 +4,12 @@
#include "components/autofill/core/browser/address_profiles/address_profile_save_manager.h" #include "components/autofill/core/browser/address_profiles/address_profile_save_manager.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "components/autofill/core/browser/autofill_test_utils.h" #include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/browser/test_autofill_client.h"
#include "components/autofill/core/browser/test_personal_data_manager.h" #include "components/autofill/core/browser/test_personal_data_manager.h"
#include "components/autofill/core/common/autofill_features.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -22,13 +26,24 @@ class MockPersonalDataManager : public TestPersonalDataManager { ...@@ -22,13 +26,24 @@ class MockPersonalDataManager : public TestPersonalDataManager {
(override)); (override));
}; };
class AddressProfileSaveManagerTest : public testing::Test {
protected:
base::test::TaskEnvironment task_environment_;
TestAutofillClient autofill_client_;
MockPersonalDataManager mock_personal_data_manager_;
};
} // namespace } // namespace
TEST(AddressProfileSaveManager, SaveProfile) { TEST_F(AddressProfileSaveManagerTest, SaveProfileWhenNoSavePrompt) {
MockPersonalDataManager pdm; base::test::ScopedFeatureList scoped_feature_list;
AddressProfileSaveManager save_manager(&pdm); scoped_feature_list.InitAndDisableFeature(
features::kAutofillAddressProfileSavePrompt);
AddressProfileSaveManager save_manager(&autofill_client_,
&mock_personal_data_manager_);
AutofillProfile test_profile = test::GetFullProfile(); AutofillProfile test_profile = test::GetFullProfile();
EXPECT_CALL(pdm, SaveImportedProfile(test_profile)); EXPECT_CALL(mock_personal_data_manager_, SaveImportedProfile(test_profile));
save_manager.SaveProfile(test_profile); save_manager.SaveProfile(test_profile);
} }
} // namespace autofill } // namespace autofill
...@@ -137,6 +137,7 @@ class AutofillClient : public RiskDataLoader { ...@@ -137,6 +137,7 @@ class AutofillClient : public RiskDataLoader {
enum class SaveAddressProfileOfferUserDecision { enum class SaveAddressProfileOfferUserDecision {
kAccepted, kAccepted,
kEdited,
kDeclined, kDeclined,
kIgnored, kIgnored,
}; };
......
...@@ -240,7 +240,8 @@ FormDataImporter::FormDataImporter(AutofillClient* client, ...@@ -240,7 +240,8 @@ FormDataImporter::FormDataImporter(AutofillClient* client,
app_locale, app_locale,
personal_data_manager)), personal_data_manager)),
address_profile_save_manager_( address_profile_save_manager_(
std::make_unique<AddressProfileSaveManager>(personal_data_manager)), std::make_unique<AddressProfileSaveManager>(client,
personal_data_manager)),
#if !defined(OS_ANDROID) && !defined(OS_IOS) #if !defined(OS_ANDROID) && !defined(OS_IOS)
local_card_migration_manager_( local_card_migration_manager_(
std::make_unique<LocalCardMigrationManager>(client, std::make_unique<LocalCardMigrationManager>(client,
...@@ -715,10 +716,7 @@ bool FormDataImporter::ImportAddressProfileForSection( ...@@ -715,10 +716,7 @@ bool FormDataImporter::ImportAddressProfileForSection(
if (!candidate_profile.FinalizeAfterImport()) if (!candidate_profile.FinalizeAfterImport())
return false; return false;
std::string guid = return address_profile_save_manager_->SaveProfile(candidate_profile);
address_profile_save_manager_->SaveProfile(candidate_profile);
return !guid.empty();
} }
bool FormDataImporter::ImportCreditCard( bool FormDataImporter::ImportCreditCard(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment