[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..c3173f5c63e3d32af8187c541fc6da85a7077437 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::OnSyncServiceConfigured(
+    sync_driver::SyncService* sync_service) {
+  // We want to know if we when, if at all, we need to run autofill profile

[sebsg, 2016/07/12 18:59:53]

nit: We want to know if we when, ... -> We want to know when, ... ?

[Roger McFarlane (Chromium), 2016/07/12 19:52:48]

Done.

+  // 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, Refresh() will attempt the de-duplication now.
+  Refresh();
+}
+
 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_);
@@ -725,6 +745,9 @@ bool PersonalDataManager::HasServerData() const {
 }
 
 void PersonalDataManager::Refresh() {

[Mathieu, 2016/07/12 18:10:12]

Refresh is called all over the place in this file. Could there be a more
appropriate place to start the dedupe? I don't know that it's guaranteed that
sync is initialized at this point.

[Roger McFarlane (Chromium), 2016/07/12 18:42:19]

Done.

+  if (is_autofill_profile_dedupe_pending_)
+    ApplyDedupingRoutine();
+
   LoadProfiles();
   LoadCreditCards();
 }
@@ -1606,16 +1629,23 @@ void PersonalDataManager::ApplyProfileUseDatesFix() {
 }
 
 bool PersonalDataManager::ApplyDedupingRoutine() {
-  if (!IsAutofillProfileCleanupEnabled())
+  if (!is_autofill_profile_dedupe_pending_) {
     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) {
+    VLOG(1)
+        << "Autofill profile de-duplication already performed for this version";
     return false;
+  }
 
+  VLOG(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());
@@ -1637,9 +1667,6 @@ bool PersonalDataManager::ApplyDedupingRoutine() {
   pref_service_->SetInteger(prefs::kAutofillLastVersionDeduped,
                             current_major_version);
 
-  // Refresh the local cache and send notifications to observers.
-  Refresh();
-
   return true;
 }