contacts: Don't save deleted contacts to disk.

GoogleContactStore was previously passing all contacts to
ContactDatabase regardless of their "deleted" flag.  This
change makes it instead not save already-deleted contacts,
drop contacts when they become deleted, and track the
most-recent last-updated time from a deleted contact (which
is needed for incremental updates).

BUG=128805
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10933127

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

14 files changed, 389 insertions, 86 deletions

diff --git a/chrome/browser/chromeos/contacts/contact.proto b/chrome/browser/chromeos/contacts/contact.proto
index 3d605e2..2926b43 100644
--- a/chrome/browser/chromeos/contacts/contact.proto
+++ b/chrome/browser/chromeos/contacts/contact.proto
@@ -104,9 +104,13 @@ message Contact {
 
 // Singleton message used by ContactDatabase to store update-related metadata.
 message UpdateMetadata {
-  // Next ID to use: 2
+  // Next ID to use: 3
 
   // Time at which the last successful update was started, as given by
   // base::Time::ToInternalValue().
   optional int64 last_update_start_time = 1;
+
+  // Latest time that we've seen in a contact's |update_time| field.  Note that
+  // the time may have come from a deleted contact that has been discarded.
+  optional int64 last_contact_update_time = 2;
 }
diff --git a/chrome/browser/chromeos/contacts/contact_database.cc b/chrome/browser/chromeos/contacts/contact_database.cc
index fe8911b..e25d163 100644
--- a/chrome/browser/chromeos/contacts/contact_database.cc
+++ b/chrome/browser/chromeos/contacts/contact_database.cc
@@ -86,7 +86,8 @@ void ContactDatabase::Init(const FilePath& database_dir,
                  base::Owned(success)));
 }
 
-void ContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts,
+void ContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts_to_save,
+                                   scoped_ptr<ContactIds> contact_ids_to_delete,
                                    scoped_ptr<UpdateMetadata> metadata,
                                    bool is_full_update,
                                    SaveCallback callback) {
@@ -96,7 +97,8 @@ void ContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts,
       FROM_HERE,
       base::Bind(&ContactDatabase::SaveContactsFromTaskRunner,
                  base::Unretained(this),
-                 base::Passed(contacts.Pass()),
+                 base::Passed(contacts_to_save.Pass()),
+                 base::Passed(contact_ids_to_delete.Pass()),
                  base::Passed(metadata.Pass()),
                  is_full_update,
                  success),
@@ -213,13 +215,15 @@ void ContactDatabase::InitFromTaskRunner(const FilePath& database_dir,
 }
 
 void ContactDatabase::SaveContactsFromTaskRunner(
-    scoped_ptr<ContactPointers> contacts,
+    scoped_ptr<ContactPointers> contacts_to_save,
+    scoped_ptr<ContactIds> contact_ids_to_delete,
     scoped_ptr<UpdateMetadata> metadata,
     bool is_full_update,
     bool* success) {
   DCHECK(IsRunByTaskRunner());
   DCHECK(success);
-  VLOG(1) << "Saving " << contacts->size() << " contact(s) to database as "
+  VLOG(1) << "Saving " << contacts_to_save->size() << " contact(s) to database "
+          << "and deleting " << contact_ids_to_delete->size() << " as "
           << (is_full_update ? "full" : "incremental") << " update";
 
   *success = false;
@@ -237,6 +241,11 @@ void ContactDatabase::SaveContactsFromTaskRunner(
         keys_to_delete.insert(key);
       db_iterator->Next();
     }
+  } else {
+    for (ContactIds::const_iterator it = contact_ids_to_delete->begin();
+         it != contact_ids_to_delete->end(); ++it) {
+      keys_to_delete.insert(*it);
+    }
   }
 
   // TODO(derat): Serializing all of the contacts and so we can write them in a
@@ -245,8 +254,8 @@ void ContactDatabase::SaveContactsFromTaskRunner(
   // crash, maybe add a dummy "write completed" contact that's removed in the
   // first batch and added in the last.)
   leveldb::WriteBatch updates;
-  for (ContactPointers::const_iterator it = contacts->begin();
-       it != contacts->end(); ++it) {
+  for (ContactPointers::const_iterator it = contacts_to_save->begin();
+       it != contacts_to_save->end(); ++it) {
     const contacts::Contact& contact = **it;
     if (contact.contact_id() == kUpdateMetadataKey) {
       LOG(WARNING) << "Skipping contact with reserved ID "
diff --git a/chrome/browser/chromeos/contacts/contact_database.h b/chrome/browser/chromeos/contacts/contact_database.h
index 25f65f1..963f61d 100644
--- a/chrome/browser/chromeos/contacts/contact_database.h
+++ b/chrome/browser/chromeos/contacts/contact_database.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_CHROMEOS_CONTACTS_CONTACT_DATABASE_H_
 #define CHROME_BROWSER_CHROMEOS_CONTACTS_CONTACT_DATABASE_H_
 
+#include <string>
 #include <vector>
 
 #include "base/basictypes.h"
@@ -33,6 +34,7 @@ typedef std::vector<const Contact*> ContactPointers;
 // Interface for classes providing persistent storage of Contact objects.
 class ContactDatabaseInterface {
  public:
+  typedef std::vector<std::string> ContactIds;
   typedef base::Callback<void(bool success)> InitCallback;
   typedef base::Callback<void(bool success)> SaveCallback;
   typedef base::Callback<void(bool success,
@@ -50,12 +52,14 @@ class ContactDatabaseInterface {
   // UI thread when complete.
   virtual void Init(const FilePath& database_dir, InitCallback callback) = 0;
 
-  // Asynchronously saves |contacts| and |metadata| to the database.  If
+  // Asynchronously saves |contacts_to_save| and |metadata| to the database and
+  // removes contacts with IDs contained in |contact_ids_to_delete|.  If
   // |is_full_update| is true, all existing contacts in the database not present
-  // in |contacts| will be removed.  |callback| will be invoked on the UI thread
-  // when complete.  The caller must not make changes to the underlying
-  // passed-in Contact objects until the callback has been invoked.
-  virtual void SaveContacts(scoped_ptr<ContactPointers> contacts,
+  // in |contacts_to_save| will be removed.  |callback| will be invoked on the
+  // UI thread when complete.  The caller must not make changes to the
+  // underlying passed-in Contact objects until the callback has been invoked.
+  virtual void SaveContacts(scoped_ptr<ContactPointers> contacts_to_save,
+                            scoped_ptr<ContactIds> contact_ids_to_delete,
                             scoped_ptr<UpdateMetadata> metadata,
                             bool is_full_update,
                             SaveCallback callback) = 0;
@@ -79,7 +83,8 @@ class ContactDatabase : public ContactDatabaseInterface {
   virtual void DestroyOnUIThread() OVERRIDE;
   virtual void Init(const FilePath& database_dir,
                     InitCallback callback) OVERRIDE;
-  virtual void SaveContacts(scoped_ptr<ContactPointers> contacts,
+  virtual void SaveContacts(scoped_ptr<ContactPointers> contacts_to_save,
+                            scoped_ptr<ContactIds> contact_ids_to_delete,
                             scoped_ptr<UpdateMetadata> metadata,
                             bool is_full_update,
                             SaveCallback callback) OVERRIDE;
@@ -109,8 +114,9 @@ class ContactDatabase : public ContactDatabaseInterface {
   // Initializes the database in |database_dir| and updates |success|.
   void InitFromTaskRunner(const FilePath& database_dir, bool* success);
 
-  // Saves |contacts| to disk and updates |success|.
-  void SaveContactsFromTaskRunner(scoped_ptr<ContactPointers> contacts,
+  // Saves data to disk and updates |success|.
+  void SaveContactsFromTaskRunner(scoped_ptr<ContactPointers> contacts_to_save,
+                                  scoped_ptr<ContactIds> contact_ids_to_delete,
                                   scoped_ptr<UpdateMetadata> metadata,
                                   bool is_full_update,
                                   bool* success);
diff --git a/chrome/browser/chromeos/contacts/contact_database_unittest.cc b/chrome/browser/chromeos/contacts/contact_database_unittest.cc
index 31f8e40..751bd02 100644
--- a/chrome/browser/chromeos/contacts/contact_database_unittest.cc
+++ b/chrome/browser/chromeos/contacts/contact_database_unittest.cc
@@ -77,11 +77,16 @@ class ContactDatabaseTest : public testing::Test {
 
   // Calls ContactDatabase::SaveContacts() and blocks until the operation is
   // complete.
-  void SaveContacts(scoped_ptr<ContactPointers> contacts,
+  void SaveContacts(scoped_ptr<ContactPointers> contacts_to_save,
+                    scoped_ptr<ContactDatabaseInterface::ContactIds>
+                        contact_ids_to_delete,
                     scoped_ptr<UpdateMetadata> metadata,
                     bool is_full_update) {
     CHECK(db_);
-    db_->SaveContacts(contacts.Pass(), metadata.Pass(), is_full_update,
+    db_->SaveContacts(contacts_to_save.Pass(),
+                      contact_ids_to_delete.Pass(),
+                      metadata.Pass(),
+                      is_full_update,
                       base::Bind(&ContactDatabaseTest::OnContactsSaved,
                                  base::Unretained(this)));
     message_loop_.Run();
@@ -165,12 +170,17 @@ TEST_F(ContactDatabaseTest, SaveAndReload) {
   SetPhoto(gfx::Size(20, 20), contact.get());
   scoped_ptr<ContactPointers> contacts_to_save(new ContactPointers);
   contacts_to_save->push_back(contact.get());
+  scoped_ptr<ContactDatabaseInterface::ContactIds> contact_ids_to_delete(
+      new ContactDatabaseInterface::ContactIds);
 
   const int64 kLastUpdateTime = 1234;
   scoped_ptr<UpdateMetadata> metadata_to_save(new UpdateMetadata);
   metadata_to_save->set_last_update_start_time(kLastUpdateTime);
 
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), true);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               true);
   scoped_ptr<ScopedVector<Contact> > loaded_contacts;
   scoped_ptr<UpdateMetadata> loaded_metadata;
   LoadContacts(&loaded_contacts, &loaded_metadata);
@@ -190,10 +200,14 @@ TEST_F(ContactDatabaseTest, SaveAndReload) {
   SetPhoto(gfx::Size(64, 64), contact.get());
   contacts_to_save.reset(new ContactPointers);
   contacts_to_save->push_back(contact.get());
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
   metadata_to_save.reset(new UpdateMetadata);
   const int64 kNewLastUpdateTime = 5678;
   metadata_to_save->set_last_update_start_time(kNewLastUpdateTime);
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), true);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               true);
 
   LoadContacts(&loaded_contacts, &loaded_metadata);
   EXPECT_EQ(VarContactsToString(1, contact.get()),
@@ -219,8 +233,13 @@ TEST_F(ContactDatabaseTest, FullAndIncrementalUpdates) {
   scoped_ptr<ContactPointers> contacts_to_save(new ContactPointers);
   contacts_to_save->push_back(contact1.get());
   contacts_to_save->push_back(contact2.get());
+  scoped_ptr<ContactDatabaseInterface::ContactIds> contact_ids_to_delete(
+      new ContactDatabaseInterface::ContactIds);
   scoped_ptr<UpdateMetadata> metadata_to_save(new UpdateMetadata);
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), true);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               true);
 
   scoped_ptr<ScopedVector<Contact> > loaded_contacts;
   scoped_ptr<UpdateMetadata> loaded_metadata;
@@ -234,8 +253,12 @@ TEST_F(ContactDatabaseTest, FullAndIncrementalUpdates) {
                    "", true, contact2.get());
   contacts_to_save.reset(new ContactPointers);
   contacts_to_save->push_back(contact2.get());
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
   metadata_to_save.reset(new UpdateMetadata);
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), false);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               false);
   LoadContacts(&loaded_contacts, &loaded_metadata);
   EXPECT_EQ(VarContactsToString(2, contact1.get(), contact2.get()),
             ContactsToString(*loaded_contacts));
@@ -243,10 +266,14 @@ TEST_F(ContactDatabaseTest, FullAndIncrementalUpdates) {
   // Do an empty incremental update and check that the metadata is still
   // updated.
   contacts_to_save.reset(new ContactPointers);
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
   metadata_to_save.reset(new UpdateMetadata);
   const int64 kLastUpdateTime = 1234;
   metadata_to_save->set_last_update_start_time(kLastUpdateTime);
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), false);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               false);
   LoadContacts(&loaded_contacts, &loaded_metadata);
   EXPECT_EQ(VarContactsToString(2, contact1.get(), contact2.get()),
             ContactsToString(*loaded_contacts));
@@ -261,16 +288,24 @@ TEST_F(ContactDatabaseTest, FullAndIncrementalUpdates) {
                  "", true, contact1.get());
   contacts_to_save.reset(new ContactPointers);
   contacts_to_save->push_back(contact1.get());
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
   metadata_to_save.reset(new UpdateMetadata);
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), true);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               true);
   LoadContacts(&loaded_contacts, &loaded_metadata);
   EXPECT_EQ(VarContactsToString(1, contact1.get()),
             ContactsToString(*loaded_contacts));
 
   // Do a full update including no contacts.  The database should be cleared.
   contacts_to_save.reset(new ContactPointers);
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
   metadata_to_save.reset(new UpdateMetadata);
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), true);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               true);
   LoadContacts(&loaded_contacts, &loaded_metadata);
   EXPECT_TRUE(loaded_contacts->empty());
 }
@@ -292,8 +327,13 @@ TEST_F(ContactDatabaseTest, DeleteWhenCorrupt) {
   InitContact("1", "1", false, contact.get());
   scoped_ptr<ContactPointers> contacts_to_save(new ContactPointers);
   contacts_to_save->push_back(contact.get());
+  scoped_ptr<ContactDatabaseInterface::ContactIds> contact_ids_to_delete(
+      new ContactDatabaseInterface::ContactIds);
   scoped_ptr<UpdateMetadata> metadata_to_save(new UpdateMetadata);
-  SaveContacts(contacts_to_save.Pass(), metadata_to_save.Pass(), true);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               true);
 
   scoped_ptr<ScopedVector<Contact> > loaded_contacts;
   scoped_ptr<UpdateMetadata> loaded_metadata;
@@ -302,5 +342,75 @@ TEST_F(ContactDatabaseTest, DeleteWhenCorrupt) {
             ContactsToString(*loaded_contacts));
 }
 
+TEST_F(ContactDatabaseTest, DeleteRequestedContacts) {
+  // Insert two contacts into the database with a full update.
+  const std::string kContactId1 = "contact_id_1";
+  scoped_ptr<Contact> contact1(new Contact);
+  InitContact(kContactId1, "1", false, contact1.get());
+  const std::string kContactId2 = "contact_id_2";
+  scoped_ptr<Contact> contact2(new Contact);
+  InitContact(kContactId2, "2", false, contact2.get());
+
+  scoped_ptr<ContactPointers> contacts_to_save(new ContactPointers);
+  contacts_to_save->push_back(contact1.get());
+  contacts_to_save->push_back(contact2.get());
+  scoped_ptr<ContactDatabaseInterface::ContactIds> contact_ids_to_delete(
+      new ContactDatabaseInterface::ContactIds);
+  scoped_ptr<UpdateMetadata> metadata_to_save(new UpdateMetadata);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               true);
+
+  // Do an incremental update that inserts a third contact and deletes the first
+  // contact.
+  const std::string kContactId3 = "contact_id_3";
+  scoped_ptr<Contact> contact3(new Contact);
+  InitContact(kContactId3, "3", false, contact3.get());
+
+  contacts_to_save.reset(new ContactPointers);
+  contacts_to_save->push_back(contact3.get());
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
+  contact_ids_to_delete->push_back(kContactId1);
+  metadata_to_save.reset(new UpdateMetadata);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               false);
+
+  // LoadContacts() should return only the second and third contacts.
+  scoped_ptr<ScopedVector<Contact> > loaded_contacts;
+  scoped_ptr<UpdateMetadata> loaded_metadata;
+  LoadContacts(&loaded_contacts, &loaded_metadata);
+  EXPECT_EQ(VarContactsToString(2, contact2.get(), contact3.get()),
+            ContactsToString(*loaded_contacts));
+
+  // Do another incremental update that deletes the second contact.
+  contacts_to_save.reset(new ContactPointers);
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
+  contact_ids_to_delete->push_back(kContactId2);
+  metadata_to_save.reset(new UpdateMetadata);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               false);
+  LoadContacts(&loaded_contacts, &loaded_metadata);
+  EXPECT_EQ(VarContactsToString(1, contact3.get()),
+            ContactsToString(*loaded_contacts));
+
+  // Deleting a contact that isn't present should be a no-op.
+  contacts_to_save.reset(new ContactPointers);
+  contact_ids_to_delete.reset(new ContactDatabaseInterface::ContactIds);
+  contact_ids_to_delete->push_back("bogus_id");
+  metadata_to_save.reset(new UpdateMetadata);
+  SaveContacts(contacts_to_save.Pass(),
+               contact_ids_to_delete.Pass(),
+               metadata_to_save.Pass(),
+               false);
+  LoadContacts(&loaded_contacts, &loaded_metadata);
+  EXPECT_EQ(VarContactsToString(1, contact3.get()),
+            ContactsToString(*loaded_contacts));
+}
+
 }  // namespace test
 }  // namespace contacts
diff --git a/chrome/browser/chromeos/contacts/contact_map.cc b/chrome/browser/chromeos/contacts/contact_map.cc
index b24fbc0..f02aef4 100644
--- a/chrome/browser/chromeos/contacts/contact_map.cc
+++ b/chrome/browser/chromeos/contacts/contact_map.cc
@@ -17,6 +17,15 @@ const Contact* ContactMap::Find(const std::string& contact_id) const {
   return (it != contacts_.end()) ? it->second : NULL;
 }
 
+void ContactMap::Erase(const std::string& contact_id) {
+  Map::iterator it = contacts_.find(contact_id);
+  if (it == contacts_.end())
+    return;
+
+  delete it->second;
+  contacts_.erase(it);
+}
+
 void ContactMap::Clear() {
   STLDeleteValues(&contacts_);
 }
@@ -50,18 +59,4 @@ void ContactMap::Merge(scoped_ptr<ScopedVector<Contact> > updated_contacts,
   updated_contacts->weak_clear();
 }
 
-base::Time ContactMap::GetMaxUpdateTime() const {
-  base::Time max_update_time;
-  for (const_iterator it = begin(); it != end(); ++it) {
-    const Contact* contact = it->second;
-    const base::Time update_time =
-        base::Time::FromInternalValue(contact->update_time());
-    if (!update_time.is_null() &&
-        (max_update_time.is_null() || max_update_time < update_time)) {
-      max_update_time = update_time;
-    }
-  }
-  return max_update_time;
-}
-
 }  // namespace contacts
diff --git a/chrome/browser/chromeos/contacts/contact_map.h b/chrome/browser/chromeos/contacts/contact_map.h
index a726ddd..22de98e 100644
--- a/chrome/browser/chromeos/contacts/contact_map.h
+++ b/chrome/browser/chromeos/contacts/contact_map.h
@@ -47,6 +47,9 @@ class ContactMap {
   // isn't present.
   const Contact* Find(const std::string& contact_id) const;
 
+  // Deletes the contact with ID |contact_id|.
+  void Erase(const std::string& contact_id);
+
   // Deletes all contacts.
   void Clear();
 
@@ -54,10 +57,6 @@ class ContactMap {
   void Merge(scoped_ptr<ScopedVector<Contact> > updated_contacts,
              DeletedContactPolicy policy);
 
-  // Returns the maximum |update_time| value stored within a contact in
-  // |contacts_|.
-  base::Time GetMaxUpdateTime() const;
-
  private:
   Map contacts_;
 
diff --git a/chrome/browser/chromeos/contacts/contact_map_unittest.cc b/chrome/browser/chromeos/contacts/contact_map_unittest.cc
index c8e1d53..dc7bb96 100644
--- a/chrome/browser/chromeos/contacts/contact_map_unittest.cc
+++ b/chrome/browser/chromeos/contacts/contact_map_unittest.cc
@@ -18,14 +18,11 @@ TEST(ContactMapTest, Merge) {
   ContactMap map;
   EXPECT_TRUE(map.empty());
   EXPECT_EQ(0U, map.size());
-  EXPECT_EQ(base::Time(), map.GetMaxUpdateTime());
 
   // Create a contact.
   const std::string kContactId1 = "contact_id_1";
   scoped_ptr<Contact> contact1(new Contact);
   InitContact(kContactId1, "1", false, contact1.get());
-  const base::Time kUpdateTime1 = base::Time::FromInternalValue(100);
-  contact1->set_update_time(kUpdateTime1.ToInternalValue());
 
   // Merge it into the map and check that it's stored.
   scoped_ptr<ScopedVector<Contact> > contacts_to_merge(
@@ -36,15 +33,11 @@ TEST(ContactMapTest, Merge) {
   EXPECT_EQ(1U, map.size());
   ASSERT_TRUE(map.Find(kContactId1));
   EXPECT_EQ(ContactMapToString(map), VarContactsToString(1, contact1.get()));
-  EXPECT_EQ(kUpdateTime1, map.GetMaxUpdateTime());
 
-  // Create a second, deleted contact with a newer update time.
+  // Create a second, deleted contact.
   const std::string kContactId2 = "contact_id_2";
   scoped_ptr<Contact> contact2(new Contact);
   InitContact(kContactId2, "2", true, contact2.get());
-  const base::Time kUpdateTime2 =
-      kUpdateTime1 + base::TimeDelta::FromSeconds(10);
-  contact2->set_update_time(kUpdateTime2.ToInternalValue());
 
   // Merge it into the map.  Since we request keeping deleted contacts, the
   // contact should be saved.
@@ -55,26 +48,19 @@ TEST(ContactMapTest, Merge) {
   ASSERT_TRUE(map.Find(kContactId2));
   EXPECT_EQ(ContactMapToString(map),
             VarContactsToString(2, contact1.get(), contact2.get()));
-  EXPECT_EQ(kUpdateTime2, map.GetMaxUpdateTime());
 
   // Update the first contact's update time and merge it into the map.
-  const base::Time kNewUpdateTime1 =
-      kUpdateTime2 + base::TimeDelta::FromSeconds(20);
-  contact1->set_update_time(kNewUpdateTime1.ToInternalValue());
+  contact1->set_update_time(contact1->update_time() + 20);
   contacts_to_merge.reset(new ScopedVector<Contact>);
   contacts_to_merge->push_back(new Contact(*contact1));
   map.Merge(contacts_to_merge.Pass(), ContactMap::KEEP_DELETED_CONTACTS);
   EXPECT_EQ(ContactMapToString(map),
             VarContactsToString(2, contact1.get(), contact2.get()));
-  EXPECT_EQ(kNewUpdateTime1, map.GetMaxUpdateTime());
 
   // Create another deleted contact.
   const std::string kContactId3 = "contact_id_3";
   scoped_ptr<Contact> contact3(new Contact);
   InitContact(kContactId3, "3", true, contact3.get());
-  const base::Time kUpdateTime3 =
-      kNewUpdateTime1 + base::TimeDelta::FromSeconds(40);
-  contact3->set_update_time(kUpdateTime3.ToInternalValue());
 
   // Merge it into the map with DROP_DELETED_CONTACTS.  The contact shouldn't be
   // saved.
@@ -83,7 +69,6 @@ TEST(ContactMapTest, Merge) {
   map.Merge(contacts_to_merge.Pass(), ContactMap::DROP_DELETED_CONTACTS);
   EXPECT_EQ(ContactMapToString(map),
             VarContactsToString(2, contact1.get(), contact2.get()));
-  EXPECT_EQ(kNewUpdateTime1, map.GetMaxUpdateTime());
 
   // Mark the first contact as being deleted and merge it with
   // DROP_DELETED_CONTACTS.  The previous version of the contact should also be
@@ -93,12 +78,27 @@ TEST(ContactMapTest, Merge) {
   contacts_to_merge->push_back(new Contact(*contact1));
   map.Merge(contacts_to_merge.Pass(), ContactMap::DROP_DELETED_CONTACTS);
   EXPECT_EQ(ContactMapToString(map), VarContactsToString(1, contact2.get()));
-  EXPECT_EQ(kUpdateTime2, map.GetMaxUpdateTime());
 
   map.Clear();
   EXPECT_TRUE(map.empty());
   EXPECT_EQ(0U, map.size());
-  EXPECT_EQ(base::Time(), map.GetMaxUpdateTime());
+}
+
+TEST(ContactMapTest, Erase) {
+  ContactMap map;
+  const std::string kContactId = "contact_id";
+  scoped_ptr<Contact> contact(new Contact);
+  InitContact(kContactId, "1", false, contact.get());
+
+  scoped_ptr<ScopedVector<Contact> > contacts_to_merge(
+      new ScopedVector<Contact>);
+  contacts_to_merge->push_back(new Contact(*contact));
+  map.Merge(contacts_to_merge.Pass(), ContactMap::KEEP_DELETED_CONTACTS);
+  EXPECT_TRUE(map.Find(kContactId));
+
+  map.Erase(kContactId);
+  EXPECT_FALSE(map.Find(kContactId));
+  EXPECT_TRUE(map.empty());
 }
 
 }  // namespace test
diff --git a/chrome/browser/chromeos/contacts/fake_contact_database.cc b/chrome/browser/chromeos/contacts/fake_contact_database.cc
index f74d453..0ce9aab 100644
--- a/chrome/browser/chromeos/contacts/fake_contact_database.cc
+++ b/chrome/browser/chromeos/contacts/fake_contact_database.cc
@@ -28,7 +28,7 @@ void FakeContactDatabase::Init(const FilePath& database_dir,
 void FakeContactDatabase::SetContacts(const ContactPointers& contacts,
                                       const UpdateMetadata& metadata) {
   contacts_.Clear();
-  MergeContacts(contacts);
+  MergeContacts(contacts, ContactIds());
   metadata_ = metadata;
 }
 
@@ -37,16 +37,18 @@ void FakeContactDatabase::DestroyOnUIThread() {
   delete this;
 }
 
-void FakeContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts,
-                                       scoped_ptr<UpdateMetadata> metadata,
-                                       bool is_full_update,
-                                       SaveCallback callback) {
+void FakeContactDatabase::SaveContacts(
+    scoped_ptr<ContactPointers> contacts_to_save,
+    scoped_ptr<ContactIds> contact_ids_to_delete,
+    scoped_ptr<UpdateMetadata> metadata,
+    bool is_full_update,
+    SaveCallback callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (save_success_) {
-    num_saved_contacts_ += contacts->size();
+    num_saved_contacts_ += contacts_to_save->size();
     if (is_full_update)
       contacts_.Clear();
-    MergeContacts(*contacts);
+    MergeContacts(*contacts_to_save, *contact_ids_to_delete);
     metadata_ = *metadata;
   }
   callback.Run(save_success_);
@@ -70,11 +72,16 @@ FakeContactDatabase::~FakeContactDatabase() {
 }
 
 void FakeContactDatabase::MergeContacts(
-    const ContactPointers& updated_contacts) {
+    const ContactPointers& updated_contacts,
+    const ContactIds& contact_ids_to_delete) {
   scoped_ptr<ScopedVector<Contact> > copied_contacts(new ScopedVector<Contact>);
   for (size_t i = 0; i < updated_contacts.size(); ++i)
     copied_contacts->push_back(new Contact(*updated_contacts[i]));
   contacts_.Merge(copied_contacts.Pass(), ContactMap::KEEP_DELETED_CONTACTS);
+  for (ContactIds::const_iterator it = contact_ids_to_delete.begin();
+       it != contact_ids_to_delete.end(); ++it) {
+    contacts_.Erase(*it);
+  }
 }
 
 }  // namespace contacts
diff --git a/chrome/browser/chromeos/contacts/fake_contact_database.h b/chrome/browser/chromeos/contacts/fake_contact_database.h
index 1f70ab6..bcadc4e 100644
--- a/chrome/browser/chromeos/contacts/fake_contact_database.h
+++ b/chrome/browser/chromeos/contacts/fake_contact_database.h
@@ -18,6 +18,7 @@ class FakeContactDatabase : public ContactDatabaseInterface {
   FakeContactDatabase();
 
   const ContactMap& contacts() const { return contacts_; }
+  const UpdateMetadata& metadata() const { return metadata_; }
 
   void set_init_success(bool success) { init_success_ = success; }
   void set_save_success(bool success) { save_success_ = success; }
@@ -35,7 +36,8 @@ class FakeContactDatabase : public ContactDatabaseInterface {
   virtual void DestroyOnUIThread() OVERRIDE;
   virtual void Init(const FilePath& database_dir,
                     InitCallback callback) OVERRIDE;
-  virtual void SaveContacts(scoped_ptr<ContactPointers> contacts,
+  virtual void SaveContacts(scoped_ptr<ContactPointers> contacts_to_save,
+                            scoped_ptr<ContactIds> contact_ids_to_delete,
                             scoped_ptr<UpdateMetadata> metadata,
                             bool is_full_update,
                             SaveCallback callback) OVERRIDE;
@@ -45,8 +47,10 @@ class FakeContactDatabase : public ContactDatabaseInterface {
   virtual ~FakeContactDatabase();
 
  private:
-  // Merges |updated_contacts| into |contacts_|.
-  void MergeContacts(const ContactPointers& updated_contacts);
+  // Merges |updated_contacts| into |contacts_| and deletes contacts with IDs in
+  // |contact_ids_to_delete|.
+  void MergeContacts(const ContactPointers& updated_contacts,
+                     const ContactIds& contact_ids_to_delete);
 
   // Should we report success in response to various requests?
   bool init_success_;
diff --git a/chrome/browser/chromeos/contacts/google_contact_store.cc b/chrome/browser/chromeos/contacts/google_contact_store.cc
index a0ff97a..ac4b41a 100644
--- a/chrome/browser/chromeos/contacts/google_contact_store.cc
+++ b/chrome/browser/chromeos/contacts/google_contact_store.cc
@@ -81,6 +81,15 @@ void GoogleContactStore::TestAPI::NotifyAboutNetworkStateChange(bool online) {
   store_->OnConnectionTypeChanged(type);
 }
 
+scoped_ptr<ContactPointers> GoogleContactStore::TestAPI::GetLoadedContacts() {
+  scoped_ptr<ContactPointers> contacts(new ContactPointers);
+  for (ContactMap::const_iterator it = store_->contacts_.begin();
+       it != store_->contacts_.end(); ++it) {
+    contacts->push_back(it->second);
+  }
+  return contacts.Pass();
+}
+
 GoogleContactStore::GoogleContactStore(Profile* profile)
     : profile_(profile),
       db_(new ContactDatabase),
@@ -131,8 +140,7 @@ void GoogleContactStore::AppendContacts(ContactPointers* contacts_out) {
 const Contact* GoogleContactStore::GetContactById(
     const std::string& contact_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  const Contact* contact = contacts_.Find(contact_id);
-  return (contact && !contact->deleted()) ? contact : NULL;
+  return contacts_.Find(contact_id);
 }
 
 void GoogleContactStore::AddObserver(ContactStoreObserver* observer) {
@@ -240,17 +248,26 @@ void GoogleContactStore::MergeContacts(
     bool is_full_update,
     scoped_ptr<ScopedVector<Contact> > updated_contacts) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (is_full_update)
+
+  if (is_full_update) {
     contacts_.Clear();
-  size_t num_updated_contacts = updated_contacts->size();
-  contacts_.Merge(updated_contacts.Pass(), ContactMap::KEEP_DELETED_CONTACTS);
+    last_contact_update_time_ = base::Time();
+  }
 
-  if (is_full_update || num_updated_contacts > 0)
-    last_contact_update_time_ = contacts_.GetMaxUpdateTime();
+  // Find the maximum update time from |updated_contacts| since contacts whose
+  // |deleted| flags are set won't be saved to |contacts_|.
+  for (ScopedVector<Contact>::iterator it = updated_contacts->begin();
+       it != updated_contacts->end(); ++it) {
+    last_contact_update_time_ =
+        std::max(last_contact_update_time_,
+                 base::Time::FromInternalValue((*it)->update_time()));
+  }
   VLOG(1) << "Last contact update time is "
           << (last_contact_update_time_.is_null() ?
               std::string("null") :
               gdata::util::FormatTimeAsString(last_contact_update_time_));
+
+  contacts_.Merge(updated_contacts.Pass(), ContactMap::DROP_DELETED_CONTACTS);
 }
 
 void GoogleContactStore::OnDownloadSuccess(
@@ -263,9 +280,16 @@ void GoogleContactStore::OnDownloadSuccess(
 
   // Copy the pointers so we can update just these contacts in the database.
   scoped_ptr<ContactPointers> contacts_to_save_to_db(new ContactPointers);
+  scoped_ptr<ContactDatabaseInterface::ContactIds>
+      contact_ids_to_delete_from_db(new ContactDatabaseInterface::ContactIds);
   if (db_) {
-    for (size_t i = 0; i < updated_contacts->size(); ++i)
-      contacts_to_save_to_db->push_back((*updated_contacts)[i]);
+    for (size_t i = 0; i < updated_contacts->size(); ++i) {
+      Contact* contact = (*updated_contacts)[i];
+      if (contact->deleted())
+        contact_ids_to_delete_from_db->push_back(contact->contact_id());
+      else
+        contacts_to_save_to_db->push_back(contact);
+    }
   }
   bool got_updates = !updated_contacts->empty();
 
@@ -283,12 +307,17 @@ void GoogleContactStore::OnDownloadSuccess(
     // contacts, we still want to write updated metadata containing
     // |update_start_time|.
     VLOG(1) << "Saving " << contacts_to_save_to_db->size() << " contact(s) to "
-            << "database as " << (is_full_update ? "full" : "incremental")
-            << " update";
+            << "database and deleting " << contact_ids_to_delete_from_db->size()
+            << " as " << (is_full_update ? "full" : "incremental") << " update";
+
     scoped_ptr<UpdateMetadata> metadata(new UpdateMetadata);
     metadata->set_last_update_start_time(update_start_time.ToInternalValue());
+    metadata->set_last_contact_update_time(
+        last_contact_update_time_.ToInternalValue());
+
     db_->SaveContacts(
         contacts_to_save_to_db.Pass(),
+        contact_ids_to_delete_from_db.Pass(),
         metadata.Pass(),
         is_full_update,
         base::Bind(&GoogleContactStore::OnDatabaseContactsSaved,
@@ -335,6 +364,9 @@ void GoogleContactStore::OnDatabaseContactsLoaded(
     MergeContacts(true, contacts.Pass());
     last_successful_update_start_time_ =
         base::Time::FromInternalValue(metadata->last_update_start_time());
+    last_contact_update_time_ = std::max(
+        last_contact_update_time_,
+        base::Time::FromInternalValue(metadata->last_contact_update_time()));
 
     if (!contacts_.empty()) {
       FOR_EACH_OBSERVER(ContactStoreObserver,
diff --git a/chrome/browser/chromeos/contacts/google_contact_store.h b/chrome/browser/chromeos/contacts/google_contact_store.h
index 01bc4d4..31f4a04 100644
--- a/chrome/browser/chromeos/contacts/google_contact_store.h
+++ b/chrome/browser/chromeos/contacts/google_contact_store.h
@@ -45,7 +45,9 @@ class GoogleContactStore
     ~TestAPI();
 
     bool update_scheduled() { return store_->update_timer_.IsRunning(); }
-
+    base::Time last_contact_update_time() const {
+      return store_->last_contact_update_time_;
+    }
     void set_current_time(const base::Time& time) {
       store_->current_time_for_testing_ = time;
     }
@@ -63,6 +65,10 @@ class GoogleContactStore
     // Notifies the store that the system has gone online or offline.
     void NotifyAboutNetworkStateChange(bool online);
 
+    // Returns pointers to all of the contacts in the store's |contacts_|
+    // member.
+    scoped_ptr<ContactPointers> GetLoadedContacts();
+
    private:
     GoogleContactStore* store_;  // not owned
 
diff --git a/chrome/browser/chromeos/contacts/google_contact_store_unittest.cc b/chrome/browser/chromeos/contacts/google_contact_store_unittest.cc
index 38e9de5..156d399 100644
--- a/chrome/browser/chromeos/contacts/google_contact_store_unittest.cc
+++ b/chrome/browser/chromeos/contacts/google_contact_store_unittest.cc
@@ -414,5 +414,124 @@ TEST_F(GoogleContactStoreTest, AvoidUpdatesWhenOffline) {
   EXPECT_EQ(1, gdata_service_->num_download_requests());
 }
 
+TEST_F(GoogleContactStoreTest, DropDeletedContacts) {
+  // Tell the GData service to return a single contact.
+  scoped_ptr<Contact> contact1(new Contact);
+  InitContact("contact1", "1", false, contact1.get());
+  ContactPointers gdata_contacts;
+  gdata_contacts.push_back(contact1.get());
+  gdata_service_->SetContacts(gdata_contacts, base::Time());
+
+  // Check that the contact store loads it into memory and saves it to the
+  // database.
+  store_->Init();
+  EXPECT_EQ(0, gdata_service_->num_download_requests_with_wrong_timestamps());
+  EXPECT_EQ(base::Time::FromInternalValue(contact1->update_time()),
+            test_api_->last_contact_update_time());
+  EXPECT_EQ(VarContactsToString(1, contact1.get()),
+            ContactsToString(*test_api_->GetLoadedContacts()));
+  EXPECT_EQ(VarContactsToString(1, contact1.get()),
+            ContactMapToString(db_->contacts()));
+  EXPECT_EQ(contact1->update_time(),
+            db_->metadata().last_contact_update_time());
+  EXPECT_TRUE(test_api_->update_scheduled());
+
+  // Now tell the GData service to return a more-newly-updated, already deleted
+  // contact.
+  scoped_ptr<Contact> contact2(new Contact);
+  InitContact("contact2", "2", true, contact2.get());
+  contact2->set_update_time(
+      (base::Time::FromInternalValue(contact1->update_time()) +
+       base::TimeDelta::FromSeconds(5)).ToInternalValue());
+  gdata_contacts.clear();
+  gdata_contacts.push_back(contact2.get());
+  gdata_service_->SetContacts(
+      gdata_contacts,
+      base::Time::FromInternalValue(contact1->update_time()) +
+      base::TimeDelta::FromMilliseconds(1));
+
+  // The contact store should save the last update time from the deleted
+  // contact, but the contact itself shouldn't be loaded into memory or written
+  // to the database.
+  test_api_->DoUpdate();
+  EXPECT_EQ(0, gdata_service_->num_download_requests_with_wrong_timestamps());
+  EXPECT_EQ(base::Time::FromInternalValue(contact2->update_time()),
+            test_api_->last_contact_update_time());
+  EXPECT_EQ(VarContactsToString(1, contact1.get()),
+            ContactsToString(*test_api_->GetLoadedContacts()));
+  EXPECT_EQ(VarContactsToString(1, contact1.get()),
+            ContactMapToString(db_->contacts()));
+  EXPECT_EQ(contact2->update_time(),
+            db_->metadata().last_contact_update_time());
+
+  // Tell the GData service to report the first contact as having been deleted.
+  contact1->set_update_time(
+      (base::Time::FromInternalValue(contact2->update_time()) +
+       base::TimeDelta::FromSeconds(10)).ToInternalValue());
+  contact1->set_deleted(true);
+  gdata_contacts.clear();
+  gdata_contacts.push_back(contact1.get());
+  gdata_service_->SetContacts(
+      gdata_contacts,
+      base::Time::FromInternalValue(contact2->update_time()) +
+      base::TimeDelta::FromMilliseconds(1));
+
+  // The contact store should drop the first contact after another update.
+  test_api_->DoUpdate();
+  EXPECT_EQ(0, gdata_service_->num_download_requests_with_wrong_timestamps());
+  EXPECT_EQ(base::Time::FromInternalValue(contact1->update_time()),
+            test_api_->last_contact_update_time());
+  EXPECT_TRUE(test_api_->GetLoadedContacts()->empty());
+  EXPECT_TRUE(db_->contacts().empty());
+  EXPECT_EQ(contact1->update_time(),
+            db_->metadata().last_contact_update_time());
+}
+
+TEST_F(GoogleContactStoreTest, UseLastContactUpdateTimeFromMetadata) {
+  base::Time::Exploded kInitTimeExploded = { 2012, 3, 0, 1, 16, 34, 56, 123 };
+  base::Time kInitTime = base::Time::FromUTCExploded(kInitTimeExploded);
+
+  // Configure the metadata to say that a contact was updated one day before the
+  // current time.  We won't create a contact that actually contains this time,
+  // though; this mimics the situation where the most-recently-updated contact
+  // has been deleted and wasn't saved to the database.
+  base::Time kDeletedContactUpdateTime =
+      kInitTime - base::TimeDelta::FromDays(1);
+  UpdateMetadata db_metadata;
+  db_metadata.set_last_contact_update_time(
+      kDeletedContactUpdateTime.ToInternalValue());
+
+  // Create a non-deleted contact with an update time one day prior to the
+  // update time in the metadata.
+  base::Time kNonDeletedContactUpdateTime =
+      kDeletedContactUpdateTime - base::TimeDelta::FromDays(1);
+  scoped_ptr<Contact> non_deleted_contact(new Contact);
+  InitContact("contact", "1", false, non_deleted_contact.get());
+  non_deleted_contact->set_update_time(
+      kNonDeletedContactUpdateTime.ToInternalValue());
+
+  // Save the contact to the database.
+  ContactPointers db_contacts;
+  db_contacts.push_back(non_deleted_contact.get());
+  db_->SetContacts(db_contacts, db_metadata);
+
+  // Tell the GData service to expect the deleted contact's update time.
+  ContactPointers gdata_contacts;
+  gdata_contacts.push_back(non_deleted_contact.get());
+  gdata_service_->SetContacts(
+      gdata_contacts,
+      kDeletedContactUpdateTime + base::TimeDelta::FromMilliseconds(1));
+
+  test_api_->set_current_time(kInitTime);
+  store_->Init();
+  EXPECT_EQ(0, gdata_service_->num_download_requests_with_wrong_timestamps());
+  ContactPointers loaded_contacts;
+  store_->AppendContacts(&loaded_contacts);
+  EXPECT_EQ(ContactsToString(gdata_contacts),
+            ContactsToString(loaded_contacts));
+  EXPECT_TRUE(test_api_->update_scheduled());
+
+}
+
 }  // namespace test
 }  // namespace contacts
diff --git a/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.cc b/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.cc
index f435a6a..d9564aa 100644
--- a/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.cc
+++ b/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.cc
@@ -17,6 +17,7 @@ namespace gdata {
 
 GDataContactsServiceStub::GDataContactsServiceStub()
     : num_download_requests_(0),
+      num_download_requests_with_wrong_timestamps_(0),
       download_should_succeed_(true) {
 }
 
@@ -51,6 +52,7 @@ void GDataContactsServiceStub::DownloadContacts(
                << "differed from expected ("
                << util::FormatTimeAsString(expected_min_update_time_)
                << "); not returning any contacts";
+    num_download_requests_with_wrong_timestamps_++;
     failure_callback.Run();
     return;
   }
diff --git a/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.h b/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.h
index b57cc6c..91acf92 100644
--- a/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.h
+++ b/chrome/browser/chromeos/gdata/gdata_contacts_service_stub.h
@@ -25,7 +25,13 @@ class GDataContactsServiceStub : public GDataContactsServiceInterface {
   virtual ~GDataContactsServiceStub();
 
   int num_download_requests() const { return num_download_requests_; }
-  void reset_stats() { num_download_requests_ = 0; }
+  int num_download_requests_with_wrong_timestamps() const {
+    return num_download_requests_with_wrong_timestamps_;
+  }
+  void reset_stats() {
+    num_download_requests_ = 0;
+    num_download_requests_with_wrong_timestamps_ = 0;
+  }
   void set_download_should_succeed(bool succeed) {
     download_should_succeed_ = succeed;
   }
@@ -45,6 +51,10 @@ class GDataContactsServiceStub : public GDataContactsServiceInterface {
   // How many times has DownloadContacts() been called?
   int num_download_requests_;
 
+  // How many times has DownloadContacts() been called with a |min_update_time|
+  // parameter that doesn't match |expected_min_update_time_|?
+  int num_download_requests_with_wrong_timestamps_;
+
   // Should calls to DownloadContacts() succeed?
   bool download_should_succeed_;