[Autofill] Run autofill-profile de-dupe after sync starts if sync is enabled.

components/autofill/core/browser/personal_data_manager.cc

Index: components/autofill/core/browser/personal_data_manager.cc
diff --git a/components/autofill/core/browser/personal_data_manager.cc b/components/autofill/core/browser/personal_data_manager.cc
index d6c311f22c2a7d11bf4cf4b410de6cab85c11d23..ca8413a432ef4f842d226054c8e7c8ef5980355e 100644
--- a/components/autofill/core/browser/personal_data_manager.cc
+++ b/components/autofill/core/browser/personal_data_manager.cc
@@ -39,6 +39,7 @@
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/signin/core/common/signin_pref_names.h"
+#include "components/sync_driver/sync_service.h"
 #include "components/variations/variations_associated_data.h"
 #include "components/version_info/version_info.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
@@ -287,6 +288,7 @@ void PersonalDataManager::Init(scoped_refptr<AutofillWebDataService> database,
   LoadCreditCards();
 
   database_->AddObserver(this);
+  is_autofill_profile_dedupe_pending_ = IsAutofillProfileCleanupEnabled();
 }
 
 PersonalDataManager::~PersonalDataManager() {
@@ -299,6 +301,27 @@ PersonalDataManager::~PersonalDataManager() {
     database_->RemoveObserver(this);
 }
 
+void PersonalDataManager::OnSyncServiceInitialized(
+    sync_driver::SyncService* sync_service) {
+  // We want to know when, if at all, we need to run autofill profile de-
+  // duplication: now or after waiting until sync has started.
+  if (!is_autofill_profile_dedupe_pending_) {
+    // De-duplication isn't enabled.
+    return;
+  }
+
+  // If the sync service is configured to start and to sync autofill profiles,
+  // then we can just let the notification that sync has started trigger the
+  // de-duplication.
+  if (sync_service && sync_service->CanSyncStart() &&
+      sync_service->GetPreferredDataTypes().Has(syncer::AUTOFILL_PROFILE)) {
+    return;
+  }
+
+  // Otherwise, run the de-duplication now.
+  ApplyDedupingRoutine();
+}
+
 void PersonalDataManager::OnWebDataServiceRequestDone(
     WebDataServiceBase::Handle h,
     const WDTypedResult* result) {
@@ -326,10 +349,7 @@ void PersonalDataManager::OnWebDataServiceRequestDone(
         ReceiveLoadedDbValues(h, result, &pending_profiles_query_,
                               &web_profiles_);
         LogProfileCount();  // This only logs local profiles.
-        // Since these two routines both re-launch the database queries, don't
-        // run them on the same query response.
-        if (!ApplyDedupingRoutine())
-          ApplyProfileUseDatesFix();
+        ApplyProfileUseDatesFix();
       } else {
         ReceiveLoadedDbValues(h, result, &pending_server_profiles_query_,
                               &server_profiles_);
@@ -383,6 +403,13 @@ void PersonalDataManager::AutofillMultipleChanged() {
   Refresh();
 }
 
+void PersonalDataManager::SyncStarted(syncer::ModelType model_type) {
+  if (model_type == syncer::AUTOFILL_PROFILE &&
+      is_autofill_profile_dedupe_pending_) {
+    ApplyDedupingRoutine();
+  }
+}
+
 void PersonalDataManager::AddObserver(PersonalDataManagerObserver* observer) {
   observers_.AddObserver(observer);
 }
@@ -1606,16 +1633,22 @@ void PersonalDataManager::ApplyProfileUseDatesFix() {
 }
 
 bool PersonalDataManager::ApplyDedupingRoutine() {
-  if (!IsAutofillProfileCleanupEnabled())
+  if (!is_autofill_profile_dedupe_pending_)

[Mathieu, 2016/07/18 14:53:19]

are these two lines necessary? Seems like callers have already checked before we
got here.

[Roger McFarlane (Chromium), 2016/07/18 15:29:18]

there's a slight mismatch between how the tests expect this to be called and how
it is actually called.

The tests treat the function as if it was called "MaybeApplyDedupingRoutine" but
the real call sites subsume the "maybe" and call it only when it will actually
be applied.

In a follow-on CL, I'll fix the tests and turn this into a DCHECK.

     return false;
 
-  int current_major_version = atoi(version_info::GetVersionNumber().c_str());
+  DCHECK(IsAutofillProfileCleanupEnabled());
+  is_autofill_profile_dedupe_pending_ = false;
 
   // Check if the deduping routine has already been run on this major version.
+  int current_major_version = atoi(version_info::GetVersionNumber().c_str());
   if (pref_service_->GetInteger(prefs::kAutofillLastVersionDeduped) >=
-      current_major_version)
+      current_major_version) {
+    DVLOG(1)
+        << "Autofill profile de-duplication already performed for this version";
     return false;
+  }
 
+  DVLOG(1) << "Starting autofill profile de-duplication.";
   std::vector<AutofillProfile*> existing_profiles = web_profiles_.get();
   std::unordered_set<AutofillProfile*> profiles_to_delete;
   profiles_to_delete.reserve(existing_profiles.size());