Autofill saves duplicate profiles and credit card info in preferences

chrome/browser/autofill/personal_data_manager.cc

Index: chrome/browser/autofill/personal_data_manager.cc
diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 8554064766ceebd56d20ab168f91a943eaf4adda..854d89d48d2cb62074249ab26cf5ee1c0cecf067 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -11,6 +11,7 @@
 #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"
@@ -28,9 +29,11 @@ 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_;
@@ -41,9 +44,11 @@ class FormGroupGUIDMatchesFunctor {
 };
 
 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_;
@@ -53,6 +58,14 @@ class FormGroupGUIDMatchesFunctor<T *> {
   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:
@@ -67,12 +80,6 @@ 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() {
@@ -313,10 +320,11 @@ void PersonalDataManager::SetProfiles(std::vector<AutoFillProfile>* profiles) {
       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);
   }
 
@@ -366,10 +374,11 @@ void PersonalDataManager::SetCreditCards(
       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);
   }
 
@@ -479,7 +488,7 @@ void PersonalDataManager::RemoveProfile(const std::string& guid) {
   // 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);
@@ -535,7 +544,7 @@ void PersonalDataManager::RemoveCreditCard(const std::string& guid) {
   // 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);