ValueStore::Error is now an inline member (not a ptr) of ResultTypes.

Simplifies the code if ValueStore::Error is not referenced by
a pointer, but instead is a member of ReadResultType and
WriteResultType.

Review URL: https://codereview.chromium.org/1462483004

Cr-Commit-Position: refs/heads/master@{#362317}

11 files changed, 159 insertions, 218 deletions

diff --git a/extensions/browser/value_store/leveldb_value_store.cc b/extensions/browser/value_store/leveldb_value_store.cc
index cf476fc..29e9c33 100644
--- a/extensions/browser/value_store/leveldb_value_store.cc
+++ b/extensions/browser/value_store/leveldb_value_store.cc
@@ -16,12 +16,10 @@
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "content/public/browser/browser_thread.h"
-#include "extensions/browser/value_store/value_store_util.h"
 #include "third_party/leveldatabase/env_chromium.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 {
@@ -114,14 +112,14 @@ size_t LeveldbValueStore::GetBytesInUse() {
 ValueStore::ReadResult LeveldbValueStore::Get(const std::string& key) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  scoped_ptr<Error> open_error = EnsureDbIsOpen();
-  if (open_error)
-    return MakeReadResult(open_error.Pass());
+  Status open_error = EnsureDbIsOpen();
+  if (!open_error.ok())
+    return MakeReadResult(open_error);
 
   scoped_ptr<base::Value> setting;
-  scoped_ptr<Error> error = ReadFromDb(leveldb::ReadOptions(), key, &setting);
-  if (error)
-    return MakeReadResult(error.Pass());
+  Status error = ReadFromDb(leveldb::ReadOptions(), key, &setting);
+  if (!error.ok())
+    return MakeReadResult(error);
 
   base::DictionaryValue* settings = new base::DictionaryValue();
   if (setting)
@@ -133,9 +131,9 @@ ValueStore::ReadResult LeveldbValueStore::Get(
     const std::vector<std::string>& keys) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  scoped_ptr<Error> open_error = EnsureDbIsOpen();
-  if (open_error)
-    return MakeReadResult(open_error.Pass());
+  Status open_error = EnsureDbIsOpen();
+  if (!open_error.ok())
+    return MakeReadResult(open_error);
 
   leveldb::ReadOptions options;
   scoped_ptr<base::DictionaryValue> settings(new base::DictionaryValue());
@@ -147,9 +145,9 @@ ValueStore::ReadResult LeveldbValueStore::Get(
   for (std::vector<std::string>::const_iterator it = keys.begin();
       it != keys.end(); ++it) {
     scoped_ptr<base::Value> setting;
-    scoped_ptr<Error> error = ReadFromDb(options, *it, &setting);
-    if (error)
-      return MakeReadResult(error.Pass());
+    Status error = ReadFromDb(options, *it, &setting);
+    if (!error.ok())
+      return MakeReadResult(error);
     if (setting)
       settings->SetWithoutPathExpansion(*it, setting.release());
   }
@@ -160,9 +158,9 @@ ValueStore::ReadResult LeveldbValueStore::Get(
 ValueStore::ReadResult LeveldbValueStore::Get() {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  scoped_ptr<Error> open_error = EnsureDbIsOpen();
-  if (open_error)
-    return MakeReadResult(open_error.Pass());
+  Status open_error = EnsureDbIsOpen();
+  if (!open_error.ok())
+    return MakeReadResult(open_error);
 
   base::JSONReader json_reader;
   leveldb::ReadOptions options = leveldb::ReadOptions();
@@ -178,7 +176,7 @@ ValueStore::ReadResult LeveldbValueStore::Get() {
     scoped_ptr<base::Value> value =
         json_reader.ReadToValue(it->value().ToString());
     if (!value)
-      return MakeReadResult(Error::Create(CORRUPTION, kInvalidJson));
+      return MakeReadResult(Status(CORRUPTION, kInvalidJson));
     settings->SetWithoutPathExpansion(key, value.Pass());
   }
 
@@ -197,44 +195,43 @@ ValueStore::WriteResult LeveldbValueStore::Set(
     WriteOptions options, const std::string& key, const base::Value& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  scoped_ptr<Error> open_error = EnsureDbIsOpen();
-  if (open_error)
-    return MakeWriteResult(open_error.Pass());
+  Status open_error = EnsureDbIsOpen();
+  if (!open_error.ok())
+    return MakeWriteResult(open_error);
 
   leveldb::WriteBatch batch;
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
-  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());
+  Status batch_error = AddToBatch(options, key, value, &batch, changes.get());
+  if (!batch_error.ok())
+    return MakeWriteResult(batch_error);
+
+  Status write_error = WriteToDb(&batch);
+  return write_error.ok() ? MakeWriteResult(changes.Pass())
+                          : MakeWriteResult(write_error);
 }
 
 ValueStore::WriteResult LeveldbValueStore::Set(
     WriteOptions options, const base::DictionaryValue& settings) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  scoped_ptr<Error> open_error = EnsureDbIsOpen();
-  if (open_error)
-    return MakeWriteResult(open_error.Pass());
+  Status open_error = EnsureDbIsOpen();
+  if (!open_error.ok())
+    return MakeWriteResult(open_error);
 
   leveldb::WriteBatch batch;
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
 
   for (base::DictionaryValue::Iterator it(settings);
        !it.IsAtEnd(); it.Advance()) {
-    scoped_ptr<Error> batch_error =
+    Status batch_error =
         AddToBatch(options, it.key(), it.value(), &batch, changes.get());
-    if (batch_error)
-      return MakeWriteResult(batch_error.Pass());
+    if (!batch_error.ok())
+      return MakeWriteResult(batch_error);
   }
 
-  scoped_ptr<Error> write_error = WriteToDb(&batch);
-  return write_error ? MakeWriteResult(write_error.Pass())
-                     : MakeWriteResult(changes.Pass());
+  Status write_error = WriteToDb(&batch);
+  return write_error.ok() ? MakeWriteResult(changes.Pass())
+                          : MakeWriteResult(write_error);
 }
 
 ValueStore::WriteResult LeveldbValueStore::Remove(const std::string& key) {
@@ -246,9 +243,9 @@ ValueStore::WriteResult LeveldbValueStore::Remove(
     const std::vector<std::string>& keys) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  scoped_ptr<Error> open_error = EnsureDbIsOpen();
-  if (open_error)
-    return MakeWriteResult(open_error.Pass());
+  Status open_error = EnsureDbIsOpen();
+  if (!open_error.ok())
+    return MakeWriteResult(open_error);
 
   leveldb::WriteBatch batch;
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
@@ -256,10 +253,9 @@ ValueStore::WriteResult LeveldbValueStore::Remove(
   for (std::vector<std::string>::const_iterator it = keys.begin();
       it != keys.end(); ++it) {
     scoped_ptr<base::Value> old_value;
-    scoped_ptr<Error> read_error =
-        ReadFromDb(leveldb::ReadOptions(), *it, &old_value);
-    if (read_error)
-      return MakeWriteResult(read_error.Pass());
+    Status read_error = ReadFromDb(leveldb::ReadOptions(), *it, &old_value);
+    if (!read_error.ok())
+      return MakeWriteResult(read_error);
 
     if (old_value) {
       changes->push_back(ValueStoreChange(*it, old_value.release(), NULL));
@@ -279,8 +275,8 @@ ValueStore::WriteResult LeveldbValueStore::Clear() {
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
 
   ReadResult read_result = Get();
-  if (read_result->HasError())
-    return MakeWriteResult(read_result->PassError());
+  if (!read_result->status().ok())
+    return MakeWriteResult(read_result->status());
 
   base::DictionaryValue& whole_db = read_result->settings();
   while (!whole_db.empty()) {
@@ -325,22 +321,22 @@ bool LeveldbValueStore::RestoreKey(const std::string& key) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   ReadResult result = Get(key);
-  if (result->IsCorrupted()) {
+  if (result->status().IsCorrupted()) {
     leveldb::WriteBatch batch;
     batch.Delete(key);
-    scoped_ptr<ValueStore::Error> error = WriteToDb(&batch);
+    ValueStore::Status error = WriteToDb(&batch);
     // If we can't delete the key, the restore failed.
-    if (error.get())
+    if (!error.ok())
       return false;
     result = Get(key);
   }
 
   // The restore succeeded if there is no corruption error.
-  return !result->IsCorrupted();
+  return !result->status().IsCorrupted();
 }
 
 bool LeveldbValueStore::WriteToDbForTest(leveldb::WriteBatch* batch) {
-  return !WriteToDb(batch).get();
+  return WriteToDb(batch).ok();
 }
 
 bool LeveldbValueStore::OnMemoryDump(
@@ -375,11 +371,11 @@ bool LeveldbValueStore::OnMemoryDump(
   return true;
 }
 
-scoped_ptr<ValueStore::Error> LeveldbValueStore::EnsureDbIsOpen() {
+ValueStore::Status LeveldbValueStore::EnsureDbIsOpen() {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   if (db_)
-    return util::NoError();
+    return Status();
 
   leveldb::Options options;
   options.max_open_files = 0;  // Use minimum.
@@ -402,10 +398,10 @@ scoped_ptr<ValueStore::Error> LeveldbValueStore::EnsureDbIsOpen() {
 
   CHECK(db);
   db_.reset(db);
-  return util::NoError();
+  return Status();
 }
 
-scoped_ptr<ValueStore::Error> LeveldbValueStore::ReadFromDb(
+ValueStore::Status LeveldbValueStore::ReadFromDb(
     leveldb::ReadOptions options,
     const std::string& key,
     scoped_ptr<base::Value>* setting) {
@@ -418,7 +414,7 @@ scoped_ptr<ValueStore::Error> LeveldbValueStore::ReadFromDb(
   if (s.IsNotFound()) {
     // Despite there being no value, it was still a success. Check this first
     // because ok() is false on IsNotFound.
-    return util::NoError();
+    return Status();
   }
 
   if (!s.ok())
@@ -426,13 +422,13 @@ scoped_ptr<ValueStore::Error> LeveldbValueStore::ReadFromDb(
 
   scoped_ptr<base::Value> value = base::JSONReader().ReadToValue(value_as_json);
   if (!value)
-    return Error::Create(CORRUPTION, kInvalidJson);
+    return Status(CORRUPTION, kInvalidJson);
 
   *setting = value.Pass();
-  return util::NoError();
+  return Status();
 }
 
-scoped_ptr<ValueStore::Error> LeveldbValueStore::AddToBatch(
+ValueStore::Status LeveldbValueStore::AddToBatch(
     ValueStore::WriteOptions options,
     const std::string& key,
     const base::Value& value,
@@ -442,14 +438,11 @@ scoped_ptr<ValueStore::Error> LeveldbValueStore::AddToBatch(
 
   if (!(options & NO_GENERATE_CHANGES)) {
     scoped_ptr<base::Value> old_value;
-    scoped_ptr<Error> read_error =
-        ReadFromDb(leveldb::ReadOptions(), key, &old_value);
-    if (read_error && read_error->code == CORRUPTION) {
+    Status read_error = ReadFromDb(leveldb::ReadOptions(), key, &old_value);
+    if (read_error.IsCorrupted())
       batch->Delete(key);
-      read_error.reset();
-    }
-    if (read_error)
-      return read_error.Pass();
+    else if (!read_error.ok())
+      return read_error;
     if (!old_value || !old_value->Equals(&value)) {
       changes->push_back(
           ValueStoreChange(key, old_value.release(), value.DeepCopy()));
@@ -461,17 +454,16 @@ scoped_ptr<ValueStore::Error> LeveldbValueStore::AddToBatch(
   if (write_new_value) {
     std::string value_as_json;
     if (!base::JSONWriter::Write(value, &value_as_json))
-      return Error::Create(OTHER_ERROR, kCannotSerialize);
+      return Status(OTHER_ERROR, kCannotSerialize);
     batch->Put(key, value_as_json);
   }
 
-  return util::NoError();
+  return Status();
 }
 
-scoped_ptr<ValueStore::Error> LeveldbValueStore::WriteToDb(
-    leveldb::WriteBatch* batch) {
+ValueStore::Status LeveldbValueStore::WriteToDb(leveldb::WriteBatch* batch) {
   leveldb::Status status = db_->Write(leveldb::WriteOptions(), batch);
-  return status.ok() ? util::NoError() : ToValueStoreError(status);
+  return status.ok() ? Status() : ToValueStoreError(status);
 }
 
 bool LeveldbValueStore::IsEmpty() {
@@ -497,7 +489,7 @@ bool LeveldbValueStore::DeleteDbFile() {
   return true;
 }
 
-scoped_ptr<ValueStore::Error> LeveldbValueStore::ToValueStoreError(
+ValueStore::Status LeveldbValueStore::ToValueStoreError(
     const leveldb::Status& status) {
   CHECK(!status.ok());
   CHECK(!status.IsNotFound());  // not an error
@@ -508,5 +500,5 @@ scoped_ptr<ValueStore::Error> LeveldbValueStore::ToValueStoreError(
   base::ReplaceSubstringsAfterOffset(
       &message, 0u, db_path_.AsUTF8Unsafe(), "...");
 
-  return Error::Create(CORRUPTION, message);
+  return Status(CORRUPTION, message);
 }
diff --git a/extensions/browser/value_store/leveldb_value_store.h b/extensions/browser/value_store/leveldb_value_store.h
index d32be6e..81956fb 100644
--- a/extensions/browser/value_store/leveldb_value_store.h
+++ b/extensions/browser/value_store/leveldb_value_store.h
@@ -64,30 +64,27 @@ class LeveldbValueStore : public ValueStore,
 
  private:
   // Tries to open the database if it hasn't been opened already.
-  scoped_ptr<ValueStore::Error> EnsureDbIsOpen();
+  ValueStore::Status EnsureDbIsOpen();
 
   // Reads a setting from the database.
-  scoped_ptr<ValueStore::Error> ReadFromDb(
-      leveldb::ReadOptions options,
-      const std::string& key,
-      // Will be reset() with the result, if any.
-      scoped_ptr<base::Value>* setting);
+  ValueStore::Status ReadFromDb(leveldb::ReadOptions options,
+                                const std::string& key,
+                                // Will be reset() with the result, if any.
+                                scoped_ptr<base::Value>* setting);
 
   // Adds a setting to a WriteBatch, and logs the change in |changes|. For use
   // with WriteToDb.
-  scoped_ptr<ValueStore::Error> AddToBatch(ValueStore::WriteOptions options,
-                                           const std::string& key,
-                                           const base::Value& value,
-                                           leveldb::WriteBatch* batch,
-                                           ValueStoreChangeList* changes);
+  ValueStore::Status AddToBatch(ValueStore::WriteOptions options,
+                                const std::string& key,
+                                const base::Value& value,
+                                leveldb::WriteBatch* batch,
+                                ValueStoreChangeList* changes);
 
   // Commits the changes in |batch| to the database.
-  scoped_ptr<ValueStore::Error> WriteToDb(leveldb::WriteBatch* batch);
+  ValueStore::Status WriteToDb(leveldb::WriteBatch* batch);
 
-  // Converts an error leveldb::Status to a ValueStore::Error. Returns a
-  // scoped_ptr for convenience; the result will always be non-empty.
-  scoped_ptr<ValueStore::Error> ToValueStoreError(
-      const leveldb::Status& status);
+  // Converts an error leveldb::Status to a ValueStore::Status.
+  ValueStore::Status ToValueStoreError(const leveldb::Status& status);
 
   // Removes the on-disk database at |db_path_|. Any file system locks should
   // be released before calling this method.
diff --git a/extensions/browser/value_store/leveldb_value_store_unittest.cc b/extensions/browser/value_store/leveldb_value_store_unittest.cc
index 0604499..d5a28ac 100644
--- a/extensions/browser/value_store/leveldb_value_store_unittest.cc
+++ b/extensions/browser/value_store/leveldb_value_store_unittest.cc
@@ -40,7 +40,7 @@ class LeveldbValueStoreUnitTest : public testing::Test {
   void SetUp() override {
     ASSERT_TRUE(database_dir_.CreateUniqueTempDir());
     OpenStore();
-    ASSERT_FALSE(store_->Get()->HasError());
+    ASSERT_TRUE(store_->Get()->status().ok());
   }
 
   void TearDown() override {
@@ -72,8 +72,9 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreKeyTest) {
 
   // Insert a valid pair.
   scoped_ptr<base::Value> value(new base::StringValue(kValue));
-  ASSERT_FALSE(
-      store()->Set(ValueStore::DEFAULTS, kNotCorruptKey, *value)->HasError());
+  ASSERT_TRUE(store()
+                  ->Set(ValueStore::DEFAULTS, kNotCorruptKey, *value)
+                  ->status().ok());
 
   // Insert a corrupt pair.
   const char kCorruptKey[] = "corrupt";
@@ -83,18 +84,18 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreKeyTest) {
 
   // Verify corruption.
   ValueStore::ReadResult result = store()->Get(kCorruptKey);
-  ASSERT_TRUE(result->HasError());
-  ASSERT_EQ(ValueStore::CORRUPTION, result->error().code);
+  ASSERT_FALSE(result->status().ok());
+  ASSERT_EQ(ValueStore::CORRUPTION, result->status().code);
 
   // Restore and verify.
   ASSERT_TRUE(store()->RestoreKey(kCorruptKey));
   result = store()->Get(kCorruptKey);
-  EXPECT_FALSE(result->HasError());
+  EXPECT_TRUE(result->status().ok());
   EXPECT_TRUE(result->settings().empty());
 
   // Verify that the valid pair is still present.
   result = store()->Get(kNotCorruptKey);
-  EXPECT_FALSE(result->HasError());
+  EXPECT_TRUE(result->status().ok());
   EXPECT_TRUE(result->settings().HasKey(kNotCorruptKey));
   std::string value_string;
   EXPECT_TRUE(result->settings().GetString(kNotCorruptKey, &value_string));
@@ -114,9 +115,9 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreDoesMinimumNecessary) {
   // Insert a collection of non-corrupted pairs.
   scoped_ptr<base::Value> value(new base::StringValue(kValue));
   for (size_t i = 0; i < kNotCorruptKeysSize; ++i) {
-    ASSERT_FALSE(store()
-                     ->Set(ValueStore::DEFAULTS, kNotCorruptKeys[i], *value)
-                     ->HasError());
+    ASSERT_TRUE(store()
+                    ->Set(ValueStore::DEFAULTS, kNotCorruptKeys[i], *value)
+                    ->status().ok());
   }
 
   // Insert a few corrupted pairs.
@@ -127,8 +128,8 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreDoesMinimumNecessary) {
 
   // Verify that we broke it, and then fix it.
   ValueStore::ReadResult result = store()->Get();
-  ASSERT_TRUE(result->HasError());
-  ASSERT_EQ(ValueStore::CORRUPTION, result->error().code);
+  ASSERT_FALSE(result->status().ok());
+  ASSERT_EQ(ValueStore::CORRUPTION, result->status().code);
 
   ASSERT_TRUE(store()->Restore());
 
@@ -136,7 +137,7 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreDoesMinimumNecessary) {
   std::string value_string;
   for (size_t i = 0; i < kNotCorruptKeysSize; ++i) {
     result = store()->Get(kNotCorruptKeys[i]);
-    EXPECT_FALSE(result->HasError());
+    EXPECT_TRUE(result->status().ok());
     EXPECT_TRUE(result->settings().HasKey(kNotCorruptKeys[i]));
     EXPECT_TRUE(
         result->settings().GetString(kNotCorruptKeys[i], &value_string));
@@ -159,9 +160,9 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreFullDatabase) {
   // Generate a database.
   scoped_ptr<base::Value> value(new base::StringValue(kValue));
   for (size_t i = 0; i < kNotCorruptKeysSize; ++i) {
-    ASSERT_FALSE(store()
-                     ->Set(ValueStore::DEFAULTS, kNotCorruptKeys[i], *value)
-                     ->HasError());
+    ASSERT_TRUE(store()
+                    ->Set(ValueStore::DEFAULTS, kNotCorruptKeys[i], *value)
+                    ->status().ok());
   }
 
   // Close it (so we remove the lock), and replace all files with LolCats.
@@ -178,12 +179,12 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreFullDatabase) {
 
   // We should definitely have an error.
   ValueStore::ReadResult result = store()->Get();
-  ASSERT_TRUE(result->HasError());
-  ASSERT_EQ(ValueStore::CORRUPTION, result->error().code);
+  ASSERT_FALSE(result->status().ok());
+  ASSERT_EQ(ValueStore::CORRUPTION, result->status().code);
 
   ASSERT_TRUE(store()->Restore());
   result = store()->Get();
-  EXPECT_FALSE(result->HasError());
+  EXPECT_TRUE(result->status().ok());
   // We couldn't recover anything, but we should be in a sane state again.
   EXPECT_EQ(0u, result->settings().size());
 }
diff --git a/extensions/browser/value_store/testing_value_store.cc b/extensions/browser/value_store/testing_value_store.cc
index 2629538..73536f3 100644
--- a/extensions/browser/value_store/testing_value_store.cc
+++ b/extensions/browser/value_store/testing_value_store.cc
@@ -12,11 +12,14 @@ const char kGenericErrorMessage[] = "TestingValueStore configured to error";
 
 }  // namespace
 
-TestingValueStore::TestingValueStore()
-    : read_count_(0), write_count_(0), error_code_(OK) {}
+TestingValueStore::TestingValueStore() : read_count_(0), write_count_(0) {}
 
 TestingValueStore::~TestingValueStore() {}
 
+void TestingValueStore::set_status_code(StatusCode status_code) {
+  status_ = ValueStore::Status(status_code, kGenericErrorMessage);
+}
+
 size_t TestingValueStore::GetBytesInUse(const std::string& key) {
   // Let SettingsStorageQuotaEnforcer implement this.
   NOTREACHED() << "Not implemented";
@@ -43,8 +46,8 @@ ValueStore::ReadResult TestingValueStore::Get(const std::string& key) {
 ValueStore::ReadResult TestingValueStore::Get(
     const std::vector<std::string>& keys) {
   read_count_++;
-  if (error_code_ != OK)
-    return MakeReadResult(TestingError());
+  if (!status_.ok())
+    return MakeReadResult(status_);
 
   base::DictionaryValue* settings = new base::DictionaryValue();
   for (std::vector<std::string>::const_iterator it = keys.begin();
@@ -59,8 +62,8 @@ ValueStore::ReadResult TestingValueStore::Get(
 
 ValueStore::ReadResult TestingValueStore::Get() {
   read_count_++;
-  if (error_code_ != OK)
-    return MakeReadResult(TestingError());
+  if (!status_.ok())
+    return MakeReadResult(status_);
   return MakeReadResult(make_scoped_ptr(storage_.DeepCopy()));
 }
 
@@ -74,8 +77,8 @@ ValueStore::WriteResult TestingValueStore::Set(
 ValueStore::WriteResult TestingValueStore::Set(
     WriteOptions options, const base::DictionaryValue& settings) {
   write_count_++;
-  if (error_code_ != OK)
-    return MakeWriteResult(TestingError());
+  if (!status_.ok())
+    return MakeWriteResult(status_);
 
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
   for (base::DictionaryValue::Iterator it(settings);
@@ -101,8 +104,8 @@ ValueStore::WriteResult TestingValueStore::Remove(const std::string& key) {
 ValueStore::WriteResult TestingValueStore::Remove(
     const std::vector<std::string>& keys) {
   write_count_++;
-  if (error_code_ != OK)
-    return MakeWriteResult(TestingError());
+  if (!status_.ok())
+    return MakeWriteResult(status_);
 
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
   for (std::vector<std::string>::const_iterator it = keys.begin();
@@ -131,7 +134,3 @@ bool TestingValueStore::Restore() {
 bool TestingValueStore::RestoreKey(const std::string& key) {
   return true;
 }
-
-scoped_ptr<ValueStore::Error> TestingValueStore::TestingError() {
-  return ValueStore::Error::Create(error_code_, kGenericErrorMessage);
-}
diff --git a/extensions/browser/value_store/testing_value_store.h b/extensions/browser/value_store/testing_value_store.h
index 38e5258..9559649 100644
--- a/extensions/browser/value_store/testing_value_store.h
+++ b/extensions/browser/value_store/testing_value_store.h
@@ -20,7 +20,7 @@ class TestingValueStore : public ValueStore {
 
   // Sets the error code for requests. If OK, errors won't be thrown.
   // Defaults to OK.
-  void set_error_code(ErrorCode error_code) { error_code_ = error_code; }
+  void set_status_code(StatusCode status_code);
 
   // Accessors for the number of reads/writes done by this value store. Each
   // Get* operation (except for the BytesInUse ones) counts as one read, and
@@ -51,12 +51,10 @@ class TestingValueStore : public ValueStore {
   bool RestoreKey(const std::string& key) override;
 
  private:
-  scoped_ptr<ValueStore::Error> TestingError();
-
   base::DictionaryValue storage_;
   int read_count_;
   int write_count_;
-  ErrorCode error_code_;
+  ValueStore::Status status_;
 
   DISALLOW_COPY_AND_ASSIGN(TestingValueStore);
 };
diff --git a/extensions/browser/value_store/value_store.cc b/extensions/browser/value_store/value_store.cc
index bd340fb..218956c 100644
--- a/extensions/browser/value_store/value_store.cc
+++ b/extensions/browser/value_store/value_store.cc
@@ -6,12 +6,14 @@
 
 #include "base/logging.h"
 
-// Implementation of Error.
+// Implementation of Status.
 
-ValueStore::Error::Error(ErrorCode code, const std::string& message)
+ValueStore::Status::Status() : code(OK) {}
+
+ValueStore::Status::Status(StatusCode code, const std::string& message)
     : code(code), message(message) {}
 
-ValueStore::Error::~Error() {}
+ValueStore::Status::~Status() {}
 
 // Implementation of ReadResultType.
 
@@ -20,10 +22,8 @@ ValueStore::ReadResultType::ReadResultType(
   CHECK(settings_);
 }
 
-ValueStore::ReadResultType::ReadResultType(scoped_ptr<Error> error)
-    : error_(error.Pass()) {
-  CHECK(error_);
-}
+ValueStore::ReadResultType::ReadResultType(const Status& status)
+    : status_(status) {}
 
 ValueStore::ReadResultType::~ReadResultType() {}
 
@@ -35,9 +35,7 @@ ValueStore::WriteResultType::WriteResultType(
   CHECK(changes_);
 }
 
-ValueStore::WriteResultType::WriteResultType(scoped_ptr<Error> error)
-    : error_(error.Pass()) {
-  CHECK(error_);
-}
+ValueStore::WriteResultType::WriteResultType(const Status& status)
+    : status_(status) {}
 
 ValueStore::WriteResultType::~WriteResultType() {}
diff --git a/extensions/browser/value_store/value_store.h b/extensions/browser/value_store/value_store.h
index 711b327..9ac652a 100644
--- a/extensions/browser/value_store/value_store.h
+++ b/extensions/browser/value_store/value_store.h
@@ -15,8 +15,8 @@
 // Interface for a storage area for Value objects.
 class ValueStore {
  public:
-  // Error codes returned from storage methods.
-  enum ErrorCode {
+  // Status codes returned from storage methods.
+  enum StatusCode {
     OK,
 
     // The failure was due to some kind of database corruption. Depending on
@@ -39,40 +39,30 @@ class ValueStore {
     OTHER_ERROR,
   };
 
-  // Bundles an ErrorCode with further metadata.
-  struct Error {
-    ~Error();
+  // The status (result) of an operation on a ValueStore.
+  struct Status {
+    Status();
+    Status(StatusCode code, const std::string& message);
+    ~Status();
 
-    static scoped_ptr<Error> Create(ErrorCode code,
-                                    const std::string& message) {
-      return make_scoped_ptr(new Error(code, message));
-    }
-
-    // The error code.
-    const ErrorCode code;
+    bool ok() const { return code == OK; }
 
-    // Message associated with the error.
-    const std::string message;
+    bool IsCorrupted() const { return code == CORRUPTION; }
 
-   private:
-    Error(ErrorCode code, const std::string& message);
+    // The status code.
+    StatusCode code;
 
-    DISALLOW_COPY_AND_ASSIGN(Error);
+    // Message associated with the status (error) if there is one.
+    std::string message;
   };
 
   // The result of a read operation (Get).
   class ReadResultType {
    public:
     explicit ReadResultType(scoped_ptr<base::DictionaryValue> settings);
-    explicit ReadResultType(scoped_ptr<Error> error);
+    explicit ReadResultType(const Status& status);
     ~ReadResultType();
 
-    bool HasError() const { return error_; }
-
-    bool IsCorrupted() const {
-      return error_.get() && error_->code == CORRUPTION;
-    }
-
     // Gets the settings read from the storage. Note that this represents
     // the root object. If you request the value for key "foo", that value will
     // be in |settings|.|foo|.
@@ -83,13 +73,11 @@ class ValueStore {
       return settings_.Pass();
     }
 
-    // Only call if HasError is true.
-    const Error& error() const { return *error_; }
-    scoped_ptr<Error> PassError() { return error_.Pass(); }
+    const Status& status() const { return status_; }
 
    private:
     scoped_ptr<base::DictionaryValue> settings_;
-    scoped_ptr<Error> error_;
+    Status status_;
 
     DISALLOW_COPY_AND_ASSIGN(ReadResultType);
   };
@@ -99,24 +87,20 @@ class ValueStore {
   class WriteResultType {
    public:
     explicit WriteResultType(scoped_ptr<ValueStoreChangeList> changes);
-    explicit WriteResultType(scoped_ptr<Error> error);
+    explicit WriteResultType(const Status& status);
     ~WriteResultType();
 
-    bool HasError() const { return error_; }
-
     // Gets the list of changes to the settings which resulted from the write.
     // Won't be present if the NO_GENERATE_CHANGES WriteOptions was given.
-    // Only call if HasError is false.
+    // Only call if no error.
     ValueStoreChangeList& changes() { return *changes_; }
     scoped_ptr<ValueStoreChangeList> PassChanges() { return changes_.Pass(); }
 
-    // Only call if HasError is true.
-    const Error& error() const { return *error_; }
-    scoped_ptr<Error> PassError() { return error_.Pass(); }
+    const Status& status() const { return status_; }
 
    private:
     scoped_ptr<ValueStoreChangeList> changes_;
-    scoped_ptr<Error> error_;
+    Status status_;
 
     DISALLOW_COPY_AND_ASSIGN(WriteResultType);
   };
@@ -142,11 +126,17 @@ class ValueStore {
   static ReadResult MakeReadResult(scoped_ptr<T> arg) {
     return ReadResult(new ReadResultType(arg.Pass()));
   }
+  static ReadResult MakeReadResult(const Status& status) {
+    return ReadResult(new ReadResultType(status));
+  }
 
   template<typename T>
   static WriteResult MakeWriteResult(scoped_ptr<T> arg) {
     return WriteResult(new WriteResultType(arg.Pass()));
   }
+  static WriteResult MakeWriteResult(const Status& status) {
+    return WriteResult(new WriteResultType(status));
+  }
 
   // Gets the amount of space being used by a single value, in bytes.
   // Note: The GetBytesInUse methods are only used by extension settings at the
diff --git a/extensions/browser/value_store/value_store_frontend.cc b/extensions/browser/value_store/value_store_frontend.cc
index 33cdce7..26c3a43 100644
--- a/extensions/browser/value_store/value_store_frontend.cc
+++ b/extensions/browser/value_store/value_store_frontend.cc
@@ -40,11 +40,11 @@ class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> {
     // Extract the value from the ReadResult and pass ownership of it to the
     // callback.
     scoped_ptr<base::Value> value;
-    if (!result->HasError()) {
+    if (result->status().ok()) {
       result->settings().RemoveWithoutPathExpansion(key, &value);
     } else {
       LOG(WARNING) << "Reading " << key << " from " << db_path_.value()
-                   << " failed: " << result->error().message;
+                   << " failed: " << result->status().message;
     }
 
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
@@ -59,8 +59,8 @@ class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> {
         ValueStore::IGNORE_QUOTA | ValueStore::NO_GENERATE_CHANGES,
         key,
         *value.get());
-    LOG_IF(ERROR, result->HasError()) << "Error while writing " << key << " to "
-                                      << db_path_.value();
+    LOG_IF(ERROR, !result->status().ok()) << "Error while writing " << key
+                                          << " to " << db_path_.value();
   }
 
   void Remove(const std::string& key) {
diff --git a/extensions/browser/value_store/value_store_unittest.cc b/extensions/browser/value_store/value_store_unittest.cc
index c2ecbab..8c5f7ae 100644
--- a/extensions/browser/value_store/value_store_unittest.cc
+++ b/extensions/browser/value_store/value_store_unittest.cc
@@ -55,9 +55,9 @@ testing::AssertionResult SettingsEq(
     const char* _1, const char* _2,
     const base::DictionaryValue& expected,
     ValueStore::ReadResult actual_result) {
-  if (actual_result->HasError()) {
-    return testing::AssertionFailure() <<
-        "Result has error: " << actual_result->error().message;
+  if (!actual_result->status().ok()) {
+    return testing::AssertionFailure() << "Result has error: "
+                                       << actual_result->status().message;
   }
 
   std::string error;
@@ -74,9 +74,9 @@ testing::AssertionResult ChangesEq(
     const char* _1, const char* _2,
     const ValueStoreChangeList& expected,
     ValueStore::WriteResult actual_result) {
-  if (actual_result->HasError()) {
-    return testing::AssertionFailure() <<
-        "Result has error: " << actual_result->error().message;
+  if (!actual_result->status().ok()) {
+    return testing::AssertionFailure() << "Result has error: "
+                                       << actual_result->status().message;
   }
 
   const ValueStoreChangeList& actual = actual_result->changes();
diff --git a/extensions/browser/value_store/value_store_util.cc b/extensions/browser/value_store/value_store_util.cc
deleted file mode 100644
index b7cdff1..0000000
--- a/extensions/browser/value_store/value_store_util.cc
+++ /dev/null
@@ -1,13 +0,0 @@
-// Copyright 2014 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 "extensions/browser/value_store/value_store_util.h"
-
-namespace value_store_util {
-
-scoped_ptr<ValueStore::Error> NoError() {
-  return scoped_ptr<ValueStore::Error>();
-}
-
-}  // namespace value_store_util
diff --git a/extensions/browser/value_store/value_store_util.h b/extensions/browser/value_store/value_store_util.h
deleted file mode 100644
index 6ee22aa..0000000
--- a/extensions/browser/value_store/value_store_util.h
+++ /dev/null
@@ -1,21 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_UTIL_H_
-#define EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_UTIL_H_
-
-#include <string>
-
-#include "base/memory/scoped_ptr.h"
-#include "extensions/browser/value_store/value_store.h"
-
-namespace value_store_util {
-
-// Return an empty Error. Useful for creating ValueStore::Error-less
-// ValueStore::Read/WriteResults.
-scoped_ptr<ValueStore::Error> NoError();
-
-}  // namespace value_store_util
-
-#endif  // EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_UTIL_H_