Changes to web database and autofill components required

to sync profiles / addresses.

The main part here is adding Refresh to the PersonalDataManager.
It changes the expectation that the PDM is the only thing modifying autofill,
which is necessary as the sync engine connects directly to the WebDatabase
on the DB thread.

The tricky part is ID generation, which I spent a great deal of time harping
over in my sync change to make sure that everything is in an eventually
consistent state.  Note that because of the way the autofill window takes an
isolated copy of the data, there *are* extremely rare cases where an edit will
get dropped - I cover this case in PersonalDataManagerTest.Refresh.

TEST=WebDataServiceTest, PersonalDataManagerTest.

Review URL: http://codereview.chromium.org/1550007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43426 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 302 insertions, 57 deletions

diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 27a0923..593e47b 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/autofill/autofill_field.h"
 #include "chrome/browser/autofill/form_structure.h"
 #include "chrome/browser/autofill/phone_number.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/webdata/web_data_service.h"
 #include "chrome/browser/pref_service.h"
@@ -78,10 +79,14 @@ void PersonalDataManager::OnAutoFillDialogApply(
     std::vector<CreditCard>* credit_cards) {
   // |profiles| may be NULL
   // |credit_cards| may be NULL
-  if (profiles)
+  if (profiles) {
+    CancelPendingQuery(&pending_profiles_query_);
     SetProfiles(profiles);
-  if (credit_cards)
+  }
+  if (credit_cards) {
+    CancelPendingQuery(&pending_creditcards_query_);
     SetCreditCards(credit_cards);
+  }
 }
 
 void PersonalDataManager::SetObserver(PersonalDataManager::Observer* observer) {
@@ -115,6 +120,7 @@ bool PersonalDataManager::ImportFormData(
     AutoFillManager* autofill_manager) {
   InitializeIfNeeded();
 
+  AutoLock lock(unique_ids_lock_);
   // Parse the form and construct a profile based on the information that is
   // possible to import.
   int importable_fields = 0;
@@ -236,6 +242,7 @@ void PersonalDataManager::SetProfiles(std::vector<AutoFillProfile>* profiles) {
   if (!wds)
     return;
 
+  AutoLock lock(unique_ids_lock_);
   // Remove the unique IDs of the new set of profiles from the unique ID set.
   for (std::vector<AutoFillProfile>::iterator iter = profiles->begin();
        iter != profiles->end(); ++iter) {
@@ -283,6 +290,9 @@ void PersonalDataManager::SetProfiles(std::vector<AutoFillProfile>* profiles) {
        iter != profiles->end(); ++iter) {
     web_profiles_.push_back(new AutoFillProfile(*iter));
   }
+
+  // Read our writes to ensure consistency with the database.
+  Refresh();
 }
 
 void PersonalDataManager::SetCreditCards(
@@ -294,6 +304,7 @@ void PersonalDataManager::SetCreditCards(
   if (!wds)
     return;
 
+  AutoLock lock(unique_ids_lock_);
   // Remove the unique IDs of the new set of credit cards from the unique ID
   // set.
   for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
@@ -407,12 +418,16 @@ const std::vector<AutoFillProfile*>& PersonalDataManager::web_profiles() {
   return web_profiles_.get();
 }
 
-PersonalDataManager::PersonalDataManager(Profile* profile)
-    : profile_(profile),
+PersonalDataManager::PersonalDataManager()
+    : profile_(NULL),
       is_initialized_(false),
       is_data_loaded_(false),
       pending_profiles_query_(0),
       pending_creditcards_query_(0) {
+}
+
+void PersonalDataManager::Init(Profile* profile) {
+  profile_ = profile;
   LoadProfiles();
   LoadCreditCards();
 }
@@ -428,6 +443,7 @@ void PersonalDataManager::InitializeIfNeeded() {
 int PersonalDataManager::CreateNextUniqueID(std::set<int>* unique_ids) {
   // Profile IDs MUST start at 1 to allow 0 as an error value when reading
   // the ID from the WebDB (see LoadData()).
+  unique_ids_lock_.AssertAcquired();
   int id = 1;
   while (unique_ids->count(id) != 0)
     ++id;
@@ -473,6 +489,7 @@ void PersonalDataManager::ReceiveLoadedProfiles(WebDataService::Handle h,
   DCHECK_EQ(pending_profiles_query_, h);
   pending_profiles_query_ = 0;
 
+  AutoLock lock(unique_ids_lock_);
   unique_profile_ids_.clear();
   web_profiles_.reset();
 
@@ -492,6 +509,7 @@ void PersonalDataManager::ReceiveLoadedCreditCards(
   DCHECK_EQ(pending_creditcards_query_, h);
   pending_creditcards_query_ = 0;
 
+  AutoLock lock(unique_ids_lock_);
   unique_creditcard_ids_.clear();
   credit_cards_.reset();
 
@@ -518,3 +536,18 @@ void PersonalDataManager::CancelPendingQuery(WebDataService::Handle* handle) {
   }
   *handle = 0;
 }
+
+AutoFillProfile* PersonalDataManager::CreateNewEmptyAutoFillProfileForDBThread(
+    const string16& label) {
+  // See comment in header for thread details.
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB));
+  AutoLock lock(unique_ids_lock_);
+  AutoFillProfile* p = new AutoFillProfile(label,
+      CreateNextUniqueID(&unique_profile_ids_));
+  return p;
+}
+
+void PersonalDataManager::Refresh() {
+  LoadProfiles();
+  LoadCreditCards();
+}
diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h
index b30913d..f90440c 100644
--- a/chrome/browser/autofill/personal_data_manager.h
+++ b/chrome/browser/autofill/personal_data_manager.h
@@ -70,6 +70,15 @@ class PersonalDataManager : public WebDataServiceConsumer,
   // Sets |web_profiles_| to the contents of |profiles| and updates the web
   // database by adding, updating and removing profiles.  Sets the unique ID of
   // newly-added profiles.
+  //
+  // The relationship between this and Refresh is subtle.
+  // A call to SetProfile could include out-of-date data that may conflict
+  // if we didn't refresh-to-latest before an autofill window was opened for
+  // editing. SetProfile is implemented to make a "best effort" to apply the
+  // changes, but in extremely rare edge cases it is possible not all of the
+  // updates in |profiles| make it to the DB.  This is why SetProfiles will
+  // invoke Refresh after finishing, to ensure we get into a
+  // consistent state.  See Refresh for details.
   void SetProfiles(std::vector<AutoFillProfile>* profiles);
 
   // Sets |credit_cards_| to the contents of |credit_cards| and updates the web
@@ -86,7 +95,7 @@ class PersonalDataManager : public WebDataServiceConsumer,
   bool HasPassword();
 
   // Returns whether the personal data has been loaded from the web database.
-  bool IsDataLoaded() const { return is_data_loaded_; }
+  virtual bool IsDataLoaded() const { return is_data_loaded_; }
 
   // This PersonalDataManager owns these profiles and credit cards.  Their
   // lifetime is until the web database is updated with new profile and credit
@@ -96,13 +105,42 @@ class PersonalDataManager : public WebDataServiceConsumer,
   const std::vector<AutoFillProfile*>& web_profiles();
   const std::vector<CreditCard*>& credit_cards() { return credit_cards_.get(); }
 
- private:
+  // Creates a profile labeled |label|, with it's own locally unique ID.
+  // This must be called on the DB thread with the expectation that the
+  // returned form will be synchronously persisted to the WebDatabase.  See
+  // Refresh and SetProfiles for details.
+  AutoFillProfile* CreateNewEmptyAutoFillProfileForDBThread(
+      const string16& label);
+
+  // Re-loads profiles and credit cards from the WebDatabase asynchronously.
+  // In the general case, this is a no-op and will re-create the same
+  // in-memory model as existed prior to the call.  If any change occurred to
+  // profiles in the WebDatabase directly, as is the case if the browser sync
+  // engine processed a change from the cloud, we will learn of these as a
+  // result of this call.
+  //
+  // Note that there is a subtle relationship with ID generation.  IDs can be
+  // generated by CreateNewEmptyAutoFillProfileForDBThread (in a synchronized
+  // way), meaning that it is possible we are aware of this new profile only
+  // by having it's ID tracked in unique_profile_ids_ for a period of time.
+  // Because the expectation of that call is that the ID we generate will be
+  // synchronously persisted to the DB, we are guaranteed to read it via
+  // the next call to Refresh.  It could get deleted before we
+  // manage, but this is safe (we just hold on to the ID a bit longer).
+  //
+  // Also see SetProfile for more details.
+  virtual void Refresh();
+
+  // Kicks off asynchronous loading of profiles and credit cards.
+  void Init(Profile* profile);
+
+ protected:
   // Make sure that only Profile and the PersonalDataManager tests can create an
   // instance of PersonalDataManager.
   friend class ProfileImpl;
   friend class PersonalDataManagerTest;
 
-  explicit PersonalDataManager(Profile* profile);
+  PersonalDataManager();
 
   // Returns the profile of the tab contents.
   Profile* profile();
@@ -116,13 +154,13 @@ class PersonalDataManager : public WebDataServiceConsumer,
   int CreateNextUniqueID(std::set<int>* unique_ids);
 
   // Loads the saved profiles from the web database.
-  void LoadProfiles();
+  virtual void LoadProfiles();
 
   // Loads the auxiliary profiles.  Currently Mac only.
   void LoadAuxiliaryProfiles();
 
   // Loads the saved credit cards from the web database.
-  void LoadCreditCards();
+  virtual void LoadCreditCards();
 
   // Receives the loaded profiles from the web data service and stores them in
   // |credit_cards_|.
@@ -155,6 +193,9 @@ class PersonalDataManager : public WebDataServiceConsumer,
   // unique credit card ID.
   std::set<int> unique_creditcard_ids_;
 
+  // Protects unique_*_ids_ members.
+  Lock unique_ids_lock_;
+
   // The loaded web profiles.
   ScopedVector<AutoFillProfile> web_profiles_;
 
diff --git a/chrome/browser/autofill/personal_data_manager_unittest.cc b/chrome/browser/autofill/personal_data_manager_unittest.cc
index 986f11f..eaeca7e 100644
--- a/chrome/browser/autofill/personal_data_manager_unittest.cc
+++ b/chrome/browser/autofill/personal_data_manager_unittest.cc
@@ -59,7 +59,8 @@ class PersonalDataManagerTest : public testing::Test {
   }
 
   void ResetPersonalDataManager() {
-    personal_data_.reset(new PersonalDataManager(profile_.get()));
+    personal_data_.reset(new PersonalDataManager());
+    personal_data_->Init(profile_.get());
     personal_data_->SetObserver(&personal_data_observer_);
 
     // Disable auxiliary profiles for unit testing.  These reach out to system
@@ -68,6 +69,13 @@ class PersonalDataManagerTest : public testing::Test {
       prefs::kAutoFillAuxiliaryProfilesEnabled, false);
   }
 
+  AutoFillProfile* MakeProfile() {
+    AutoLock lock(personal_data_->unique_ids_lock_);
+    return new AutoFillProfile(string16(),
+        personal_data_->CreateNextUniqueID(
+        &personal_data_->unique_profile_ids_));
+  }
+
   MessageLoopForUI message_loop_;
   ChromeThread ui_thread_;
   ChromeThread db_thread_;
@@ -246,3 +254,91 @@ TEST_F(PersonalDataManagerTest, SetCreditCards) {
   EXPECT_EQ(creditcard0, *results3.at(0));
   EXPECT_EQ(creditcard2, *results3.at(1));
 }
+
+TEST_F(PersonalDataManagerTest, Refresh) {
+  AutoFillProfile profile0(string16(), 0);
+  autofill_unittest::SetProfileInfo(&profile0,
+      "Billing", "Marion", "Mitchell", "Morrison",
+      "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA",
+      "91601", "US", "12345678910", "01987654321");
+
+  AutoFillProfile profile1(string16(), 0);
+  autofill_unittest::SetProfileInfo(&profile1,
+      "Home", "Josephine", "Alicia", "Saenz",
+      "joewayne@me.xyz", "Fox", "903 Apple Ct.", NULL, "Orlando", "FL", "32801",
+      "US", "19482937549", "13502849239");
+
+  EXPECT_CALL(personal_data_observer_,
+      OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+
+  MessageLoop::current()->Run();
+
+  // Add the test profiles to the database.
+  std::vector<AutoFillProfile> update;
+  update.push_back(profile0);
+  update.push_back(profile1);
+  personal_data_->SetProfiles(&update);
+
+  profile0.set_unique_id(update[0].unique_id());
+  profile1.set_unique_id(update[1].unique_id());
+
+  // Wait for the refresh.
+  EXPECT_CALL(personal_data_observer_,
+      OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+
+  MessageLoop::current()->Run();
+
+  const std::vector<AutoFillProfile*>& results1 = personal_data_->profiles();
+  ASSERT_EQ(2U, results1.size());
+  EXPECT_EQ(profile0, *results1.at(0));
+  EXPECT_EQ(profile1, *results1.at(1));
+
+  scoped_ptr<AutoFillProfile> profile2(MakeProfile());
+  autofill_unittest::SetProfileInfo(profile2.get(),
+      "Work", "Josephine", "Alicia", "Saenz",
+      "joewayne@me.xyz", "Fox", "1212 Center.", "Bld. 5", "Orlando", "FL",
+      "32801", "US", "19482937549", "13502849239");
+
+  WebDataService* wds = profile_->GetWebDataService(Profile::EXPLICIT_ACCESS);
+  ASSERT_TRUE(wds);
+  wds->AddAutoFillProfile(*profile2.get());
+
+  personal_data_->Refresh();
+
+  // Wait for the refresh.
+  EXPECT_CALL(personal_data_observer_,
+    OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+
+  MessageLoop::current()->Run();
+
+  const std::vector<AutoFillProfile*>& results2 = personal_data_->profiles();
+  ASSERT_EQ(3U, results2.size());
+  EXPECT_EQ(profile0, *results2.at(0));
+  EXPECT_EQ(profile1, *results2.at(1));
+  EXPECT_EQ(*profile2.get(), *results2.at(2));
+
+  wds->RemoveAutoFillProfile(profile1.unique_id());
+  wds->RemoveAutoFillProfile(profile2->unique_id());
+
+  // Before telling the PDM to refresh, simulate an edit to one of the profiles
+  // via a SetProfile update (this would happen if the autofill window was
+  // open with a previous snapshot of the profiles, and something [e.g. sync]
+  // removed a profile from the browser.  In this edge case, we will end up
+  // in a consistent state by dropping the write).
+  profile2->SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Jo"));
+  update.clear();
+  update.push_back(profile0);
+  update.push_back(profile1);
+  update.push_back(*profile2.get());
+  personal_data_->SetProfiles(&update);
+
+  // And wait for the refresh.
+  EXPECT_CALL(personal_data_observer_,
+      OnPersonalDataLoaded()).WillOnce(QuitUIMessageLoop());
+
+  MessageLoop::current()->Run();
+
+  const std::vector<AutoFillProfile*>& results3 = personal_data_->profiles();
+  ASSERT_EQ(1U, results3.size());
+  EXPECT_EQ(profile0, *results2.at(0));
+}
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index c808574..504c10d 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -1192,7 +1192,8 @@ bool ProfileImpl::HasCreatedDownloadManager() const {
 
 PersonalDataManager* ProfileImpl::GetPersonalDataManager() {
   if (!personal_data_manager_.get()) {
-    personal_data_manager_.reset(new PersonalDataManager(this));
+    personal_data_manager_.reset(new PersonalDataManager());
+    personal_data_manager_->Init(this);
   }
   return personal_data_manager_.get();
 }
diff --git a/chrome/browser/webdata/autofill_change.h b/chrome/browser/webdata/autofill_change.h
index 2b7be32..b1108ad 100644
--- a/chrome/browser/webdata/autofill_change.h
+++ b/chrome/browser/webdata/autofill_change.h
@@ -46,26 +46,36 @@ class AutofillChange : public GenericAutofillChange<AutofillKey> {
   }
 };
 
-class AutofillProfileChange : public GenericAutofillChange<int> {
+class AutofillProfileChange : public GenericAutofillChange<string16> {
  public:
-  // If t == REMOVE, |p| should be NULL.
-  AutofillProfileChange(Type t, int k, const AutoFillProfile* p)
-      : GenericAutofillChange<int>(t, k), profile_(p) {}
+  // If t == REMOVE, |p| should be NULL. |pre_update_label| only applies to
+  // UPDATE changes.
+  AutofillProfileChange(Type t, string16 k, const AutoFillProfile* p,
+      const string16& pre_update_label)
+      : GenericAutofillChange<string16>(t, k), profile_(p),
+        pre_update_label_(pre_update_label) {}
 
   const AutoFillProfile* profile() const { return profile_; }
+  const string16& pre_update_label() const { return pre_update_label_; }
   bool operator==(const AutofillProfileChange& change) const {
-    return type() == change.type() && key() == change.key() &&
-        (type() != REMOVE) ? *profile() == *change.profile() : true;
+    if (type() != change.type() || key() != change.key())
+      return false;
+    if (type() == REMOVE)
+      return true;
+    if (*profile() != *change.profile())
+      return false;
+    return type() == ADD || pre_update_label_ == change.pre_update_label();
   }
  private:
   const AutoFillProfile* profile_;  // Unowned pointer, can be NULL.
+  const string16 pre_update_label_;
 };
 
-class AutofillCreditCardChange : public GenericAutofillChange<int> {
+class AutofillCreditCardChange : public GenericAutofillChange<string16> {
  public:
   // If t == REMOVE, |card| should be NULL.
-  AutofillCreditCardChange(Type t, int k, const CreditCard* card)
-      : GenericAutofillChange<int>(t, k), credit_card_(card) {}
+  AutofillCreditCardChange(Type t, string16 k, const CreditCard* card)
+      : GenericAutofillChange<string16>(t, k), credit_card_(card) {}
 
   const CreditCard* credit_card() const { return credit_card_; }
   bool operator==(const AutofillCreditCardChange& change) const {
diff --git a/chrome/browser/webdata/web_data_service.cc b/chrome/browser/webdata/web_data_service.cc
index 8811e10..1874564 100644
--- a/chrome/browser/webdata/web_data_service.cc
+++ b/chrome/browser/webdata/web_data_service.cc
@@ -789,7 +789,7 @@ void WebDataService::AddAutoFillProfileImpl(
     ScheduleCommit();
 
     AutofillProfileChange change(AutofillProfileChange::ADD,
-                                 profile.unique_id(), &profile);
+                                 profile.Label(), &profile, string16());
     NotificationService::current()->Notify(
         NotificationType::AUTOFILL_PROFILE_CHANGED,
         NotificationService::AllSources(),
@@ -803,16 +803,25 @@ void WebDataService::UpdateAutoFillProfileImpl(
   InitializeDatabaseIfNecessary();
   if (db_ && !request->IsCancelled()) {
     const AutoFillProfile& profile = request->GetArgument();
-    if (!db_->UpdateAutoFillProfile(profile))
+    // The AUTOFILL_PROFILE_CHANGED contract for an update requires that we
+    // send along the label of the un-updated profile, to detect label
+    // changes separately.  So first, we query for the existing profile.
+    AutoFillProfile* old_profile = NULL;
+    if (!db_->GetAutoFillProfileForID(profile.unique_id(), &old_profile))
       NOTREACHED();
-    ScheduleCommit();
+    if (old_profile) {
+      if (!db_->UpdateAutoFillProfile(profile))
+        NOTREACHED();
+      ScheduleCommit();
 
-    AutofillProfileChange change(AutofillProfileChange::UPDATE,
-                                 profile.unique_id(), &profile);
-    NotificationService::current()->Notify(
-        NotificationType::AUTOFILL_PROFILE_CHANGED,
-        NotificationService::AllSources(),
-        Details<AutofillProfileChange>(&change));
+      AutofillProfileChange change(AutofillProfileChange::UPDATE,
+                                   profile.Label(), &profile,
+                                   old_profile->Label());
+      NotificationService::current()->Notify(
+          NotificationType::AUTOFILL_PROFILE_CHANGED,
+          NotificationService::AllSources(),
+          Details<AutofillProfileChange>(&change));
+    }
   }
   request->RequestComplete();
 }
@@ -822,16 +831,23 @@ void WebDataService::RemoveAutoFillProfileImpl(
   InitializeDatabaseIfNecessary();
   if (db_ && !request->IsCancelled()) {
     int profile_id = request->GetArgument();
-    if (!db_->RemoveAutoFillProfile(profile_id))
+    AutoFillProfile* dead_profile = NULL;
+    if (!db_->GetAutoFillProfileForID(profile_id, &dead_profile))
       NOTREACHED();
-    ScheduleCommit();
 
-    AutofillProfileChange change(AutofillProfileChange::REMOVE,
-                                 profile_id, NULL);
-    NotificationService::current()->Notify(
-        NotificationType::AUTOFILL_PROFILE_CHANGED,
-        NotificationService::AllSources(),
-        Details<AutofillProfileChange>(&change));
+    if (dead_profile) {
+      if (!db_->RemoveAutoFillProfile(profile_id))
+        NOTREACHED();
+      ScheduleCommit();
+
+      AutofillProfileChange change(AutofillProfileChange::REMOVE,
+                                   dead_profile->Label(),
+                                   NULL, string16());
+      NotificationService::current()->Notify(
+          NotificationType::AUTOFILL_PROFILE_CHANGED,
+          NotificationService::AllSources(),
+          Details<AutofillProfileChange>(&change));
+    }
   }
   request->RequestComplete();
 }
@@ -858,7 +874,7 @@ void WebDataService::AddCreditCardImpl(
     ScheduleCommit();
 
     AutofillCreditCardChange change(AutofillCreditCardChange::ADD,
-                                    creditcard.unique_id(), &creditcard);
+        creditcard.Label(), &creditcard);
     NotificationService::current()->Notify(
         NotificationType::AUTOFILL_CREDIT_CARD_CHANGED,
         NotificationService::AllSources(),
@@ -877,7 +893,7 @@ void WebDataService::UpdateCreditCardImpl(
     ScheduleCommit();
 
     AutofillCreditCardChange change(AutofillCreditCardChange::UPDATE,
-                                    creditcard.unique_id(), &creditcard);
+        creditcard.Label(), &creditcard);
     NotificationService::current()->Notify(
         NotificationType::AUTOFILL_CREDIT_CARD_CHANGED,
         NotificationService::AllSources(),
@@ -891,16 +907,21 @@ void WebDataService::RemoveCreditCardImpl(
   InitializeDatabaseIfNecessary();
   if (db_ && !request->IsCancelled()) {
     int creditcard_id = request->GetArgument();
+    CreditCard* dead_card = NULL;
+    if (!db_->GetCreditCardForID(creditcard_id, &dead_card))
+      NOTREACHED();
     if (!db_->RemoveCreditCard(creditcard_id))
       NOTREACHED();
     ScheduleCommit();
 
-    AutofillCreditCardChange change(AutofillCreditCardChange::REMOVE,
-                                    creditcard_id, NULL);
-    NotificationService::current()->Notify(
-        NotificationType::AUTOFILL_CREDIT_CARD_CHANGED,
-        NotificationService::AllSources(),
-        Details<AutofillCreditCardChange>(&change));
+    if (dead_card) {
+      AutofillCreditCardChange change(AutofillCreditCardChange::REMOVE,
+                                      dead_card->Label(), NULL);
+      NotificationService::current()->Notify(
+          NotificationType::AUTOFILL_CREDIT_CARD_CHANGED,
+          NotificationService::AllSources(),
+          Details<AutofillCreditCardChange>(&change));
+    }
   }
   request->RequestComplete();
 }
diff --git a/chrome/browser/webdata/web_data_service_unittest.cc b/chrome/browser/webdata/web_data_service_unittest.cc
index d615902..80688a8 100644
--- a/chrome/browser/webdata/web_data_service_unittest.cc
+++ b/chrome/browser/webdata/web_data_service_unittest.cc
@@ -308,7 +308,7 @@ TEST_F(WebDataServiceAutofillTest,FormFillRemoveMany) {
 TEST_F(WebDataServiceAutofillTest, ProfileAdd) {
   AutoFillProfile profile(name1_, unique_id1_);
   const AutofillProfileChange expected_change(
-      AutofillProfileChange::ADD, unique_id1_, &profile);
+      AutofillProfileChange::ADD, name1_, &profile, string16());
 
   EXPECT_CALL(
       *observer_helper_->observer(),
@@ -331,7 +331,7 @@ TEST_F(WebDataServiceAutofillTest, ProfileRemove) {
   done_event_.TimedWait(test_timeout_);
 
   const AutofillProfileChange expected_change(
-      AutofillProfileChange::REMOVE, unique_id1_, NULL);
+      AutofillProfileChange::REMOVE, name1_, NULL, string16());
   EXPECT_CALL(
       *observer_helper_->observer(),
       Observe(NotificationType(NotificationType::AUTOFILL_PROFILE_CHANGED),
@@ -358,9 +358,10 @@ TEST_F(WebDataServiceAutofillTest, ProfileUpdate) {
   done_event_.TimedWait(test_timeout_);
 
   AutoFillProfile profile1_delta(profile1);
-  profile1_delta.set_label(ASCIIToUTF16("new_label!"));
+  string16 new_label(ASCIIToUTF16("new_label!"));
+  profile1_delta.set_label(new_label);
   const AutofillProfileChange expected_change(
-      AutofillProfileChange::UPDATE, unique_id1_, &profile1_delta);
+      AutofillProfileChange::UPDATE, new_label, &profile1_delta, name1_);
 
   EXPECT_CALL(
       *observer_helper_->observer(),
@@ -377,7 +378,7 @@ TEST_F(WebDataServiceAutofillTest, ProfileUpdate) {
 TEST_F(WebDataServiceAutofillTest, CreditAdd) {
   CreditCard card(name1_, unique_id1_);
   const AutofillCreditCardChange expected_change(
-      AutofillCreditCardChange::ADD, unique_id1_, &card);
+      AutofillCreditCardChange::ADD, name1_, &card);
 
   EXPECT_CALL(
       *observer_helper_->observer(),
@@ -399,7 +400,7 @@ TEST_F(WebDataServiceAutofillTest, CreditRemove) {
   done_event_.TimedWait(test_timeout_);
 
   const AutofillCreditCardChange expected_change(
-      AutofillCreditCardChange::REMOVE, unique_id1_, NULL);
+      AutofillCreditCardChange::REMOVE, name1_, NULL);
 
   EXPECT_CALL(
       *observer_helper_->observer(),
@@ -428,7 +429,7 @@ TEST_F(WebDataServiceAutofillTest, CreditUpdate) {
   CreditCard card1_delta(card1);
   card1_delta.set_label(ASCIIToUTF16("new_label!"));
   const AutofillCreditCardChange expected_change(
-      AutofillCreditCardChange::UPDATE, unique_id1_, &card1_delta);
+      AutofillCreditCardChange::UPDATE, name1_, &card1_delta);
 
   EXPECT_CALL(
       *observer_helper_->observer(),
diff --git a/chrome/browser/webdata/web_database.cc b/chrome/browser/webdata/web_database.cc
index 91bb373..e447881 100644
--- a/chrome/browser/webdata/web_database.cc
+++ b/chrome/browser/webdata/web_database.cc
@@ -1519,6 +1519,23 @@ bool WebDatabase::RemoveAutoFillProfile(int profile_id) {
   return s.Run();
 }
 
+bool WebDatabase::GetAutoFillProfileForID(int profile_id,
+    AutoFillProfile** profile) {
+  sql::Statement s(db_.GetUniqueStatement(
+      "SELECT * FROM autofill_profiles "
+      "WHERE unique_id = ?"));
+  if (!s) {
+    NOTREACHED() << "Statement prepare failed";
+    return false;
+  }
+
+  s.BindInt(0, profile_id);
+  if (s.Step())
+    *profile = AutoFillProfileFromStatement(s);
+
+  return s.Succeeded();
+}
+
 static void BindCreditCardToStatement(const CreditCard& creditcard,
                                       sql::Statement* s) {
   s->BindString(0, UTF16ToUTF8(creditcard.Label()));
@@ -1602,6 +1619,24 @@ bool WebDatabase::GetCreditCardForLabel(const string16& label,
   return s.Succeeded();
 }
 
+bool WebDatabase::GetCreditCardForID(int card_id, CreditCard** card) {
+  sql::Statement s(db_.GetUniqueStatement(
+      "SELECT * FROM credit_cards "
+      "WHERE unique_id = ?"));
+  if (!s) {
+    NOTREACHED() << "Statement prepare failed";
+    return false;
+  }
+
+  s.BindInt(0, card_id);
+  if (!s.Step())
+    return false;
+
+  *card = CreditCardFromStatement(s);
+
+  return s.Succeeded();
+}
+
 bool WebDatabase::GetCreditCards(
     std::vector<CreditCard*>* creditcards) {
   DCHECK(creditcards);
diff --git a/chrome/browser/webdata/web_database.h b/chrome/browser/webdata/web_database.h
index 06727e9..07f30a9 100644
--- a/chrome/browser/webdata/web_database.h
+++ b/chrome/browser/webdata/web_database.h
@@ -217,21 +217,24 @@ class WebDatabase {
   virtual bool UpdateAutofillEntries(const std::vector<AutofillEntry>& entries);
 
   // Records a single AutoFill profile in the autofill_profiles table.
-  bool AddAutoFillProfile(const AutoFillProfile& profile);
+  virtual bool AddAutoFillProfile(const AutoFillProfile& profile);
 
   // Updates the database values for the specified profile.
-  bool UpdateAutoFillProfile(const AutoFillProfile& profile);
+  virtual bool UpdateAutoFillProfile(const AutoFillProfile& profile);
 
   // Removes a row from the autofill_profiles table.  |profile_id| is the
   // unique ID of the profile to remove.
-  bool RemoveAutoFillProfile(int profile_id);
+  virtual bool RemoveAutoFillProfile(int profile_id);
+
+  // Retrieves profile for unique id |profile_id|, owned by caller.
+  bool GetAutoFillProfileForID(int profile_id, AutoFillProfile** profile);
 
   // Retrieves a profile with label |label|.  The caller owns |profile|.
   bool GetAutoFillProfileForLabel(const string16& label,
                                   AutoFillProfile** profile);
 
   // Retrieves all profiles in the database.  Caller owns the returned profiles.
-  bool GetAutoFillProfiles(std::vector<AutoFillProfile*>* profiles);
+  virtual bool GetAutoFillProfiles(std::vector<AutoFillProfile*>* profiles);
 
   // Records a single credit card in the credit_cards table.
   bool AddCreditCard(const CreditCard& creditcard);
@@ -247,8 +250,11 @@ class WebDatabase {
   bool GetCreditCardForLabel(const string16& label,
                                   CreditCard** profile);
 
+  // Retrieves credit card for a card with unique id |card_id|.
+  bool GetCreditCardForID(int card_id, CreditCard** card);
+
   // Retrieves all profiles in the database.  Caller owns the returned profiles.
-  bool GetCreditCards(std::vector<CreditCard*>* profiles);
+  virtual bool GetCreditCards(std::vector<CreditCard*>* profiles);
 
   //////////////////////////////////////////////////////////////////////////////
   //
diff --git a/chrome/test/profile_mock.h b/chrome/test/profile_mock.h
index 11401ea..bfc4329 100644
--- a/chrome/test/profile_mock.h
+++ b/chrome/test/profile_mock.h
@@ -15,6 +15,7 @@ class ProfileMock : public TestingProfile {
   MOCK_METHOD1(GetHistoryService, HistoryService*(ServiceAccessType access));
   MOCK_METHOD0(GetHistoryServiceWithoutCreating, HistoryService*());
   MOCK_METHOD1(GetWebDataService, WebDataService*(ServiceAccessType access));
+  MOCK_METHOD0(GetPersonalDataManager, PersonalDataManager*());
 };
 
 #endif  // CHROME_TEST_PROFILE_MOCK_H__