Clear wallet data when sync is disabled.

Stop syncing (and therefore clear) wallet data when wallet autofill is disabled in the autofill settings dialog.

Stop re-masking cards on shutdown. Now masked and unmasked cards and addresses are deleted and not repopulated when the wallet integration is disabled.

Deleted an unused registrar class from the PrefMetricsService I noticed when researching this.

BUG=469847,462787,450843

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

Cr-Commit-Position: refs/heads/master@{#323354}

4 files changed, 165 insertions, 52 deletions

diff --git a/chrome/browser/prefs/pref_metrics_service.h b/chrome/browser/prefs/pref_metrics_service.h
index 0335817..f7f7ff4 100644
--- a/chrome/browser/prefs/pref_metrics_service.h
+++ b/chrome/browser/prefs/pref_metrics_service.h
@@ -11,7 +11,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
-#include "base/prefs/pref_change_registrar.h"
 #include "chrome/browser/prefs/synced_pref_change_registrar.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
@@ -84,7 +83,6 @@ class PrefMetricsService : public KeyedService {
   PrefService* prefs_;
   PrefService* local_state_;
 
-  PrefChangeRegistrar pref_registrar_;
   scoped_ptr<SyncedPrefChangeRegistrar> synced_pref_change_registrar_;
 
   base::WeakPtrFactory<PrefMetricsService> weak_factory_;
diff --git a/chrome/browser/sync/glue/autofill_wallet_data_type_controller.cc b/chrome/browser/sync/glue/autofill_wallet_data_type_controller.cc
index 00ec1a8..2f644ba 100644
--- a/chrome/browser/sync/glue/autofill_wallet_data_type_controller.cc
+++ b/chrome/browser/sync/glue/autofill_wallet_data_type_controller.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/prefs/pref_service.h"
+#include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/glue/chrome_report_unrecoverable_error.h"
 #include "chrome/browser/sync/profile_sync_components_factory.h"
@@ -13,6 +14,7 @@
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/webdata/web_data_service_factory.h"
 #include "chrome/common/pref_names.h"
+#include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "sync/api/sync_error.h"
@@ -30,12 +32,17 @@ AutofillWalletDataTypeController::AutofillWalletDataTypeController(
           base::Bind(&ChromeReportUnrecoverableError),
           profile_sync_factory),
       profile_(profile),
-      callback_registered_(false) {
+      callback_registered_(false),
+      currently_enabled_(IsEnabled()) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   pref_registrar_.Init(profile->GetPrefs());
   pref_registrar_.Add(
       autofill::prefs::kAutofillWalletSyncExperimentEnabled,
-      base::Bind(&AutofillWalletDataTypeController::OnSyncExperimentPrefChanged,
+      base::Bind(&AutofillWalletDataTypeController::OnSyncPrefChanged,
+                 base::Unretained(this)));
+  pref_registrar_.Add(
+      autofill::prefs::kAutofillWalletImportEnabled,
+      base::Bind(&AutofillWalletDataTypeController::OnSyncPrefChanged,
                  base::Unretained(this)));
 }
 
@@ -83,24 +90,55 @@ bool AutofillWalletDataTypeController::StartModels() {
 
 void AutofillWalletDataTypeController::StopModels() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // This function is called when shutting down (nothing is changing), when
+  // sync is disabled completely, or when wallet sync is disabled. In the
+  // cases where wallet sync or sync in general is disabled, clear wallet cards
+  // and addresses copied from the server. This is different than other sync
+  // cases since this type of data reflects what's on the server rather than
+  // syncing local data between clients, so this extra step is required.
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetForProfile(profile_);
+
+  // HasSyncSetupCompleted indicates if sync is currently enabled at all. The
+  // preferred data type indicates if wallet sync is enabled, and
+  // currently_enabled_ indicates if the other prefs are enabled. All of these
+  // have to be enabled to sync wallet cards.
+  if (!service->HasSyncSetupCompleted() ||
+      !service->GetPreferredDataTypes().Has(syncer::AUTOFILL_WALLET_DATA) ||
+      !currently_enabled_) {
+    autofill::PersonalDataManager* pdm =
+        autofill::PersonalDataManagerFactory::GetForProfile(profile_);
+    if (pdm)
+      pdm->ClearAllServerData();
+  }
 }
 
 bool AutofillWalletDataTypeController::ReadyForStart() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  return profile_->GetPrefs()->GetBoolean(
-      autofill::prefs::kAutofillWalletSyncExperimentEnabled);
+  return currently_enabled_;
 }
 
 void AutofillWalletDataTypeController::WebDatabaseLoaded() {
   OnModelLoaded();
 }
 
-void AutofillWalletDataTypeController::OnSyncExperimentPrefChanged() {
+void AutofillWalletDataTypeController::OnSyncPrefChanged() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!profile_->GetPrefs()->GetBoolean(
-          autofill::prefs::kAutofillWalletSyncExperimentEnabled)) {
-    // If autofill wallet sync is disabled, post a task to the backend thread to
-    // stop the datatype.
+
+  bool new_enabled = IsEnabled();
+  if (currently_enabled_ == new_enabled)
+    return;  // No change to sync state.
+  currently_enabled_ = new_enabled;
+
+  if (currently_enabled_) {
+    // The experiment was just enabled. Trigger a reconfiguration. This will do
+    // nothing if the type isn't preferred.
+    ProfileSyncService* sync_service =
+        ProfileSyncServiceFactory::GetForProfile(profile_);
+    sync_service->ReenableDatatype(type());
+  } else {
+    // Post a task to the backend thread to stop the datatype.
     if (state() != NOT_RUNNING && state() != STOPPING) {
       syncer::SyncError error(FROM_HERE,
                               syncer::SyncError::DATATYPE_POLICY_ERROR,
@@ -112,13 +150,18 @@ void AutofillWalletDataTypeController::OnSyncExperimentPrefChanged() {
                      this,
                      error));
     }
-  } else {
-    // The experiment was just enabled. Trigger a reconfiguration. This will do
-    // nothing if the type isn't preferred.
-    ProfileSyncService* sync_service =
-        ProfileSyncServiceFactory::GetForProfile(profile_);
-    sync_service->ReenableDatatype(type());
   }
 }
 
+bool AutofillWalletDataTypeController::IsEnabled() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Require both the sync experiment and the user-visible pref to be
+  // enabled to sync Wallet data.
+  PrefService* ps = profile_->GetPrefs();
+  return
+      ps->GetBoolean(autofill::prefs::kAutofillWalletSyncExperimentEnabled) &&
+      ps->GetBoolean(autofill::prefs::kAutofillWalletImportEnabled);
+}
+
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/glue/autofill_wallet_data_type_controller.h b/chrome/browser/sync/glue/autofill_wallet_data_type_controller.h
index a652f6d..18aaf36 100644
--- a/chrome/browser/sync/glue/autofill_wallet_data_type_controller.h
+++ b/chrome/browser/sync/glue/autofill_wallet_data_type_controller.h
@@ -38,12 +38,19 @@ class AutofillWalletDataTypeController
 
   void WebDatabaseLoaded();
 
-  // Callback for changes to kAutofillWalletSyncExperimentEnabled.
-  void OnSyncExperimentPrefChanged();
+  // Callback for changes to the autofill prefs.
+  void OnSyncPrefChanged();
+
+  // Returns true if the prefs are set such that wallet sync should be enabled.
+  bool IsEnabled();
 
   Profile* const profile_;
   bool callback_registered_;
 
+  // Stores whether we're currently syncing wallet data. This is the last
+  // value computed by IsEnabled.
+  bool currently_enabled_;
+
   // Registrar for listening to kAutofillWalletSyncExperimentEnabled status.
   PrefChangeRegistrar pref_registrar_;
 
diff --git a/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc b/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc
index 672e6d1..f61d05a 100644
--- a/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc
+++ b/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc
@@ -5,6 +5,7 @@
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/sync/profile_sync_service.h"
@@ -13,9 +14,11 @@
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
+#include "chrome/browser/webdata/web_data_service_factory.h"
 #include "components/autofill/core/browser/credit_card.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
+#include "components/autofill/core/common/autofill_pref_names.h"
 #include "content/public/browser/notification_service.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/test/fake_server/fake_server_entity.h"
@@ -33,6 +36,37 @@ const char kWalletSyncEnabledPreferencesContents[] =
 
 const char kWalletSyncExperimentTag[] = "wallet_sync";
 
+const char kDefaultCardID[] = "wallet entity ID";
+const int kDefaultCardExpMonth = 8;
+const int kDefaultCardExpYear = 2087;
+const char kDefaultCardLastFour[] = "1234";
+const char kDefaultCardName[] = "Patrick Valenzuela";
+const sync_pb::WalletMaskedCreditCard_WalletCardType kDefaultCardType =
+    sync_pb::WalletMaskedCreditCard::AMEX;
+
+void AddDefaultCard(fake_server::FakeServer* server) {
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::AutofillWalletSpecifics* wallet_specifics =
+      specifics.mutable_autofill_wallet();
+  wallet_specifics->set_type(
+      sync_pb::AutofillWalletSpecifics::MASKED_CREDIT_CARD);
+
+  sync_pb::WalletMaskedCreditCard* credit_card =
+      wallet_specifics->mutable_masked_card();
+  credit_card->set_id(kDefaultCardID);
+  credit_card->set_exp_month(kDefaultCardExpMonth);
+  credit_card->set_exp_year(kDefaultCardExpYear);
+  credit_card->set_last_four(kDefaultCardLastFour);
+  credit_card->set_name_on_card(kDefaultCardName);
+  credit_card->set_status(sync_pb::WalletMaskedCreditCard::VALID);
+  credit_card->set_type(kDefaultCardType);
+
+  server->InjectEntity(fake_server::UniqueClientEntity::CreateForInjection(
+      syncer::AUTOFILL_WALLET_DATA,
+      kDefaultCardID,
+      specifics));
+}
+
 }  // namespace
 
 class SingleClientWalletSyncTest : public SyncTest {
@@ -167,49 +201,80 @@ IN_PROC_BROWSER_TEST_F(SingleClientWalletSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientWalletSyncTest, Download) {
   SetPreexistingPreferencesFileContents(kWalletSyncEnabledPreferencesContents);
+  AddDefaultCard(GetFakeServer());
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed";
 
-  std::string id = "wallet entity ID";
-  int expiration_month = 8;
-  int expiration_year = 2087;
-  std::string last_four_digits = "1234";
-  std::string name_on_card = "Patrick Valenzuela";
+  autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
+  ASSERT_TRUE(pdm != nullptr);
+  std::vector<autofill::CreditCard*> cards = pdm->GetCreditCards();
+  ASSERT_EQ(1uL, cards.size());
 
-  sync_pb::EntitySpecifics specifics;
-  sync_pb::AutofillWalletSpecifics* wallet_specifics =
-      specifics.mutable_autofill_wallet();
-  wallet_specifics->set_type(
-      sync_pb::AutofillWalletSpecifics::MASKED_CREDIT_CARD);
+  autofill::CreditCard* card = cards[0];
+  ASSERT_EQ(autofill::CreditCard::MASKED_SERVER_CARD, card->record_type());
+  ASSERT_EQ(kDefaultCardID, card->server_id());
+  ASSERT_EQ(base::UTF8ToUTF16(kDefaultCardLastFour), card->LastFourDigits());
+  ASSERT_EQ(autofill::kAmericanExpressCard, card->type());
+  ASSERT_EQ(kDefaultCardExpMonth, card->expiration_month());
+  ASSERT_EQ(kDefaultCardExpYear, card->expiration_year());
+  ASSERT_EQ(base::UTF8ToUTF16(kDefaultCardName),
+            card->GetRawInfo(autofill::ServerFieldType::CREDIT_CARD_NAME));
+}
 
-  sync_pb::WalletMaskedCreditCard* credit_card =
-      wallet_specifics->mutable_masked_card();
-  credit_card->set_id(id);
-  credit_card->set_exp_month(expiration_month);
-  credit_card->set_exp_year(expiration_year);
-  credit_card->set_last_four(last_four_digits);
-  credit_card->set_name_on_card(name_on_card);
-  credit_card->set_status(sync_pb::WalletMaskedCreditCard::VALID);
-  credit_card->set_type(sync_pb::WalletMaskedCreditCard::AMEX);
+// Wallet data should get cleared from the database when sync is disabled.
+IN_PROC_BROWSER_TEST_F(SingleClientWalletSyncTest, ClearOnDisableSync) {
+  SetPreexistingPreferencesFileContents(kWalletSyncEnabledPreferencesContents);
+  AddDefaultCard(GetFakeServer());
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed";
 
-  GetFakeServer()->InjectEntity(
-      fake_server::UniqueClientEntity::CreateForInjection(
-          syncer::AUTOFILL_WALLET_DATA,
-          id,
-          specifics));
+  // Make sure the card is in the DB.
+  autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
+  ASSERT_TRUE(pdm != nullptr);
+  std::vector<autofill::CreditCard*> cards = pdm->GetCreditCards();
+  ASSERT_EQ(1uL, cards.size());
 
+  // Turn off sync, the card should be gone.
+  ASSERT_TRUE(GetClient(0)->DisableSyncForAllDatatypes());
+  cards = pdm->GetCreditCards();
+  ASSERT_EQ(0uL, cards.size());
+}
+
+// Wallet data should get cleared from the database when the wallet sync type
+// flag is disabled.
+IN_PROC_BROWSER_TEST_F(SingleClientWalletSyncTest, ClearOnDisableWalletSync) {
+  SetPreexistingPreferencesFileContents(kWalletSyncEnabledPreferencesContents);
+  AddDefaultCard(GetFakeServer());
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed";
 
+  // Make sure the card is in the DB.
   autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
   ASSERT_TRUE(pdm != nullptr);
   std::vector<autofill::CreditCard*> cards = pdm->GetCreditCards();
   ASSERT_EQ(1uL, cards.size());
 
-  autofill::CreditCard* card = cards[0];
-  ASSERT_EQ(autofill::CreditCard::MASKED_SERVER_CARD, card->record_type());
-  ASSERT_EQ(id, card->server_id());
-  ASSERT_EQ(base::UTF8ToUTF16(last_four_digits), card->LastFourDigits());
-  ASSERT_EQ(autofill::kAmericanExpressCard, card->type());
-  ASSERT_EQ(expiration_month, card->expiration_month());
-  ASSERT_EQ(expiration_year, card->expiration_year());
-  ASSERT_EQ(base::UTF8ToUTF16(name_on_card),
-            card->GetRawInfo(autofill::ServerFieldType::CREDIT_CARD_NAME));
+  // Turn off autofill sync, the card should be gone.
+  ASSERT_TRUE(GetClient(0)->DisableSyncForDatatype(syncer::AUTOFILL));
+  cards = pdm->GetCreditCards();
+  ASSERT_EQ(0uL, cards.size());
+}
+
+// Wallet data should get cleared from the database when the wallet autofill
+// integration flag is disabled.
+IN_PROC_BROWSER_TEST_F(SingleClientWalletSyncTest,
+                       ClearOnDisableWalletAutofill) {
+  SetPreexistingPreferencesFileContents(kWalletSyncEnabledPreferencesContents);
+  AddDefaultCard(GetFakeServer());
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed";
+
+  // Make sure the card is in the DB.
+  autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
+  ASSERT_TRUE(pdm != nullptr);
+  std::vector<autofill::CreditCard*> cards = pdm->GetCreditCards();
+  ASSERT_EQ(1uL, cards.size());
+
+  // Turn off the wallet autofill pref, the card should be gone as a side
+  // effect of the wallet data type controller noticing.
+  GetProfile(0)->GetPrefs()->SetBoolean(
+      autofill::prefs::kAutofillWalletImportEnabled, false);
+  cards = pdm->GetCreditCards();
+  ASSERT_EQ(0uL, cards.size());
 }