Autofill saves duplicate profiles and credit card info in preferences

chrome/browser/autofill/personal_data_manager.cc

 // Copyright (c) 2010 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 "chrome/browser/autofill/personal_data_manager.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_field.h"
+#include "chrome/browser/autofill/autofill-inl.h"
 #include "chrome/browser/autofill/form_structure.h"
 #include "chrome/browser/autofill/phone_number.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/webdata/web_data_service.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
 
 namespace {
 
 // The minimum number of fields that must contain user data and have known types
 // before AutoFill will attempt to import the data into a profile or a credit
 // card.
 const int kMinProfileImportSize = 3;
 const int kMinCreditCardImportSize = 2;
 
 template<typename T>
-class FormGroupGUIDMatchesFunctor {
+class FormGroupMatchesByGUIDFunctor {
  public:
-  explicit FormGroupGUIDMatchesFunctor(const std::string& guid) : guid_(guid) {}
+  explicit FormGroupMatchesByGUIDFunctor(const std::string& guid)
+      : guid_(guid) {
+  }
 
   bool operator()(const T& form_group) {
     return form_group.guid() == guid_;
   }
 
  private:
   std::string guid_;
 };
 
 template<typename T>
-class FormGroupGUIDMatchesFunctor<T *> {
+class FormGroupMatchesByGUIDFunctor<T *> {
  public:
-  explicit FormGroupGUIDMatchesFunctor(const std::string& guid) : guid_(guid) {}
+  explicit FormGroupMatchesByGUIDFunctor(const std::string& guid)
+      : guid_(guid) {
+  }
 
   bool operator()(const T* form_group) {
     return form_group->guid() == guid_;
   }
 
  private:
   std::string guid_;
 };

[Ilya Sherman, 2010/12/20 23:41:32]

nit: Can we similarly merge these two |FormGroupMatchesByGUIDFunctor| classes
into one with an overloaded parens-operator?

[dhollowa, 2010/12/20 23:55:34]

Done.

 
+template<typename T, typename C>
+bool FindByGUID(const C& container, const std::string& guid) {
+  return std::find_if(
+      container.begin(),
+      container.end(),
+      FormGroupMatchesByGUIDFunctor<T>(guid)) != container.end();
+}
+
 template<typename T>
 class DereferenceFunctor {
  public:
   template<typename T_Iterator>
   const T& operator()(const T_Iterator& iterator) {
     return *iterator;
   }
 };
 
 template<typename T>
 T* address_of(T& v) {
   return &v;
 }
 
-template<typename T, typename C>
-bool FindByGUID(const C& container, const std::string& guid) {
-  return std::find_if(container.begin(), container.end(),
-                      FormGroupGUIDMatchesFunctor<T>(guid)) != container.end();
-}
-
 }  // namespace
 
 PersonalDataManager::~PersonalDataManager() {
   CancelPendingQuery(&pending_profiles_query_);
   CancelPendingQuery(&pending_creditcards_query_);
 }
 
 void PersonalDataManager::OnWebDataServiceRequestDone(
     WebDataService::Handle h,
     const WDTypedResult* result) {
@@ skipping 220 matching lines @@
       wds->RemoveAutoFillProfileGUID((*iter)->guid());
   }
 
   // Update the web database with the existing profiles.
   for (std::vector<AutoFillProfile>::iterator iter = profiles->begin();
        iter != profiles->end(); ++iter) {
     if (FindByGUID<AutoFillProfile*>(web_profiles_, iter->guid()))
       wds->UpdateAutoFillProfileGUID(*iter);
   }
 
-  // Add the new profiles to the web database.
+  // Add the new profiles to the web database.  Don't add a duplicate.
   for (std::vector<AutoFillProfile>::iterator iter = profiles->begin();
        iter != profiles->end(); ++iter) {
-    if (!FindByGUID<AutoFillProfile*>(web_profiles_, iter->guid()))
+    if (!FindByGUID<AutoFillProfile*>(web_profiles_, iter->guid()) &&
+        !FindByContents(web_profiles_, *iter))
       wds->AddAutoFillProfileGUID(*iter);
   }
 
   // Copy in the new profiles.
   web_profiles_.reset();
   for (std::vector<AutoFillProfile>::iterator iter = profiles->begin();
        iter != profiles->end(); ++iter) {
     web_profiles_.push_back(new AutoFillProfile(*iter));
   }
 
@@ skipping 29 matching lines @@
       wds->RemoveCreditCardGUID((*iter)->guid());
   }
 
   // Update the web database with the existing credit cards.
   for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
        iter != credit_cards->end(); ++iter) {
     if (FindByGUID<CreditCard*>(credit_cards_, iter->guid()))
       wds->UpdateCreditCardGUID(*iter);
   }
 
-  // Add the new credit cards to the web database.
+  // Add the new credit cards to the web database.  Don't add a duplicate.
   for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
        iter != credit_cards->end(); ++iter) {
-    if (!FindByGUID<CreditCard*>(credit_cards_, iter->guid()))
+    if (!FindByGUID<CreditCard*>(credit_cards_, iter->guid()) &&
+        !FindByContents(credit_cards_, *iter))
       wds->AddCreditCardGUID(*iter);
   }
 
   // Copy in the new credit cards.
   credit_cards_.reset();
   for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
        iter != credit_cards->end(); ++iter) {
     credit_cards_.push_back(new CreditCard(*iter));
   }
 
@@ skipping 89 matching lines @@
 void PersonalDataManager::RemoveProfile(const std::string& guid) {
   // TODO(jhawkins): Refactor SetProfiles so this isn't so hacky.
   std::vector<AutoFillProfile> profiles(web_profiles_.size());
   std::transform(web_profiles_.begin(), web_profiles_.end(),
                  profiles.begin(),
                  DereferenceFunctor<AutoFillProfile>());
 
   // Remove the profile that matches |guid|.
   profiles.erase(
       std::remove_if(profiles.begin(), profiles.end(),
-                     FormGroupGUIDMatchesFunctor<AutoFillProfile>(guid)),
+                     FormGroupMatchesByGUIDFunctor<AutoFillProfile>(guid)),
       profiles.end());
 
   SetProfiles(&profiles);
 }
 
 AutoFillProfile* PersonalDataManager::GetProfileByGUID(
     const std::string& guid) {
   for (std::vector<AutoFillProfile*>::iterator iter = web_profiles_->begin();
        iter != web_profiles_->end(); ++iter) {
     if ((*iter)->guid() == guid)
@@ skipping 35 matching lines @@
 void PersonalDataManager::RemoveCreditCard(const std::string& guid) {
   // TODO(jhawkins): Refactor SetCreditCards so this isn't so hacky.
   std::vector<CreditCard> credit_cards(credit_cards_.size());
   std::transform(credit_cards_.begin(), credit_cards_.end(),
                  credit_cards.begin(),
                  DereferenceFunctor<CreditCard>());
 
   // Remove the credit card that matches |guid|.
   credit_cards.erase(
       std::remove_if(credit_cards.begin(), credit_cards.end(),
-                     FormGroupGUIDMatchesFunctor<CreditCard>(guid)),
+                     FormGroupMatchesByGUIDFunctor<CreditCard>(guid)),
       credit_cards.end());
 
   SetCreditCards(&credit_cards);
 }
 
 CreditCard* PersonalDataManager::GetCreditCardByGUID(const std::string& guid) {
   for (std::vector<CreditCard*>::iterator iter = credit_cards_.begin();
        iter != credit_cards_.end(); ++iter) {
     if ((*iter)->guid() == guid)
       return *iter;
@@ skipping 245 matching lines @@
     }
 
     creditcards.push_back(**iter);
   }
 
   if (!merged)
     creditcards.push_back(*imported_credit_card_);
 
   SetCreditCards(&creditcards);
 }