Implement an initial extension settings API.

chrome/browser/extensions/extension_settings_leveldb_storage.cc

+// Copyright (c) 2011 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/extensions/extension_settings_leveldb_storage.h"
+
+#include "base/bind.h"
+#include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
+#include "base/logging.h"
+#include "content/browser/browser_thread.h"
+#include "third_party/leveldb/include/leveldb/iterator.h"
+#include "third_party/leveldb/include/leveldb/write_batch.h"
+
+namespace {
+
+// General closure type for computing a value on the FILE thread and calling
+// back on the UI thread.  The *OnFileThread methods should run on the file
+// thread, all others should run on the UI thread.
+//
+// Subclasses should implement RunOnFileThreadImpl(), manipulating settings_
+// or error_, then call either SucceedOnFileThread() or FailOnFileThread().
+class ResultClosure {
+ public:
+  ResultClosure(leveldb::DB* db, ExtensionSettingsStorage::Callback* callback)
+      : db_(db), settings_(new DictionaryValue()), callback_(callback) {}
+
+  ~ResultClosure() {}
+
+  void Run() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(&ResultClosure::RunOnFileThread, base::Unretained(this)));
+  }
+
+ protected:
+  virtual void RunOnFileThreadImpl() = 0;
+
+  void SucceedOnFileThread() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&ResultClosure::Succeed, base::Unretained(this)));
+  }
+
+  void FailOnFileThread(std::string error) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    error_ = error;
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&ResultClosure::Fail, base::Unretained(this)));
+  }
+
+  // Helper for the Get() methods; reads a single key from the database using
+  // an existing leveldb options object.
+  // Returns whether the read was successful, in which case the given settings
+  // will have the value set.  Otherwise, error will be set.
+  bool ReadFromDb(leveldb::ReadOptions options, const std::string& key,
+      DictionaryValue* settings, std::string* error) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    std::string value_as_json;
+    leveldb::Status s = db_->Get(options, key, &value_as_json);
+    if (s.ok()) {
+      base::JSONReader json_reader;
+      Value* value = json_reader.JsonToValue(value_as_json, false, false);
+      if (value != NULL) {
+        settings->Set(key, value);
+        return true;
+      } else {
+        // TODO(kalman): clear the offending non-JSON value from the database.
+        LOG(ERROR) << "Invalid JSON: " << value_as_json;
+        *error = "Invalid JSON";
+        return false;
+      }
+    } else if (s.IsNotFound()) {
+      // Despite there being no value, it was still a success.
+      return true;
+    } else {
+      LOG(ERROR) << "Error reading from database: " << s.ToString();
+      *error = s.ToString();
+      return false;
+    }
+  }
+
+  leveldb::DB* db_;
+  DictionaryValue* settings_;
+  std::string error_;
+
+ private:
+  void RunOnFileThread() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    RunOnFileThreadImpl();
+  }
+
+  void Succeed() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    callback_->OnSuccess(settings_);
+    delete this;
+  }
+
+  void Fail() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    delete settings_;
+    callback_->OnFailure(error_);
+    delete this;
+  }
+
+  scoped_ptr<ExtensionSettingsStorage::Callback> callback_;
+};
+
+// Result closure for Get(key).
+class Get1ResultClosure : public ResultClosure {
+ public:
+  Get1ResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback, const std::string& key)
+      : ResultClosure(db, callback), key_(key) {}
+
+ protected:
+  void RunOnFileThreadImpl() {
+    if (ReadFromDb(leveldb::ReadOptions(), key_, settings_, &error_)) {
+      SucceedOnFileThread();
+    } else {
+      FailOnFileThread("Failed");
+    }
+  }
+
+ private:
+  std::string key_;
+};
+
+// Result closure for Get(keys).
+class Get2ResultClosure : public ResultClosure {
+ public:
+  // Ownership of keys is taken.
+  Get2ResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback, ListValue* keys)
+      : ResultClosure(db, callback), keys_(keys) {}
+
+ protected:
+  void RunOnFileThreadImpl() {
+    leveldb::ReadOptions options;
+    options.snapshot = db_->GetSnapshot();

[dgrogan, 2011/06/18 01:54:40]

Why do you use snapshots when you're reading?  Won't all the reads and writes
happen on the same thread?

[not at google - send to devlin, 2011/06/20 05:33:11]

Hm.  Yes.  It just seems... instinctively wrong to read from the db without
using snapshotting, even though it's all from the same thread.

E.g. what if the leveldb instance is somehow modified outside the knowledge of
this class?  Maybe even... two Chrome processes are running somehow, and the
same extension in each is reading while the other is writing?  I don't even know
if that situation can arise, and if it did... there might be other problems too.
 But at least there would be one less :-)

Happy to change this to not use a snapshot, though, if there's a reason not to.

[dgrogan, 2011/06/21 02:36:05]

Ok, they are just defensive.  Using them is fine, just wanted to make sure I
understood was going on.
That situation shouldn't arise as far as leveldb is concerned, I think it'll
throw an error if opened by more than one process at a time.  But I know what
you mean, "one less" is good.

[not at google - send to devlin, 2011/06/22 09:40:38]

Cool, I added a comment.

+    bool success = true;
+    std::string key;
+
+    for (ListValue::const_iterator it = keys_->begin();
+        success && it != keys_->end(); ++it) {
+      if ((*it)->GetAsString(&key)) {
+        if (!ReadFromDb(options, key, settings_, &error_)) {
+          success = false;
+        }
+      }
+    }
+    db_->ReleaseSnapshot(options.snapshot);
+
+    if (success) {
+      SucceedOnFileThread();
+    } else {
+      FailOnFileThread("Failed");
+    }
+  }
+
+ private:
+  scoped_ptr<ListValue> keys_;
+};
+
+// Result closure for Get() .
+class Get3ResultClosure : public ResultClosure {
+ public:
+  Get3ResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback)
+      : ResultClosure(db, callback) {
+  }
+
+ protected:
+  void RunOnFileThreadImpl() {
+    base::JSONReader json_reader;
+    leveldb::ReadOptions options = leveldb::ReadOptions();
+    options.snapshot = db_->GetSnapshot();
+    scoped_ptr<leveldb::Iterator> it(db_->NewIterator(options));
+
+    for (it->SeekToFirst(); it->Valid(); it->Next()) {
+      Value* value =
+          json_reader.JsonToValue(it->value().ToString(), false, false);
+      if (value != NULL) {
+        settings_->Set(it->key().ToString(), value);
+      } else {
+        // TODO(kalman): clear the offending non-JSON value from the database.
+        LOG(ERROR) << "Invalid JSON: " << it->value().ToString();
+      }
+    }
+    db_->ReleaseSnapshot(options.snapshot);
+
+    if (!it->status().ok()) {
+      LOG(ERROR) << "DB iteration failed: " << it->status().ToString();
+      FailOnFileThread("Failed");

[dgrogan, 2011/06/18 01:54:40]

Why don't you pass it->status().ToString() back through to the callback instead
of "Failed"?

[not at google - send to devlin, 2011/06/20 05:33:11]

This is the message sent back to the extension.  I left it out since it seems
like... some kind of attack that an extension might be able to do to gather
information about the browser.  I.e. not sure what kind of messages you get in
here.

Happy to change it if you think it's safe.

[dgrogan, 2011/06/21 02:36:05]

Ok, I see where you're coming from.
No idea if it is or not.  Hopefully Mihai will know.

[not at google - send to devlin, 2011/06/22 09:40:38]

Btw I had a look through the leveldb code but wasn't able to conclude a whole
lot.

[Mihai Parparita -not on Chrome, 2011/06/23 03:16:46]

Sorry, didn't see this question earlier. Unless the extension author can
actually do something about (even while developing/debugging), I don't think we
need additional information in these error messages (though they should probably
reference "extension settings" instead of just "failed" and use a constant).

[not at google - send to devlin, 2011/06/23 13:44:26]

Done.

+    } else {
+      SucceedOnFileThread();
+    }
+  }
+};
+
+// Result closure for Set(key, value).
+class Set1ResultClosure : public ResultClosure {
+ public:
+  // Ownership of the value is taken.
+  Set1ResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback, const std::string& key,
+      Value* value) : ResultClosure(db, callback), key_(key), value_(value) {}
+
+ protected:
+  void RunOnFileThreadImpl() {
+    std::string value_as_json;
+    base::JSONWriter::Write(value_, false, &value_as_json);
+    leveldb::Status status =
+        db_->Put(leveldb::WriteOptions(), key_, value_as_json);
+    if (status.ok()) {
+      settings_->Set(key_, value_);
+      SucceedOnFileThread();
+    } else {
+      LOG(WARNING) << "DB write failed: " << status.ToString();
+      delete value_;
+      FailOnFileThread("Failed");
+    }
+  }
+
+ private:
+  std::string key_;
+  Value* value_;
+};
+
+// Result callback for Set(values).
+class Set2ResultClosure : public ResultClosure {
+ public:
+  // Ownership of values is taken.
+  Set2ResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback, DictionaryValue* values)
+      : ResultClosure(db, callback), values_(values) {
+  }
+
+ protected:
+  void RunOnFileThreadImpl() {
+    // Gather keys since a dictionary can't be modified during iteration.
+    std::vector<std::string> keys;
+    for (DictionaryValue::key_iterator it = values_->begin_keys();
+        it != values_->end_keys(); ++it) {
+      keys.push_back(*it);
+    }
+    // Write values while transferring ownership from values_ to settings_.
+    std::string value_as_json;
+    leveldb::WriteBatch batch;
+    for (unsigned i = 0; i < keys.size(); ++i) {
+      Value* value = NULL;
+      values_->RemoveWithoutPathExpansion(keys[i], &value);
+      base::JSONWriter::Write(value, false, &value_as_json);
+      batch.Put(keys[i], value_as_json);
+      settings_->SetWithoutPathExpansion(keys[i], value);
+    }
+    DCHECK(values_->empty());
+
+    leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch);
+    if (status.ok()) {
+      SucceedOnFileThread();
+    } else {
+      LOG(WARNING) << "DB batch write failed: " << status.ToString();
+      FailOnFileThread("Failed");
+    }
+  }
+
+ private:
+  scoped_ptr<DictionaryValue> values_; // will be empty on destruction
+};
+
+// Result closure for Remove(key).
+class Remove1ResultClosure : public ResultClosure {
+ public:
+  Remove1ResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback, const std::string& key)
+      : ResultClosure(db, callback), key_(key) {
+  }
+
+ protected:
+  void RunOnFileThreadImpl() {
+    leveldb::Status status = db_->Delete(leveldb::WriteOptions(), key_);
+    if (status.ok() || status.IsNotFound()) {
+      SucceedOnFileThread();
+    } else {
+      LOG(WARNING) << "DB delete failed: " << status.ToString();
+      FailOnFileThread("Failed");
+    }
+  }
+
+ private:
+  std::string key_;
+};
+
+// Result closure for Remove(keys).
+class Remove2ResultClosure : public ResultClosure {
+ public:
+  // Ownership of keys is taken.
+  Remove2ResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback, ListValue* keys)
+      : ResultClosure(db, callback), keys_(keys) {
+  }
+
+ protected:
+  void RunOnFileThreadImpl() {
+    std::string key;
+    leveldb::WriteBatch batch;
+    for (ListValue::const_iterator it = keys_->begin();
+        it != keys_->end(); ++it) {
+      if ((*it)->GetAsString(&key)) {
+        batch.Delete(key);
+      }
+    }
+
+    leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch);
+    if (status.ok() || status.IsNotFound()) {
+      SucceedOnFileThread();
+    } else {
+      LOG(WARNING) << "DB batch delete failed: " << status.ToString();
+      FailOnFileThread("Failed");
+    }
+  }
+
+ private:
+  scoped_ptr<ListValue> keys_;
+};
+
+// Result closure for Clear().
+class ClearResultClosure : public ResultClosure {
+ public:
+  ClearResultClosure(leveldb::DB* db,
+      ExtensionSettingsStorage::Callback* callback)
+      : ResultClosure(db, callback) {
+  }
+
+ protected:
+  void RunOnFileThreadImpl() {
+    leveldb::ReadOptions options;
+    options.snapshot = db_->GetSnapshot();
+    scoped_ptr<leveldb::Iterator> it(db_->NewIterator(options));
+    leveldb::WriteBatch batch;
+
+    for (it->SeekToFirst(); it->Valid(); it->Next()) {
+      batch.Delete(it->key());

[dgrogan, 2011/06/18 01:54:40]

Does batching up a bunch of deletes on an iterator over a snapshot work?  Do the
real non-snapshot values get deleted?

It must work because you have a unit test for it, it just seems weird.

[not at google - send to devlin, 2011/06/20 05:33:11]

Maybe I'm misunderstanding something.  The snapshotting is to get a consistent
state of all the keys in the db at the time of clearing... so their values don't
come in to it.

And the batch object that the deletes are getting applied to isn't linked with
the database until after the snapshot is released, so that should be fine.

[dgrogan, 2011/06/21 02:36:05]

Oh, I didn't realize it gets a consistent set of keys only.
This all seems fine; I hadn't used snapshots before and didn't know how they
worked.

[not at google - send to devlin, 2011/06/22 09:40:38]

I think it gets a consistent view of both keys and values?  I was just saying
that this code only cares about the keys.

+    }
+    db_->ReleaseSnapshot(options.snapshot);
+
+    if (it->status().ok()) {
+      leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch);
+      if (status.ok() || status.IsNotFound()) {
+        SucceedOnFileThread();
+      } else {
+        LOG(WARNING) << "Clear failed: " << status.ToString();
+        FailOnFileThread("Failed");
+      }
+    } else {
+      LOG(WARNING) << "Clear iteration failed: " << it->status().ToString();
+      FailOnFileThread("Failed");
+    }
+  }
+};
+
+}  // namespace
+
+ExtensionSettingsLeveldbStorage::ExtensionSettingsLeveldbStorage(
+    leveldb::DB* db) : db_(db) {
+}
+
+void ExtensionSettingsLeveldbStorage::Get(const std::string& key,
+    ExtensionSettingsStorage::Callback* callback) {
+  (new Get1ResultClosure(db_.get(), callback, key))->Run();
+}
+
+void ExtensionSettingsLeveldbStorage::Get(const ListValue& keys,
+    ExtensionSettingsStorage::Callback* callback) {
+  (new Get2ResultClosure(db_.get(), callback, keys.DeepCopy()))->Run();
+}
+
+void ExtensionSettingsLeveldbStorage::Get(
+    ExtensionSettingsStorage::Callback* callback) {
+  (new Get3ResultClosure(db_.get(), callback))->Run();
+}
+
+void ExtensionSettingsLeveldbStorage::Set(const std::string& key,
+    const Value& value, ExtensionSettingsStorage::Callback* callback) {
+  (new Set1ResultClosure(db_.get(), callback, key, value.DeepCopy()))->Run();
+}
+
+void ExtensionSettingsLeveldbStorage::Set(const DictionaryValue& values,
+    ExtensionSettingsStorage::Callback* callback) {
+  (new Set2ResultClosure(db_.get(), callback, values.DeepCopy()))->Run();
+}
+
+void ExtensionSettingsLeveldbStorage::Remove(const std::string& key,
+    ExtensionSettingsStorage::Callback *callback) {
+  (new Remove1ResultClosure(db_.get(), callback, key))->Run();
+}
+
+void ExtensionSettingsLeveldbStorage::Remove(const ListValue& keys,
+    ExtensionSettingsStorage::Callback *callback) {
+  (new Remove2ResultClosure(db_.get(), callback, keys.DeepCopy()))->Run();
+}
+
+void ExtensionSettingsLeveldbStorage::Clear(
+    ExtensionSettingsStorage::Callback* callback) {
+  (new ClearResultClosure(db_.get(), callback))->Run();
+}