contacts: Add UpdateMetadata protocol buffer.

chrome/browser/chromeos/contacts/contact_database.cc

Index: chrome/browser/chromeos/contacts/contact_database.cc
diff --git a/chrome/browser/chromeos/contacts/contact_database.cc b/chrome/browser/chromeos/contacts/contact_database.cc
index c5b4c62d80068d1c27e78c998d0a5acfa364ff34..485440ad76ba355429e0acdeda6f80af2590f535 100644
--- a/chrome/browser/chromeos/contacts/contact_database.cc
+++ b/chrome/browser/chromeos/contacts/contact_database.cc
@@ -22,6 +22,13 @@ using content::BrowserThread;
 
 namespace contacts {
 
+namespace {
+
+// LevelDB key used for storing UpdateMetadata messages.
+const char kUpdateMetadataKey[] = "__chrome_update_metadata__";
+
+}  // namespace
+
 ContactDatabase::ContactDatabase() : weak_ptr_factory_(this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   base::SequencedWorkerPool* pool = BrowserThread::GetBlockingPool();
@@ -54,6 +61,7 @@ void ContactDatabase::Init(const FilePath& database_dir,
 }
 
 void ContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts,
+                                   scoped_ptr<UpdateMetadata> metadata,
                                    bool is_full_update,
                                    SaveCallback callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -63,6 +71,7 @@ void ContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts,
       base::Bind(&ContactDatabase::SaveContactsFromTaskRunner,
                  base::Unretained(this),
                  base::Passed(contacts.Pass()),
+                 base::Passed(metadata.Pass()),
                  is_full_update,
                  success),
       base::Bind(&ContactDatabase::RunSaveCallback,
@@ -73,20 +82,26 @@ void ContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts,
 
 void ContactDatabase::LoadContacts(LoadCallback callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   bool* success = new bool(false);
   scoped_ptr<ScopedVector<Contact> > contacts(new ScopedVector<Contact>);
   ScopedVector<Contact>* contacts_ptr = contacts.get();
+  scoped_ptr<UpdateMetadata> metadata(new UpdateMetadata);
+  UpdateMetadata* metadata_ptr = metadata.get();
+
   task_runner_->PostTaskAndReply(
       FROM_HERE,
       base::Bind(&ContactDatabase::LoadContactsFromTaskRunner,
                  base::Unretained(this),
                  success,
-                 contacts_ptr),
+                 contacts_ptr,

[satorux1, 2012/08/01 20:12:32]

were you bitten by doing  contacts.get() here? This is subtle...

for objects used for PostTaskAndReply, raw pointers with base::Owned() may be a
better approach. just my 2c.

[Daniel Erat, 2012/08/01 20:22:02]

Yeah, I was. :-(

I think I still prefer scoped_ptr, since it makes it clear to the caller that
they're taking ownership of the contacts.

I'll add a comment, if that's okay.

+                 metadata_ptr),
       base::Bind(&ContactDatabase::RunLoadCallback,
                  weak_ptr_factory_.GetWeakPtr(),
                  callback,
                  base::Owned(success),
-                 base::Passed(contacts.Pass())));
+                 base::Passed(contacts.Pass()),
+                 base::Passed(metadata.Pass())));
 }
 
 ContactDatabase::~ContactDatabase() {
@@ -117,9 +132,10 @@ void ContactDatabase::RunSaveCallback(SaveCallback callback,
 void ContactDatabase::RunLoadCallback(
     LoadCallback callback,
     const bool* success,
-    scoped_ptr<ScopedVector<Contact> > contacts) {
+    scoped_ptr<ScopedVector<Contact> > contacts,
+    scoped_ptr<UpdateMetadata> metadata) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  callback.Run(*success, contacts.Pass());
+  callback.Run(*success, contacts.Pass(), metadata.Pass());
 }
 
 void ContactDatabase::InitFromTaskRunner(const FilePath& database_dir,
@@ -161,6 +177,7 @@ void ContactDatabase::InitFromTaskRunner(const FilePath& database_dir,
 
 void ContactDatabase::SaveContactsFromTaskRunner(
     scoped_ptr<ContactPointers> contacts,
+    scoped_ptr<UpdateMetadata> metadata,
     bool is_full_update,
     bool* success) {
   DCHECK(IsRunByTaskRunner());
@@ -178,7 +195,9 @@ void ContactDatabase::SaveContactsFromTaskRunner(
     scoped_ptr<leveldb::Iterator> db_iterator(db_->NewIterator(options));
     db_iterator->SeekToFirst();
     while (db_iterator->Valid()) {
-      keys_to_delete.insert(db_iterator->key().ToString());
+      std::string key = db_iterator->key().ToString();
+      if (key != kUpdateMetadataKey)

[satorux1, 2012/08/01 20:12:32]

do you plan to add more special keys? If so, checking by a prefix may be better
like StartsWith(key, kSpecialKeyPrefix)

Another approach would be to use two separate prefixes: 1) for provider ids, 2)
for metadata

However, the 2nd approach breaks the compatibility with the exsting database
file. not sure if it matters, though.

[Daniel Erat, 2012/08/01 20:22:02]

I don't have any plans to add more, but if it happens, using "__chrome_" as a
prefix seems okay to me.

+        keys_to_delete.insert(key);
       db_iterator->Next();
     }
   }

[satorux1, 2012/08/01 20:37:06]

BTW, is there any better way to clear the table than removing every entry? would
be nice if we have something like db_->DeleteAll()

@@ -192,9 +211,15 @@ void ContactDatabase::SaveContactsFromTaskRunner(
   for (ContactPointers::const_iterator it = contacts->begin();
        it != contacts->end(); ++it) {
     const contacts::Contact& contact = **it;
+    if (contact.provider_id() == kUpdateMetadataKey) {

[satorux1, 2012/08/01 20:12:32]

this may be a bit dirty? if we are to use two separate prefixes, we don't need
this.

[Daniel Erat, 2012/08/01 20:22:02]

I think it might be overkill, but I'll change it if you feel strongly. :-)

[satorux1, 2012/08/01 20:37:06]

No, I don't feel strongly

+      LOG(WARNING) << "Skipping contact with reserved ID "
+                   << contact.provider_id();
+      continue;
+    }
     updates.Put(leveldb::Slice(contact.provider_id()),
                 leveldb::Slice(contact.SerializeAsString()));
-    keys_to_delete.erase(contact.provider_id());
+    if (is_full_update)
+      keys_to_delete.erase(contact.provider_id());
   }
 
   for (std::set<std::string>::const_iterator it = keys_to_delete.begin();
@@ -202,6 +227,9 @@ void ContactDatabase::SaveContactsFromTaskRunner(
     updates.Delete(leveldb::Slice(*it));
   }
 
+  updates.Put(leveldb::Slice(kUpdateMetadataKey),
+              leveldb::Slice(metadata->SerializeAsString()));
+
   leveldb::WriteOptions options;
   options.sync = true;
   leveldb::Status status = db_->Write(options, &updates);
@@ -213,26 +241,37 @@ void ContactDatabase::SaveContactsFromTaskRunner(
 
 void ContactDatabase::LoadContactsFromTaskRunner(
     bool* success,
-    ScopedVector<Contact>* contacts_out) {
+    ScopedVector<Contact>* contacts,
+    UpdateMetadata* metadata) {
   DCHECK(IsRunByTaskRunner());
   DCHECK(success);
-  DCHECK(contacts_out);
+  DCHECK(contacts);
+  DCHECK(metadata);
 
   *success = false;
-  contacts_out->clear();
+  contacts->clear();
+  metadata->Clear();
 
   leveldb::ReadOptions options;
   scoped_ptr<leveldb::Iterator> db_iterator(db_->NewIterator(options));
   db_iterator->SeekToFirst();
   while (db_iterator->Valid()) {
-    scoped_ptr<Contact> contact(new Contact);
     leveldb::Slice value_slice = db_iterator->value();
-    if (!contact->ParseFromArray(value_slice.data(), value_slice.size())) {
-      LOG(WARNING) << "Unable to parse contact "
-                   << db_iterator->key().ToString();
-      return;
+
+    if (db_iterator->key().ToString() == kUpdateMetadataKey) {
+      if (!metadata->ParseFromArray(value_slice.data(), value_slice.size())) {
+        LOG(WARNING) << "Unable to parse metadata";
+        return;
+      }
+    } else {
+      scoped_ptr<Contact> contact(new Contact);
+      if (!contact->ParseFromArray(value_slice.data(), value_slice.size())) {
+        LOG(WARNING) << "Unable to parse contact "
+                     << db_iterator->key().ToString();
+        return;
+      }
+      contacts->push_back(contact.release());
     }
-    contacts_out->push_back(contact.release());
     db_iterator->Next();
   }