contacts: Add contacts::ContactDatabase.

chrome/browser/chromeos/contacts/contact_database.cc

+// Copyright (c) 2012 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/chromeos/contacts/contact_database.h"
+
+#include <set>
+
+#include "base/file_util.h"
+#include "base/sequenced_task_runner.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "chrome/browser/chromeos/contacts/contact.pb.h"
+#include "content/public/browser/browser_thread.h"
+#include "leveldb/db.h"
+#include "leveldb/iterator.h"
+#include "leveldb/options.h"
+#include "leveldb/slice.h"
+#include "leveldb/status.h"
+#include "leveldb/write_batch.h"
+
+using content::BrowserThread;
+
+namespace contacts {
+
+ContactDatabase::ContactDatabase() {

[satorux1, 2012/07/31 07:26:30]

might want to add

DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

to ensure that the object is created on the expected thread.

[Daniel Erat, 2012/07/31 16:44:12]

Done.

+  base::SequencedWorkerPool* pool = BrowserThread::GetBlockingPool();
+  task_runner_ = pool->GetSequencedTaskRunner(pool->GetSequenceToken());
+}
+
+void ContactDatabase::DestroyOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  task_runner_->PostNonNestableTask(
+      FROM_HERE,
+      base::Bind(&ContactDatabase::DestroyFromTaskRunner,
+                 base::Unretained(this)));
+}
+
+void ContactDatabase::Init(const FilePath& database_dir,
+                           InitCallback callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  task_runner_->PostNonNestableTask(
+      FROM_HERE,
+      base::Bind(&ContactDatabase::InitFromTaskRunner,
+                 base::Unretained(this),
+                 database_dir, callback));
+}
+
+void ContactDatabase::SaveContacts(scoped_ptr<ContactPointers> contacts,
+                                   bool is_full_update,
+                                   SaveCallback callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  task_runner_->PostNonNestableTask(

[satorux1, 2012/07/31 16:19:19]

You might want to use PostTaskAndReply instead. That's an idiom to post a task
to another thread, and the process the result from the task

[Daniel Erat, 2012/07/31 16:44:12]

Interesting.  Setting the reply's parameters seems a bit tricky.  I suppose I'd
need to change LoadCallback to:

  base::Callback<void(scoped_refptr<bool> success,
                      scoped_refptr<ScopedVector<Contact> >)>

and then pass the scoped_refptrs to LoadContactsFromTaskRunner() so it can
update them, right?

[satorux1, 2012/07/31 17:34:26]

No scoped_refptr please. :) Usually, the result is passed from the 1st callback
to the 2nd callback like:

bool* success = new bool(false);
PostTaskAndReply(
  FROM_HERE,
  base::Bind(&DoSomehing, success);
  base::Bind(&ProcessResult, base::Owned(success)));

using base::Owned(), success is guaranteed to be deleted.

[Daniel Erat, 2012/07/31 19:34:42]

Nice!  Done.

+      FROM_HERE,
+      base::Bind(&ContactDatabase::SaveContactsFromTaskRunner,
+                 base::Unretained(this),
+                 base::Passed(contacts.Pass()), is_full_update, callback));
+}
+
+void ContactDatabase::LoadContacts(LoadCallback callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  task_runner_->PostNonNestableTask(
+      FROM_HERE,
+      base::Bind(&ContactDatabase::LoadContactsFromTaskRunner,
+                 base::Unretained(this),
+                 callback));
+}
+
+ContactDatabase::~ContactDatabase() {
+  DCHECK(IsRunByTaskRunner());
+}
+
+bool ContactDatabase::IsRunByTaskRunner() const {
+  return BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread();
+}
+
+void ContactDatabase::DestroyFromTaskRunner() {
+  DCHECK(IsRunByTaskRunner());
+  delete this;
+}
+
+void ContactDatabase::InitFromTaskRunner(const FilePath& database_dir,
+                                         InitCallback callback) {
+  DCHECK(IsRunByTaskRunner());
+  bool success = InitInternal(database_dir);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE, base::Bind(callback, success));
+}
+
+bool ContactDatabase::InitInternal(const FilePath& database_dir) {
+  DCHECK(IsRunByTaskRunner());
+  VLOG(1) << "Opening " << database_dir.value();
+
+  leveldb::Options options;
+  options.create_if_missing = true;
+  bool delete_and_retry_on_corruption = true;
+
+  while (true) {
+    leveldb::DB* db = NULL;
+    leveldb::Status status =
+        leveldb::DB::Open(options, database_dir.value(), &db);
+    if (status.ok()) {
+      CHECK(db);
+      db_.reset(db);
+      return true;
+    }
+
+    LOG(WARNING) << "Unable to open " << database_dir.value() << ": "
+                 << status.ToString();
+
+    // Delete the existing database and try again (just once, though).
+    if (status.IsCorruption() && delete_and_retry_on_corruption) {
+      LOG(WARNING) << "Deleting possibly-corrupt database";
+      file_util::Delete(database_dir, true);
+      delete_and_retry_on_corruption = false;
+    } else {
+      break;
+    }
+  }
+
+  return false;
+}
+
+void ContactDatabase::SaveContactsFromTaskRunner(
+    scoped_ptr<ContactPointers> contacts,
+    bool is_full_update,
+    SaveCallback callback) {
+  DCHECK(IsRunByTaskRunner());
+  VLOG(1) << "Saving " << contacts->size() << " contact(s) to database as "
+          << (is_full_update ? "full" : "partial") << " update";
+
+  // If we're doing a full update, find all of the existing keys first so we can
+  // delete ones that aren't present in the new set of contacts.
+  std::set<std::string> keys_to_delete;
+  if (is_full_update) {
+    leveldb::ReadOptions options;
+    scoped_ptr<leveldb::Iterator> db_iterator(db_->NewIterator(options));
+    db_iterator->SeekToFirst();
+    while (db_iterator->Valid()) {
+      keys_to_delete.insert(db_iterator->key().ToString());
+      db_iterator->Next();
+    }
+  }
+
+  // TODO(derat): Serializing all of the contacts and so we can write them in a
+  // single batch may be expensive, memory-wise.  Consider writing them in
+  // several batches instead.  (To avoid using partial writes in the event of a
+  // 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) {
+    const contacts::Contact& contact = **it;
+    updates.Put(leveldb::Slice(contact.provider_id()),
+                leveldb::Slice(contact.SerializeAsString()));
+    keys_to_delete.erase(contact.provider_id());
+  }
+
+  for (std::set<std::string>::const_iterator it = keys_to_delete.begin();
+       it != keys_to_delete.end(); ++it) {
+    updates.Delete(leveldb::Slice(*it));
+  }
+
+  leveldb::WriteOptions options;
+  options.sync = true;
+  leveldb::Status status = db_->Write(options, &updates);
+  if (!status.ok())
+    LOG(WARNING) << "Failed writing contacts: " << status.ToString();
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, status.ok()));

[satorux1, 2012/07/31 16:19:19]

This may be potentially dangerous. by the time |callback| is run on UI thread,
the ContactDatabase object may be already deleted on the blocking pool thread.
If |callback| uses ContactDatabase object, it'll cause a use-after-free.

To solve this problem, I think you can use PosTaskAndReply like:

PostTaskAndReply(
  FROM_HERE,
  base::Bind(&ContactDatabase::SaveContactsFromTaskRunner,
             base::Unretained(this), ...),
  base::Bind(&ContactDatabase::RunCallback,
             weak_ptr_factory_.GetWeakPtr(), callback));

Then, add the following to DestroyOnUIThread()

  weak_ptr_factory_.InvalidateWeakPtrs();

This way, the 2nd callback passed to PostTaskAndReply won't be run after the
point that the weak pointers are invalidated.

[Daniel Erat, 2012/07/31 16:44:12]

I'm not convinced of the risk -- if we initiate deletion of the ContactDatabase
from the UI thread, we shouldn't be trying to use it from the UI thread later. 
That said, I'm not opposed to what you're suggesting.  Were you envisioning
RunCallback() as just calling callback.Run()?  Note that a "success" parameter
needs to be passed to the callback in this case (although maybe that should be a
pre-bound scoped_refptr, as described above).

[satorux1, 2012/07/31 17:34:26]

Yes, RunCallback() just runs callback.Run(). "success" parameter can be passed
to  RunCallback() as a pointer as described above.


BTW, callback accessing a deleted object may not be a real risk here, but it's
not uncommon for the callback to access the object that ran the callback, like:

  some_reader_->Read(base::Bind(&OnRead, ...));

SomeClass::OnRead(...) {
  if (size == 0) {
    // All bytes read. Close the reader.
    some_reader_->Close();
  }
  ...
}

[Daniel Erat, 2012/07/31 19:34:42]

Makes sense; thanks for the explanation.  Done (with lots of calls to
base::Bind()).

+}
+
+void ContactDatabase::LoadContactsFromTaskRunner(
+    LoadCallback callback) {
+  DCHECK(IsRunByTaskRunner());
+  scoped_ptr<ScopedVector<Contact> > contacts(new ScopedVector<Contact>());
+  bool success = LoadContactsInternal(contacts.get());
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(callback, success, base::Passed(contacts.Pass())));
+}
+
+bool ContactDatabase::LoadContactsInternal(ScopedVector<Contact>* contacts) {
+  DCHECK(IsRunByTaskRunner());
+  DCHECK(contacts);
+  contacts->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 false;
+    }
+    contacts->push_back(contact.release());
+    db_iterator->Next();
+  }
+  return true;
+}
+
+}  // namespace contacts