Index: chrome/browser/value_store/leveldb_value_store.cc |
diff --git a/chrome/browser/value_store/leveldb_value_store.cc b/chrome/browser/value_store/leveldb_value_store.cc |
index 1a2dbe6cd55bafe7f26363e2584f6ec240b4a5e2..8e7e37bd453cbbadeee06c3c14fbe9a86f0aa1a5 100644 |
--- a/chrome/browser/value_store/leveldb_value_store.cc |
+++ b/chrome/browser/value_store/leveldb_value_store.cc |
@@ -11,47 +11,24 @@ |
#include "base/strings/string_util.h" |
#include "base/strings/stringprintf.h" |
#include "base/strings/sys_string_conversions.h" |
+#include "chrome/browser/value_store/value_store_util.h" |
#include "content/public/browser/browser_thread.h" |
#include "third_party/leveldatabase/src/include/leveldb/iterator.h" |
#include "third_party/leveldatabase/src/include/leveldb/write_batch.h" |
+namespace util = value_store_util; |
using content::BrowserThread; |
namespace { |
-const char* kInvalidJson = "Invalid JSON"; |
+const char kInvalidJson[] = "Invalid JSON"; |
-ValueStore::ReadResult ReadFailure(const std::string& action, |
- const std::string& reason) { |
- CHECK_NE("", reason); |
- return ValueStore::MakeReadResult(base::StringPrintf( |
- "Failure to %s: %s", action.c_str(), reason.c_str())); |
-} |
- |
-ValueStore::ReadResult ReadFailureForKey(const std::string& action, |
- const std::string& key, |
- const std::string& reason) { |
- CHECK_NE("", reason); |
- return ValueStore::MakeReadResult(base::StringPrintf( |
- "Failure to %s for key %s: %s", |
- action.c_str(), key.c_str(), reason.c_str())); |
-} |
- |
-ValueStore::WriteResult WriteFailure(const std::string& action, |
- const std::string& reason) { |
- CHECK_NE("", reason); |
- return ValueStore::MakeWriteResult(base::StringPrintf( |
- "Failure to %s: %s", action.c_str(), reason.c_str())); |
-} |
- |
-ValueStore::WriteResult WriteFailureForKey(const std::string& action, |
- const std::string& key, |
- const std::string& reason) { |
- CHECK_NE("", reason); |
- return ValueStore::MakeWriteResult(base::StringPrintf( |
- "Failure to %s for key %s: %s", |
- action.c_str(), key.c_str(), reason.c_str())); |
-} |
+// Hack for converting a base::FilePath to a std::string for use by leveldb. |
Matt Perry
2013/09/06 20:07:30
FilePath already has an AsUTF8Unsafe
not at google - send to devlin
2013/09/06 21:44:28
Done.
|
+#if defined(OS_POSIX) |
+#define OS_PATH(file_path) ((file_path).value()) |
+#elif defined(OS_WIN) |
+#define OS_PATH(file_path) base::SysWideToUTF8((file_path).value()) |
+#endif |
// Scoped leveldb snapshot which releases the snapshot on destruction. |
class ScopedSnapshot { |
@@ -80,9 +57,9 @@ LeveldbValueStore::LeveldbValueStore(const base::FilePath& db_path) |
: db_path_(db_path) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- LOG(WARNING) << error; |
+ scoped_ptr<Error> open_error = EnsureDbIsOpen(); |
+ if (open_error) |
+ LOG(WARNING) << open_error->message; |
} |
LeveldbValueStore::~LeveldbValueStore() { |
@@ -91,14 +68,8 @@ LeveldbValueStore::~LeveldbValueStore() { |
// Delete the database from disk if it's empty (but only if we managed to |
// open it!). This is safe on destruction, assuming that we have exclusive |
// access to the database. |
- if (db_ && IsEmpty()) { |
- // Close |db_| now to release any lock on the directory. |
- db_.reset(); |
- if (!base::DeleteFile(db_path_, true)) { |
- LOG(WARNING) << "Failed to delete LeveldbValueStore database " << |
- db_path_.value(); |
- } |
- } |
+ if (db_ && IsEmpty()) |
+ DeleteDbFile(); |
} |
size_t LeveldbValueStore::GetBytesInUse(const std::string& key) { |
@@ -123,28 +94,28 @@ size_t LeveldbValueStore::GetBytesInUse() { |
ValueStore::ReadResult LeveldbValueStore::Get(const std::string& key) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- return ValueStore::MakeReadResult(error); |
+ scoped_ptr<Error> open_error = EnsureDbIsOpen(); |
+ if (open_error) |
+ return MakeReadResult(open_error.Pass()); |
scoped_ptr<Value> setting; |
- error = ReadFromDb(leveldb::ReadOptions(), key, &setting); |
- if (!error.empty()) |
- return ReadFailureForKey("get", key, error); |
+ scoped_ptr<Error> error = ReadFromDb(leveldb::ReadOptions(), key, &setting); |
+ if (error) |
+ return MakeReadResult(error.Pass()); |
DictionaryValue* settings = new DictionaryValue(); |
- if (setting.get()) |
+ if (setting) |
settings->SetWithoutPathExpansion(key, setting.release()); |
- return MakeReadResult(settings); |
+ return MakeReadResult(make_scoped_ptr(settings)); |
} |
ValueStore::ReadResult LeveldbValueStore::Get( |
const std::vector<std::string>& keys) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- return ValueStore::MakeReadResult(error); |
+ scoped_ptr<Error> open_error = EnsureDbIsOpen(); |
+ if (open_error) |
+ return MakeReadResult(open_error.Pass()); |
leveldb::ReadOptions options; |
scoped_ptr<DictionaryValue> settings(new DictionaryValue()); |
@@ -156,22 +127,22 @@ ValueStore::ReadResult LeveldbValueStore::Get( |
for (std::vector<std::string>::const_iterator it = keys.begin(); |
it != keys.end(); ++it) { |
scoped_ptr<Value> setting; |
- error = ReadFromDb(options, *it, &setting); |
- if (!error.empty()) |
- return ReadFailureForKey("get multiple items", *it, error); |
- if (setting.get()) |
+ scoped_ptr<Error> error = ReadFromDb(options, *it, &setting); |
+ if (error) |
+ return MakeReadResult(error.Pass()); |
+ if (setting) |
settings->SetWithoutPathExpansion(*it, setting.release()); |
} |
- return MakeReadResult(settings.release()); |
+ return MakeReadResult(settings.Pass()); |
} |
ValueStore::ReadResult LeveldbValueStore::Get() { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- return ValueStore::MakeReadResult(error); |
+ scoped_ptr<Error> open_error = EnsureDbIsOpen(); |
+ if (open_error) |
+ return MakeReadResult(open_error.Pass()); |
base::JSONReader json_reader; |
leveldb::ReadOptions options = leveldb::ReadOptions(); |
@@ -185,68 +156,65 @@ ValueStore::ReadResult LeveldbValueStore::Get() { |
for (it->SeekToFirst(); it->Valid(); it->Next()) { |
std::string key = it->key().ToString(); |
Value* value = json_reader.ReadToValue(it->value().ToString()); |
- if (value == NULL) { |
- // TODO(kalman): clear the offending non-JSON value from the database. |
- return ReadFailureForKey("get all", key, kInvalidJson); |
+ if (!value) { |
+ return MakeReadResult( |
+ Error::Create(CORRUPTION, kInvalidJson, util::NewKey(key))); |
} |
settings->SetWithoutPathExpansion(key, value); |
} |
if (it->status().IsNotFound()) { |
NOTREACHED() << "IsNotFound() but iterating over all keys?!"; |
- return MakeReadResult(settings.release()); |
+ return MakeReadResult(settings.Pass()); |
} |
if (!it->status().ok()) |
- return ReadFailure("get all items", it->status().ToString()); |
+ return MakeReadResult(ToValueStoreError(it->status(), util::NoKey())); |
- return MakeReadResult(settings.release()); |
+ return MakeReadResult(settings.Pass()); |
} |
ValueStore::WriteResult LeveldbValueStore::Set( |
WriteOptions options, const std::string& key, const Value& value) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- return ValueStore::MakeWriteResult(error); |
+ scoped_ptr<Error> open_error = EnsureDbIsOpen(); |
+ if (open_error) |
+ return MakeWriteResult(open_error.Pass()); |
leveldb::WriteBatch batch; |
scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList()); |
- error = AddToBatch(options, key, value, &batch, changes.get()); |
- if (!error.empty()) |
- return WriteFailureForKey("find changes to set", key, error); |
- |
- error = WriteToDb(&batch); |
- if (!error.empty()) |
- return WriteFailureForKey("set", key, error); |
- return MakeWriteResult(changes.release()); |
+ scoped_ptr<Error> batch_error = |
+ AddToBatch(options, key, value, &batch, changes.get()); |
+ if (batch_error) |
+ return MakeWriteResult(batch_error.Pass()); |
+ |
+ scoped_ptr<Error> write_error = WriteToDb(&batch); |
+ return write_error ? MakeWriteResult(write_error.Pass()) |
+ : MakeWriteResult(changes.Pass()); |
} |
ValueStore::WriteResult LeveldbValueStore::Set( |
WriteOptions options, const DictionaryValue& settings) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- return ValueStore::MakeWriteResult(error); |
+ scoped_ptr<Error> open_error = EnsureDbIsOpen(); |
+ if (open_error) |
+ return MakeWriteResult(open_error.Pass()); |
leveldb::WriteBatch batch; |
scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList()); |
for (DictionaryValue::Iterator it(settings); !it.IsAtEnd(); it.Advance()) { |
- error = AddToBatch(options, it.key(), it.value(), &batch, changes.get()); |
- if (!error.empty()) { |
- return WriteFailureForKey("find changes to set multiple items", |
- it.key(), |
- error); |
- } |
+ scoped_ptr<Error> batch_error = |
+ AddToBatch(options, it.key(), it.value(), &batch, changes.get()); |
+ if (batch_error) |
+ return MakeWriteResult(batch_error.Pass()); |
} |
- error = WriteToDb(&batch); |
- if (!error.empty()) |
- return WriteFailure("set multiple items", error); |
- return MakeWriteResult(changes.release()); |
+ scoped_ptr<Error> write_error = WriteToDb(&batch); |
+ return write_error ? MakeWriteResult(write_error.Pass()) |
+ : MakeWriteResult(changes.Pass()); |
} |
ValueStore::WriteResult LeveldbValueStore::Remove(const std::string& key) { |
@@ -258,9 +226,9 @@ ValueStore::WriteResult LeveldbValueStore::Remove( |
const std::vector<std::string>& keys) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- return ValueStore::MakeWriteResult(error); |
+ scoped_ptr<Error> open_error = EnsureDbIsOpen(); |
+ if (open_error) |
+ return MakeWriteResult(open_error.Pass()); |
leveldb::WriteBatch batch; |
scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList()); |
@@ -268,14 +236,12 @@ ValueStore::WriteResult LeveldbValueStore::Remove( |
for (std::vector<std::string>::const_iterator it = keys.begin(); |
it != keys.end(); ++it) { |
scoped_ptr<Value> old_value; |
- error = ReadFromDb(leveldb::ReadOptions(), *it, &old_value); |
- if (!error.empty()) { |
- return WriteFailureForKey("find changes to remove multiple items", |
- *it, |
- error); |
- } |
+ scoped_ptr<Error> read_error = |
+ ReadFromDb(leveldb::ReadOptions(), *it, &old_value); |
+ if (read_error) |
+ return MakeWriteResult(read_error.Pass()); |
- if (old_value.get()) { |
+ if (old_value) { |
changes->push_back(ValueStoreChange(*it, old_value.release(), NULL)); |
batch.Delete(*it); |
} |
@@ -283,144 +249,115 @@ ValueStore::WriteResult LeveldbValueStore::Remove( |
leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch); |
if (!status.ok() && !status.IsNotFound()) |
- return WriteFailure("remove multiple items", status.ToString()); |
- return MakeWriteResult(changes.release()); |
+ return MakeWriteResult(ToValueStoreError(status, util::NoKey())); |
+ return MakeWriteResult(changes.Pass()); |
} |
ValueStore::WriteResult LeveldbValueStore::Clear() { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- std::string error = EnsureDbIsOpen(); |
- if (!error.empty()) |
- return ValueStore::MakeWriteResult(error); |
- |
- leveldb::ReadOptions read_options; |
- // All interaction with the db is done on the same thread, so snapshotting |
- // isn't strictly necessary. This is just defensive. |
- leveldb::WriteBatch batch; |
scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList()); |
- ScopedSnapshot snapshot(db_.get()); |
- read_options.snapshot = snapshot.get(); |
- scoped_ptr<leveldb::Iterator> it(db_->NewIterator(read_options)); |
- for (it->SeekToFirst(); it->Valid(); it->Next()) { |
- const std::string key = it->key().ToString(); |
- const std::string old_value_json = it->value().ToString(); |
- Value* old_value = base::JSONReader().ReadToValue(old_value_json); |
- if (!old_value) { |
- // TODO: delete the bad JSON. |
- return WriteFailureForKey("find changes to clear", key, kInvalidJson); |
- } |
- changes->push_back(ValueStoreChange(key, old_value, NULL)); |
- batch.Delete(key); |
+ ReadResult read_result = Get(); |
+ if (read_result->HasError()) |
+ return MakeWriteResult(read_result->PassError()); |
+ |
+ base::DictionaryValue& whole_db = read_result->settings(); |
+ while (!whole_db.empty()) { |
+ std::string next_key = DictionaryValue::Iterator(whole_db).key(); |
+ scoped_ptr<base::Value> next_value; |
+ whole_db.RemoveWithoutPathExpansion(next_key, &next_value); |
+ changes->push_back( |
+ ValueStoreChange(next_key, next_value.release(), NULL)); |
} |
- if (it->status().IsNotFound()) |
- NOTREACHED() << "IsNotFound() but clearing?!"; |
- else if (!it->status().ok()) |
- return WriteFailure("find changes to clear", it->status().ToString()); |
- |
- leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch); |
- if (status.IsNotFound()) { |
- NOTREACHED() << "IsNotFound() but clearing?!"; |
- return MakeWriteResult(changes.release()); |
- } |
- if (!status.ok()) |
- return WriteFailure("clear", status.ToString()); |
- return MakeWriteResult(changes.release()); |
+ DeleteDbFile(); |
+ return MakeWriteResult(changes.Pass()); |
} |
-std::string LeveldbValueStore::EnsureDbIsOpen() { |
+scoped_ptr<ValueStore::Error> LeveldbValueStore::EnsureDbIsOpen() { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- if (db_.get()) |
- return std::string(); |
- |
-#if defined(OS_POSIX) |
- std::string os_path(db_path_.value()); |
-#elif defined(OS_WIN) |
- std::string os_path = base::SysWideToUTF8(db_path_.value()); |
-#endif |
+ if (db_) |
+ return util::NoError(); |
leveldb::Options options; |
options.max_open_files = 0; // Use minimum. |
options.create_if_missing = true; |
- leveldb::DB* db; |
- leveldb::Status status = leveldb::DB::Open(options, os_path, &db); |
- if (!status.ok()) { |
- // |os_path| may contain sensitive data, and these strings are passed |
- // through to the extension, so strip that out. |
- std::string status_string = status.ToString(); |
- ReplaceSubstringsAfterOffset(&status_string, 0u, os_path, "..."); |
- return base::StringPrintf("Failed to open database: %s", |
- status_string.c_str()); |
- } |
+ leveldb::DB* db = NULL; |
+ leveldb::Status status = leveldb::DB::Open(options, OS_PATH(db_path_), &db); |
+ if (!status.ok()) |
+ return ToValueStoreError(status, util::NoKey()); |
+ |
+ CHECK(db); |
db_.reset(db); |
- return std::string(); |
+ return util::NoError(); |
} |
-std::string LeveldbValueStore::ReadFromDb( |
+scoped_ptr<ValueStore::Error> LeveldbValueStore::ReadFromDb( |
leveldb::ReadOptions options, |
const std::string& key, |
scoped_ptr<Value>* setting) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
- DCHECK(setting != NULL); |
+ DCHECK(setting); |
+ |
std::string value_as_json; |
leveldb::Status s = db_->Get(options, key, &value_as_json); |
if (s.IsNotFound()) { |
- // Despite there being no value, it was still a success. |
- // Check this first because ok() is false on IsNotFound. |
- return std::string(); |
+ // Despite there being no value, it was still a success. Check this first |
+ // because ok() is false on IsNotFound. |
+ return util::NoError(); |
} |
if (!s.ok()) |
- return s.ToString(); |
+ return ToValueStoreError(s, util::NewKey(key)); |
Value* value = base::JSONReader().ReadToValue(value_as_json); |
- if (value == NULL) { |
- // TODO(kalman): clear the offending non-JSON value from the database. |
- return kInvalidJson; |
- } |
+ if (!value) |
+ return Error::Create(CORRUPTION, kInvalidJson, util::NewKey(key)); |
setting->reset(value); |
- return std::string(); |
+ return util::NoError(); |
} |
-std::string LeveldbValueStore::AddToBatch( |
+scoped_ptr<ValueStore::Error> LeveldbValueStore::AddToBatch( |
ValueStore::WriteOptions options, |
const std::string& key, |
const base::Value& value, |
leveldb::WriteBatch* batch, |
ValueStoreChangeList* changes) { |
- scoped_ptr<Value> old_value; |
- if (!(options & NO_CHECK_OLD_VALUE)) { |
- std::string error = ReadFromDb(leveldb::ReadOptions(), key, &old_value); |
- if (!error.empty()) |
- return error; |
- } |
+ bool write_new_value = true; |
- if (!old_value.get() || !old_value->Equals(&value)) { |
- if (!(options & NO_GENERATE_CHANGES)) { |
+ if (!(options & NO_GENERATE_CHANGES)) { |
+ scoped_ptr<Value> old_value; |
+ scoped_ptr<Error> read_error = |
+ ReadFromDb(leveldb::ReadOptions(), key, &old_value); |
+ if (read_error) |
+ return read_error.Pass(); |
+ if (!old_value || !old_value->Equals(&value)) { |
changes->push_back( |
ValueStoreChange(key, old_value.release(), value.DeepCopy())); |
+ } else { |
+ write_new_value = false; |
} |
+ } |
+ |
+ if (write_new_value) { |
std::string value_as_json; |
base::JSONWriter::Write(&value, &value_as_json); |
batch->Put(key, value_as_json); |
} |
- return std::string(); |
+ return util::NoError(); |
} |
-std::string LeveldbValueStore::WriteToDb(leveldb::WriteBatch* batch) { |
+scoped_ptr<ValueStore::Error> LeveldbValueStore::WriteToDb( |
+ leveldb::WriteBatch* batch) { |
leveldb::Status status = db_->Write(leveldb::WriteOptions(), batch); |
- if (status.IsNotFound()) { |
- NOTREACHED() << "IsNotFound() but writing?!"; |
- return std::string(); |
- } |
- return status.ok() ? std::string() : status.ToString(); |
+ return status.ok() ? util::NoError() |
+ : ToValueStoreError(status, util::NoKey()); |
} |
bool LeveldbValueStore::IsEmpty() { |
@@ -435,3 +372,25 @@ bool LeveldbValueStore::IsEmpty() { |
} |
return is_empty; |
} |
+ |
+void LeveldbValueStore::DeleteDbFile() { |
+ db_.reset(); // release any lock on the directory |
+ if (!base::DeleteFile(db_path_, true /* recursive */)) { |
+ LOG(WARNING) << "Failed to delete LeveldbValueStore database at " << |
+ db_path_.value(); |
+ } |
+} |
+ |
+scoped_ptr<ValueStore::Error> LeveldbValueStore::ToValueStoreError( |
+ const leveldb::Status& status, |
+ scoped_ptr<std::string> key) { |
+ CHECK(!status.ok()); |
+ CHECK(!status.IsNotFound()); // not an error |
+ |
+ std::string message = status.ToString(); |
+ // The message may contain |db_path_|, which may be considered sensitive |
+ // data, and those strings are passed to the extension, so strip it out. |
+ ReplaceSubstringsAfterOffset(&message, 0u, OS_PATH(db_path_), "..."); |
+ |
+ return Error::Create(CORRUPTION, message, key.Pass()); |
+} |