Commit 93d1d248 authored by bondd's avatar bondd Committed by Commit bot

Autofill: Fix WalletIntegrationAvailable() regression.

Accidentally inverted a check during refactoring in
https://crrev.com/1137513002

Add back the '!' character that I deleted, and add unit tests to
guard against future regressions.

BUG=486675

Review URL: https://codereview.chromium.org/1138373002

Cr-Commit-Position: refs/heads/master@{#329933}
parent a437dab3
...@@ -228,6 +228,7 @@ source_set("unit_tests") { ...@@ -228,6 +228,7 @@ source_set("unit_tests") {
"form_field_unittest.cc", "form_field_unittest.cc",
"form_structure_unittest.cc", "form_structure_unittest.cc",
"name_field_unittest.cc", "name_field_unittest.cc",
"options_util_unittest.cc",
"password_generator_unittest.cc", "password_generator_unittest.cc",
"personal_data_manager_unittest.cc", "personal_data_manager_unittest.cc",
"phone_field_unittest.cc", "phone_field_unittest.cc",
...@@ -251,6 +252,7 @@ source_set("unit_tests") { ...@@ -251,6 +252,7 @@ source_set("unit_tests") {
"//components/resources", "//components/resources",
"//components/signin/core/browser:test_support", "//components/signin/core/browser:test_support",
"//components/strings", "//components/strings",
"//components/sync_driver",
"//components/webdata/common", "//components/webdata/common",
"//net:test_support", "//net:test_support",
"//sql", "//sql",
......
...@@ -26,7 +26,7 @@ bool WalletIntegrationAvailable( ...@@ -26,7 +26,7 @@ bool WalletIntegrationAvailable(
// If the user is signed in and the feature is enabled, but no data is being // If the user is signed in and the feature is enabled, but no data is being
// synced, hide the option. The user doesn't have a Wallet account. If the // synced, hide the option. The user doesn't have a Wallet account. If the
// feature is disabled, we can't know, so show the checkbox. // feature is disabled, we can't know, so show the checkbox.
if (pref_service.GetBoolean(prefs::kAutofillWalletImportEnabled)) if (!pref_service.GetBoolean(prefs::kAutofillWalletImportEnabled))
return true; return true;
// If wallet is preferred but we haven't gotten the sync data yet, we don't // If wallet is preferred but we haven't gotten the sync data yet, we don't
......
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/prefs/pref_registry_simple.h"
#include "base/prefs/testing_pref_service.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/browser/options_util.h"
#include "components/autofill/core/browser/test_personal_data_manager.h"
#include "components/autofill/core/common/autofill_pref_names.h"
#include "components/sync_driver/data_type_manager.h"
#include "components/sync_driver/sync_service.h"
#include "google_apis/gaia/google_service_auth_error.h"
#include "testing/gmock/include/gmock/gmock.h"
using testing::Return;
namespace autofill {
namespace {
class SyncServiceMock : public sync_driver::SyncService {
public:
MOCK_CONST_METHOD0(HasSyncSetupCompleted, bool());
MOCK_CONST_METHOD0(SyncActive, bool());
MOCK_CONST_METHOD0(GetActiveDataTypes, syncer::ModelTypeSet());
MOCK_METHOD1(AddObserver, void(sync_driver::SyncServiceObserver*));
MOCK_METHOD1(RemoveObserver, void(sync_driver::SyncServiceObserver*));
MOCK_CONST_METHOD1(HasObserver,
bool(const sync_driver::SyncServiceObserver*));
MOCK_METHOD0(IsSyncEnabledAndLoggedIn, bool());
MOCK_METHOD0(DisableForUser, void());
MOCK_METHOD0(StopAndSuppress, void());
MOCK_METHOD0(UnsuppressAndStart, void());
MOCK_CONST_METHOD0(GetPreferredDataTypes, syncer::ModelTypeSet());
MOCK_METHOD2(OnUserChoseDatatypes,
void(bool sync_everything, syncer::ModelTypeSet chosen_types));
MOCK_METHOD0(SetSyncSetupCompleted, void());
MOCK_CONST_METHOD0(FirstSetupInProgress, bool());
MOCK_METHOD1(SetSetupInProgress, void(bool));
MOCK_CONST_METHOD0(setup_in_progress, bool());
MOCK_CONST_METHOD0(ConfigurationDone, bool());
MOCK_CONST_METHOD0(GetAuthError, const GoogleServiceAuthError&());
MOCK_CONST_METHOD0(HasUnrecoverableError, bool());
MOCK_CONST_METHOD0(backend_initialized, bool());
MOCK_CONST_METHOD0(IsPassphraseRequiredForDecryption, bool());
MOCK_CONST_METHOD0(GetExplicitPassphraseTime, base::Time());
MOCK_CONST_METHOD0(IsUsingSecondaryPassphrase, bool());
MOCK_METHOD0(EnableEncryptEverything, void());
MOCK_METHOD2(SetEncryptionPassphrase,
void(const std::string& passphrase, PassphraseType type));
MOCK_METHOD1(SetDecryptionPassphrase, bool(const std::string& passphrase));
// DataTypeEncryptionHandler mocks.
MOCK_CONST_METHOD0(IsPassphraseRequired, bool());
MOCK_CONST_METHOD0(GetEncryptedDataTypes, syncer::ModelTypeSet());
};
scoped_ptr<SyncServiceMock> CreateSyncService(bool has_autofill_profile,
bool has_autofill_wallet_data,
bool is_enabled_and_logged_in) {
scoped_ptr<SyncServiceMock> sync(new SyncServiceMock());
syncer::ModelTypeSet type_set;
if (has_autofill_profile)
type_set.Put(syncer::AUTOFILL_PROFILE);
if (has_autofill_wallet_data)
type_set.Put(syncer::AUTOFILL_WALLET_DATA);
ON_CALL(*sync, GetActiveDataTypes()).WillByDefault(Return(type_set));
ON_CALL(*sync, GetPreferredDataTypes()).WillByDefault(Return(type_set));
ON_CALL(*sync, IsSyncEnabledAndLoggedIn())
.WillByDefault(Return(is_enabled_and_logged_in));
return sync;
}
scoped_ptr<TestingPrefServiceSimple> CreatePrefService(
bool autofill_enabled,
bool autofill_wallet_import_enabled,
bool autofill_wallet_sync_experiment_enabled) {
scoped_ptr<TestingPrefServiceSimple> prefs(new TestingPrefServiceSimple());
prefs->registry()->RegisterBooleanPref(prefs::kAutofillEnabled,
autofill_enabled);
prefs->registry()->RegisterBooleanPref(prefs::kAutofillWalletImportEnabled,
autofill_wallet_import_enabled);
prefs->registry()->RegisterBooleanPref(
prefs::kAutofillWalletSyncExperimentEnabled,
autofill_wallet_sync_experiment_enabled);
return prefs;
}
scoped_ptr<TestPersonalDataManager> CreatePersonalDataManager(
PrefService* prefs,
bool has_server_data) {
scoped_ptr<TestPersonalDataManager> pdm(new TestPersonalDataManager());
pdm->SetTestingPrefService(prefs);
if (has_server_data) {
// This will cause pdm->HasServerData() to return true.
pdm->AddTestingServerCreditCard(test::GetVerifiedCreditCard());
}
return pdm;
}
} // namespace
// Verify that true is returned when all inputs are complete.
TEST(WalletIntegrationAvailableTest, AllInputsComplete) {
scoped_ptr<SyncServiceMock> sync = CreateSyncService(true, true, true);
scoped_ptr<TestingPrefServiceSimple> prefs =
CreatePrefService(true, true, true);
scoped_ptr<TestPersonalDataManager> pdm =
CreatePersonalDataManager(prefs.get(), true);
EXPECT_TRUE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
}
// Verify that false is returned when SyncService is missing or incomplete.
TEST(WalletIntegrationAvailableTest, MissingOrIncompleteSyncService) {
// Setup |prefs| and |pdm| to do their part to make
// WalletIntegrationAvailable() return true.
scoped_ptr<TestingPrefServiceSimple> prefs =
CreatePrefService(true, true, true);
scoped_ptr<TestPersonalDataManager> pdm =
CreatePersonalDataManager(prefs.get(), true);
// Incomplete SyncService data should return false.
EXPECT_FALSE(WalletIntegrationAvailable(NULL, *prefs, *pdm));
scoped_ptr<SyncServiceMock> sync = CreateSyncService(false, false, false);
EXPECT_FALSE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
sync = CreateSyncService(false, false, true);
EXPECT_FALSE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
sync = CreateSyncService(true, false, false);
EXPECT_FALSE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
// Complete SyncService data should return true.
sync = CreateSyncService(true, true, true);
EXPECT_TRUE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
}
// Verify that false is returned when
// !prefs::kAutofillWalletSyncExperimentEnabled.
TEST(WalletIntegrationAvailableTest, ExperimentalWalletIntegrationDisabled) {
scoped_ptr<SyncServiceMock> sync = CreateSyncService(true, true, true);
// Set kAutofillWalletSyncExperimentEnabled to false.
scoped_ptr<TestingPrefServiceSimple> prefs =
CreatePrefService(true, true, false);
scoped_ptr<TestPersonalDataManager> pdm =
CreatePersonalDataManager(prefs.get(), true);
EXPECT_FALSE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
}
// Verify that false is returned if server data is missing.
TEST(WalletIntegrationAvailableTest, NoServerData) {
scoped_ptr<SyncServiceMock> sync = CreateSyncService(true, true, true);
scoped_ptr<TestingPrefServiceSimple> prefs =
CreatePrefService(true, true, true);
// Set server data as missing.
scoped_ptr<TestPersonalDataManager> pdm =
CreatePersonalDataManager(prefs.get(), false);
EXPECT_FALSE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
}
// Verify that true is returned when !prefs::kAutofillWalletImportEnabled,
// even if server data is missing.
TEST(WalletIntegrationAvailableTest, WalletImportDisabled) {
scoped_ptr<SyncServiceMock> sync = CreateSyncService(true, true, true);
// Set kAutofillWalletImportEnabled to false.
scoped_ptr<TestingPrefServiceSimple> prefs =
CreatePrefService(true, false, true);
// Set server data as missing.
scoped_ptr<TestPersonalDataManager> pdm =
CreatePersonalDataManager(prefs.get(), false);
EXPECT_TRUE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
}
// Verify that true is returned when data hasn't been synced yet, even if
// server data is missing.
TEST(WalletIntegrationAvailableTest, WalletDataNotSyncedYet) {
// Set wallet data as not synced yet.
scoped_ptr<SyncServiceMock> sync = CreateSyncService(true, false, true);
scoped_ptr<TestingPrefServiceSimple> prefs =
CreatePrefService(true, true, true);
// Set server data as missing.
scoped_ptr<TestPersonalDataManager> pdm =
CreatePersonalDataManager(prefs.get(), false);
EXPECT_TRUE(WalletIntegrationAvailable(sync.get(), *prefs, *pdm));
}
} // namespace autofill
...@@ -13,6 +13,10 @@ TestPersonalDataManager::TestPersonalDataManager() ...@@ -13,6 +13,10 @@ TestPersonalDataManager::TestPersonalDataManager()
TestPersonalDataManager::~TestPersonalDataManager() {} TestPersonalDataManager::~TestPersonalDataManager() {}
void TestPersonalDataManager::SetTestingPrefService(PrefService* pref_service) {
SetPrefService(pref_service);
}
void TestPersonalDataManager::AddTestingProfile(AutofillProfile* profile) { void TestPersonalDataManager::AddTestingProfile(AutofillProfile* profile) {
profiles_.push_back(profile); profiles_.push_back(profile);
NotifyPersonalDataChanged(); NotifyPersonalDataChanged();
...@@ -23,6 +27,11 @@ void TestPersonalDataManager::AddTestingCreditCard(CreditCard* credit_card) { ...@@ -23,6 +27,11 @@ void TestPersonalDataManager::AddTestingCreditCard(CreditCard* credit_card) {
NotifyPersonalDataChanged(); NotifyPersonalDataChanged();
} }
void TestPersonalDataManager::AddTestingServerCreditCard(
const CreditCard& credit_card) {
server_credit_cards_.push_back(new CreditCard(credit_card));
}
const std::vector<AutofillProfile*>& TestPersonalDataManager::GetProfiles() const std::vector<AutofillProfile*>& TestPersonalDataManager::GetProfiles()
const { const {
return profiles_; return profiles_;
......
...@@ -19,6 +19,10 @@ class TestPersonalDataManager : public PersonalDataManager { ...@@ -19,6 +19,10 @@ class TestPersonalDataManager : public PersonalDataManager {
TestPersonalDataManager(); TestPersonalDataManager();
~TestPersonalDataManager() override; ~TestPersonalDataManager() override;
// Sets which PrefService to use and observe. |pref_service| is not owned by
// this class and must outlive |this|.
void SetTestingPrefService(PrefService* pref_service);
// Adds |profile| to |profiles_|. This does not take ownership of |profile|. // Adds |profile| to |profiles_|. This does not take ownership of |profile|.
void AddTestingProfile(AutofillProfile* profile); void AddTestingProfile(AutofillProfile* profile);
...@@ -26,6 +30,9 @@ class TestPersonalDataManager : public PersonalDataManager { ...@@ -26,6 +30,9 @@ class TestPersonalDataManager : public PersonalDataManager {
// |credit_card|. // |credit_card|.
void AddTestingCreditCard(CreditCard* credit_card); void AddTestingCreditCard(CreditCard* credit_card);
// Adds |credit_card| to |server_credit_cards_| by copying.
void AddTestingServerCreditCard(const CreditCard& credit_card);
const std::vector<AutofillProfile*>& GetProfiles() const override; const std::vector<AutofillProfile*>& GetProfiles() const override;
const std::vector<AutofillProfile*>& web_profiles() const override; const std::vector<AutofillProfile*>& web_profiles() const override;
const std::vector<CreditCard*>& GetCreditCards() const override; const std::vector<CreditCard*>& GetCreditCards() const override;
......
...@@ -49,6 +49,7 @@ ...@@ -49,6 +49,7 @@
'autofill/core/browser/form_field_unittest.cc', 'autofill/core/browser/form_field_unittest.cc',
'autofill/core/browser/form_structure_unittest.cc', 'autofill/core/browser/form_structure_unittest.cc',
'autofill/core/browser/name_field_unittest.cc', 'autofill/core/browser/name_field_unittest.cc',
'autofill/core/browser/options_util_unittest.cc',
'autofill/core/browser/password_generator_unittest.cc', 'autofill/core/browser/password_generator_unittest.cc',
'autofill/core/browser/personal_data_manager_unittest.cc', 'autofill/core/browser/personal_data_manager_unittest.cc',
'autofill/core/browser/phone_field_unittest.cc', 'autofill/core/browser/phone_field_unittest.cc',
......
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