Commit 6fb111eb authored by siyua's avatar siyua Committed by Commit Bot

[AF][Paradise] Enable migration flow for non-sync users

Also removed the EnableAutofillCreditCardLocalCardMigrationExperiment()
in unittests since as we add more experiment flags, this function does
not help.

Set a helper function in TestPersonalDataManager to change the sync
state, which can help us in case of such need in the future.

Bug: 897998
Change-Id: Id212077092d80738b8495f4b50bd7034e9f1ead6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1501754
Commit-Queue: Siyu An <siyua@chromium.org>
Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638336}
parent a909223e
......@@ -208,18 +208,24 @@ bool LocalCardMigrationManager::IsCreditCardMigrationEnabled() {
observer_for_testing_ ||
::autofill::IsCreditCardUploadEnabled(
client_->GetPrefs(), client_->GetSyncService(),
client_->GetIdentityManager()->GetPrimaryAccountInfo().email);
personal_data_manager_->GetAccountInfoForPaymentsServer().email);
bool has_google_payments_account =
(payments::GetBillingCustomerId(personal_data_manager_,
payments_client_->GetPrefService()) != 0);
bool sync_feature_enabled =
(personal_data_manager_->GetSyncSigninState() ==
AutofillSyncSigninState::kSignedInAndSyncFeature);
AutofillSyncSigninState sync_state =
personal_data_manager_->GetSyncSigninState();
return migration_experiment_enabled && credit_card_upload_enabled &&
has_google_payments_account && sync_feature_enabled;
has_google_payments_account &&
// User signed-in and turned sync on.
(sync_state == AutofillSyncSigninState::kSignedInAndSyncFeature ||
// User signed-in but not turned on sync.
(sync_state == AutofillSyncSigninState::
kSignedInAndWalletSyncTransportEnabled &&
base::FeatureList::IsEnabled(
features::kAutofillEnableLocalCardMigrationForNonSyncUser)));
}
void LocalCardMigrationManager::OnDidGetUploadDetails(
......
......@@ -126,7 +126,7 @@ class PersonalDataManager : public KeyedService,
void OnAccountsCookieDeletedByUserAction() override;
// Returns the current sync status.
AutofillSyncSigninState GetSyncSigninState() const;
virtual AutofillSyncSigninState GetSyncSigninState() const;
// Adds a listener to be notified of PersonalDataManager events.
virtual void AddObserver(PersonalDataManagerObserver* observer);
......
......@@ -8,7 +8,6 @@
#include "components/autofill/core/browser/payments/test_payments_client.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_prefs.h"
#include "services/identity/public/cpp/identity_manager.h"
namespace autofill {
......@@ -16,11 +15,12 @@ TestLocalCardMigrationManager::TestLocalCardMigrationManager(
AutofillDriver* driver,
AutofillClient* client,
payments::TestPaymentsClient* payments_client,
PersonalDataManager* personal_data_manager)
TestPersonalDataManager* personal_data_manager)
: LocalCardMigrationManager(client,
payments_client,
"en-US",
personal_data_manager) {}
personal_data_manager),
personal_data_manager_(personal_data_manager) {}
TestLocalCardMigrationManager::~TestLocalCardMigrationManager() {}
......@@ -28,10 +28,19 @@ bool TestLocalCardMigrationManager::IsCreditCardMigrationEnabled() {
bool migration_experiment_enabled =
features::GetLocalCardMigrationExperimentalFlag() !=
features::LocalCardMigrationExperimentalFlag::kMigrationDisabled;
bool has_google_payments_account =
(static_cast<int64_t>(payments_client_->GetPrefService()->GetDouble(
prefs::kAutofillBillingCustomerNumber)) != 0);
return migration_experiment_enabled && has_google_payments_account;
bool sync_feature_enabled =
(personal_data_manager_->GetSyncSigninState() ==
AutofillSyncSigninState::kSignedInAndSyncFeature);
return migration_experiment_enabled && has_google_payments_account &&
(sync_feature_enabled ||
base::FeatureList::IsEnabled(
features::kAutofillEnableLocalCardMigrationForNonSyncUser));
}
bool TestLocalCardMigrationManager::LocalCardMigrationWasTriggered() {
......@@ -58,6 +67,11 @@ void TestLocalCardMigrationManager::OnUserAcceptedMainMigrationDialog(
LocalCardMigrationManager::OnUserAcceptedMainMigrationDialog(selected_cards);
}
void TestLocalCardMigrationManager::ResetSyncState(
AutofillSyncSigninState sync_state) {
personal_data_manager_->SetSyncAndSignInState(sync_state);
}
void TestLocalCardMigrationManager::OnDidGetUploadDetails(
bool is_from_settings_page,
AutofillClient::PaymentsRpcResult result,
......
......@@ -8,6 +8,8 @@
#include <string>
#include "components/autofill/core/browser/local_card_migration_manager.h"
#include "components/autofill/core/browser/sync_utils.h"
#include "components/autofill/core/browser/test_personal_data_manager.h"
namespace autofill {
......@@ -17,14 +19,13 @@ class TestPaymentsClient;
class AutofillClient;
class AutofillDriver;
class PersonalDataManager;
class TestLocalCardMigrationManager : public LocalCardMigrationManager {
public:
TestLocalCardMigrationManager(AutofillDriver* driver,
AutofillClient* client,
payments::TestPaymentsClient* payments_client,
PersonalDataManager* personal_data_manager);
TestPersonalDataManager* personal_data_manager);
~TestLocalCardMigrationManager() override;
// Override the base function. Checks the existnece of billing customer number
......@@ -50,6 +51,10 @@ class TestLocalCardMigrationManager : public LocalCardMigrationManager {
void OnUserAcceptedMainMigrationDialog(
const std::vector<std::string>& selected_cards) override;
// Mock the Chrome Sync state in the LocalCardMigrationManager. If not set,
// default to AutofillSyncSigninState::kSignedInAndSyncFeature.
void ResetSyncState(AutofillSyncSigninState sync_state);
private:
void OnDidGetUploadDetails(
bool is_from_settings_page,
......@@ -63,6 +68,8 @@ class TestLocalCardMigrationManager : public LocalCardMigrationManager {
bool main_prompt_was_shown_ = false;
TestPersonalDataManager* personal_data_manager_;
DISALLOW_COPY_AND_ASSIGN(TestLocalCardMigrationManager);
};
......
......@@ -19,6 +19,10 @@ void TestPersonalDataManager::OnSyncServiceInitialized(
sync_service_initialized_ = true;
}
AutofillSyncSigninState TestPersonalDataManager::GetSyncSigninState() const {
return sync_and_signin_state_;
}
void TestPersonalDataManager::RecordUseOf(const AutofillDataModel& data_model) {
CreditCard* credit_card = GetCreditCardWithGUID(data_model.guid().c_str());
if (credit_card)
......
......@@ -28,6 +28,7 @@ class TestPersonalDataManager : public PersonalDataManager {
// for various tests, whether to skip calls to uncreated databases/services,
// or to make things easier in general to toggle.
void OnSyncServiceInitialized(syncer::SyncService* sync_service) override;
AutofillSyncSigninState GetSyncSigninState() const override;
void RecordUseOf(const AutofillDataModel& data_model) override;
std::string SaveImportedProfile(
const AutofillProfile& imported_profile) override;
......@@ -115,6 +116,10 @@ class TestPersonalDataManager : public PersonalDataManager {
void SetSyncFeatureEnabled(bool enabled) { sync_feature_enabled_ = enabled; }
void SetSyncAndSignInState(AutofillSyncSigninState sync_and_signin_state) {
sync_and_signin_state_ = sync_and_signin_state;
}
void SetAccountInfoForPayments(const CoreAccountInfo& account_info) {
account_info_ = account_info;
}
......@@ -129,6 +134,8 @@ class TestPersonalDataManager : public PersonalDataManager {
base::Optional<bool> autofill_credit_card_enabled_;
base::Optional<bool> autofill_wallet_import_enabled_;
bool sync_feature_enabled_ = false;
AutofillSyncSigninState sync_and_signin_state_ =
AutofillSyncSigninState::kSignedInAndSyncFeature;
bool sync_service_initialized_ = false;
CoreAccountInfo account_info_;
......
......@@ -96,6 +96,12 @@ const base::Feature kAutofillEnableCompanyName{
const base::Feature kAutofillEnableIFrameSupportOniOS{
"AutofillEnableIFrameSupportOniOS", base::FEATURE_ENABLED_BY_DEFAULT};
// When enabled, enable local card migration flow for user who has signed in but
// has not turned on sync.
const base::Feature kAutofillEnableLocalCardMigrationForNonSyncUser{
"AutofillEnableLocalCardMigrationForNonSyncUser",
base::FEATURE_DISABLED_BY_DEFAULT};
// When enabled, no local copy of server card will be saved when upload
// succeeds.
const base::Feature kAutofillNoLocalSaveOnUploadSuccess{
......
......@@ -39,6 +39,7 @@ extern const base::Feature kAutofillEnableAccountWalletStorage;
extern const base::Feature kAutofillEnableAccountWalletStorageUpload;
extern const base::Feature kAutofillEnableCompanyName;
extern const base::Feature kAutofillEnableIFrameSupportOniOS;
extern const base::Feature kAutofillEnableLocalCardMigrationForNonSyncUser;
extern const base::Feature kAutofillEnforceMinRequiredFieldsForHeuristics;
extern const base::Feature kAutofillEnforceMinRequiredFieldsForQuery;
extern const base::Feature kAutofillEnforceMinRequiredFieldsForUpload;
......
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