ValueStore will now automatically repair corrupted db's.

Previously the owner of the ValueStore was responsible for handling
corruption errors, and calling RestoreKey() or Restore() accordingly.
This change shifts that policy decision to ValueStore. ValueStore::Status
has been enhanced to report the database restore status for the
caller to deal as deemed appropriate.

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

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

13 files changed, 398 insertions, 356 deletions

diff --git a/extensions/browser/api/storage/settings_storage_quota_enforcer.cc b/extensions/browser/api/storage/settings_storage_quota_enforcer.cc
index 82a0d60..98c299b 100644
--- a/extensions/browser/api/storage/settings_storage_quota_enforcer.cc
+++ b/extensions/browser/api/storage/settings_storage_quota_enforcer.cc
@@ -43,15 +43,6 @@ void Allocate(
   (*used_per_setting)[key] = new_size;
 }
 
-// Frees the allocation of a setting in a record of total and per-setting usage.
-void Free(
-    size_t* used_total,
-    std::map<std::string, size_t>* used_per_setting,
-    const std::string& key) {
-  *used_total -= (*used_per_setting)[key];
-  used_per_setting->erase(key);
-}
-
 ValueStore::Status QuotaExceededError(Resource resource) {
   const char* name = NULL;
   // TODO(kalman): These hisograms are both silly and untracked. Fix.
@@ -79,15 +70,17 @@ ValueStore::Status QuotaExceededError(Resource resource) {
 
 }  // namespace
 
-SettingsStorageQuotaEnforcer::SettingsStorageQuotaEnforcer(
-    const Limits& limits, ValueStore* delegate)
-    : limits_(limits), delegate_(delegate), used_total_(0) {
-  CalculateUsage();
-}
+SettingsStorageQuotaEnforcer::SettingsStorageQuotaEnforcer(const Limits& limits,
+                                                           ValueStore* delegate)
+    : limits_(limits),
+      delegate_(delegate),
+      used_total_(0),
+      usage_calculated_(false) {}
 
 SettingsStorageQuotaEnforcer::~SettingsStorageQuotaEnforcer() {}
 
 size_t SettingsStorageQuotaEnforcer::GetBytesInUse(const std::string& key) {
+  LazyCalculateUsage();
   std::map<std::string, size_t>::iterator maybe_used =
       used_per_setting_.find(key);
   return maybe_used == used_per_setting_.end() ? 0u : maybe_used->second;
@@ -96,35 +89,35 @@ size_t SettingsStorageQuotaEnforcer::GetBytesInUse(const std::string& key) {
 size_t SettingsStorageQuotaEnforcer::GetBytesInUse(
     const std::vector<std::string>& keys) {
   size_t used = 0;
-  for (std::vector<std::string>::const_iterator it = keys.begin();
-      it != keys.end(); ++it) {
-    used += GetBytesInUse(*it);
-  }
+  for (const std::string& key : keys)
+    used += GetBytesInUse(key);
   return used;
 }
 
 size_t SettingsStorageQuotaEnforcer::GetBytesInUse() {
   // All ValueStore implementations rely on GetBytesInUse being
   // implemented here.
+  LazyCalculateUsage();
   return used_total_;
 }
 
 ValueStore::ReadResult SettingsStorageQuotaEnforcer::Get(
     const std::string& key) {
-  return delegate_->Get(key);
+  return HandleResult(delegate_->Get(key));
 }
 
 ValueStore::ReadResult SettingsStorageQuotaEnforcer::Get(
     const std::vector<std::string>& keys) {
-  return delegate_->Get(keys);
+  return HandleResult(delegate_->Get(keys));
 }
 
 ValueStore::ReadResult SettingsStorageQuotaEnforcer::Get() {
-  return delegate_->Get();
+  return HandleResult(delegate_->Get());
 }
 
 ValueStore::WriteResult SettingsStorageQuotaEnforcer::Set(
     WriteOptions options, const std::string& key, const base::Value& value) {
+  LazyCalculateUsage();
   size_t new_used_total = used_total_;
   std::map<std::string, size_t> new_used_per_setting = used_per_setting_;
   Allocate(key, value, &new_used_total, &new_used_per_setting);
@@ -138,17 +131,20 @@ ValueStore::WriteResult SettingsStorageQuotaEnforcer::Set(
       return MakeWriteResult(QuotaExceededError(MAX_ITEMS));
   }
 
-  WriteResult result = delegate_->Set(options, key, value);
+  WriteResult result = HandleResult(delegate_->Set(options, key, value));
   if (!result->status().ok())
     return result.Pass();
 
-  used_total_ = new_used_total;
-  used_per_setting_.swap(new_used_per_setting);
+  if (usage_calculated_) {
+    used_total_ = new_used_total;
+    used_per_setting_.swap(new_used_per_setting);
+  }
   return result.Pass();
 }
 
 ValueStore::WriteResult SettingsStorageQuotaEnforcer::Set(
     WriteOptions options, const base::DictionaryValue& values) {
+  LazyCalculateUsage();
   size_t new_used_total = used_total_;
   std::map<std::string, size_t> new_used_per_setting = used_per_setting_;
   for (base::DictionaryValue::Iterator it(values); !it.IsAtEnd();
@@ -168,83 +164,78 @@ ValueStore::WriteResult SettingsStorageQuotaEnforcer::Set(
       return MakeWriteResult(QuotaExceededError(MAX_ITEMS));
   }
 
-  WriteResult result = delegate_->Set(options, values);
+  WriteResult result = HandleResult(delegate_->Set(options, values));
   if (!result->status().ok())
     return result.Pass();
 
-  used_total_ = new_used_total;
-  used_per_setting_ = new_used_per_setting;
+  if (usage_calculated_) {
+    used_total_ = new_used_total;
+    used_per_setting_ = new_used_per_setting;
+  }
+
   return result.Pass();
 }
 
 ValueStore::WriteResult SettingsStorageQuotaEnforcer::Remove(
     const std::string& key) {
-  WriteResult result = delegate_->Remove(key);
+  LazyCalculateUsage();
+  WriteResult result = HandleResult(delegate_->Remove(key));
   if (!result->status().ok())
     return result.Pass();
-  Free(&used_total_, &used_per_setting_, key);
+  Free(key);
+
   return result.Pass();
 }
 
 ValueStore::WriteResult SettingsStorageQuotaEnforcer::Remove(
     const std::vector<std::string>& keys) {
-  WriteResult result = delegate_->Remove(keys);
+  WriteResult result = HandleResult(delegate_->Remove(keys));
   if (!result->status().ok())
     return result.Pass();
 
-  for (std::vector<std::string>::const_iterator it = keys.begin();
-      it != keys.end(); ++it) {
-    Free(&used_total_, &used_per_setting_, *it);
-  }
+  for (const std::string& key : keys)
+    Free(key);
+
   return result.Pass();
 }
 
 ValueStore::WriteResult SettingsStorageQuotaEnforcer::Clear() {
-  WriteResult result = delegate_->Clear();
+  LazyCalculateUsage();
+  WriteResult result = HandleResult(delegate_->Clear());
   if (!result->status().ok())
     return result.Pass();
 
   used_per_setting_.clear();
-  used_total_ = 0;
+  used_total_ = 0u;
+
   return result.Pass();
 }
 
-bool SettingsStorageQuotaEnforcer::Restore() {
-  if (!delegate_->Restore()) {
-    // If we failed, we can't calculate the usage - that's okay, though, because
-    // next time we Restore() (if it succeeds) we will recalculate usage anyway.
-    // So reset storage counters now to free up resources.
+template <class T>
+T SettingsStorageQuotaEnforcer::HandleResult(T result) {
+  if (result->status().restore_status != RESTORE_NONE) {
+    // Restoration means that an unknown amount, possibly all, of the data was
+    // lost from the database. Reset our counters - they will be lazily
+    // recalculated if/when needed.
     used_per_setting_.clear();
     used_total_ = 0u;
-    return false;
+    usage_calculated_ = false;
   }
-  CalculateUsage();
-  return true;
+  return result;
 }
 
-bool SettingsStorageQuotaEnforcer::RestoreKey(const std::string& key) {
-  if (!delegate_->RestoreKey(key))
-    return false;
+void SettingsStorageQuotaEnforcer::LazyCalculateUsage() {
+  if (usage_calculated_)
+    return;
 
-  ReadResult result = Get(key);
-  // If the key was deleted as a result of the Restore() call, free it.
-  if (!result->settings().HasKey(key) && ContainsKey(used_per_setting_, key))
-    Free(&used_total_, &used_per_setting_, key);
-  return true;
-}
+  DCHECK_EQ(0u, used_total_);
+  DCHECK(used_per_setting_.empty());
 
-void SettingsStorageQuotaEnforcer::CalculateUsage() {
-  ReadResult maybe_settings = delegate_->Get();
+  ReadResult maybe_settings = HandleResult(delegate_->Get());
   if (!maybe_settings->status().ok()) {
-    // Try to restore the database if it's corrupt.
-    if (maybe_settings->status().IsCorrupted() && delegate_->Restore())
-      maybe_settings = delegate_->Get();
-
-    if (!maybe_settings->status().ok()) {
-      LOG(WARNING) << "Failed to get settings for quota:"
-                   << maybe_settings->status().message;
-      return;
-    }
+    LOG(WARNING) << "Failed to get settings for quota:"
+                 << maybe_settings->status().message;
+    return;
   }
 
   for (base::DictionaryValue::Iterator it(maybe_settings->settings());
@@ -252,6 +243,19 @@ void SettingsStorageQuotaEnforcer::CalculateUsage() {
        it.Advance()) {
     Allocate(it.key(), it.value(), &used_total_, &used_per_setting_);
   }
+
+  usage_calculated_ = true;
+}
+
+void SettingsStorageQuotaEnforcer::Free(const std::string& key) {
+  if (!usage_calculated_)
+    return;
+  auto it = used_per_setting_.find(key);
+  if (it == used_per_setting_.end())
+    return;
+  DCHECK_GE(used_total_, it->second);
+  used_total_ -= it->second;
+  used_per_setting_.erase(it);
 }
 
 }  // namespace extensions
diff --git a/extensions/browser/api/storage/settings_storage_quota_enforcer.h b/extensions/browser/api/storage/settings_storage_quota_enforcer.h
index e3e5363..2b6d08b 100644
--- a/extensions/browser/api/storage/settings_storage_quota_enforcer.h
+++ b/extensions/browser/api/storage/settings_storage_quota_enforcer.h
@@ -49,14 +49,19 @@ class SettingsStorageQuotaEnforcer : public ValueStore {
   WriteResult Remove(const std::string& key) override;
   WriteResult Remove(const std::vector<std::string>& keys) override;
   WriteResult Clear() override;
-  bool Restore() override;
-  bool RestoreKey(const std::string& key) override;
 
   ValueStore* get_delegate_for_test() { return delegate_.get(); }
 
  private:
-  // Calculate the current usage for the database.
-  void CalculateUsage();
+  template <class T>
+  T HandleResult(T result);
+
+  // Calculate the current usage for the database if not already calculated.
+  void LazyCalculateUsage();
+
+  // Frees the allocation of a setting in a record of total and per-setting
+  // usage.
+  void Free(const std::string& key);
 
   // Limits configuration.
   const Limits limits_;
@@ -68,6 +73,9 @@ class SettingsStorageQuotaEnforcer : public ValueStore {
   // JSON-encoded values.
   size_t used_total_;
 
+  // Have the total bytes used been calculated?
+  bool usage_calculated_;
+
   // Map of item key to its size, including the key itself.
   std::map<std::string, size_t> used_per_setting_;
 
diff --git a/extensions/browser/api/storage/storage_api.cc b/extensions/browser/api/storage/storage_api.cc
index 6f754ad..2db23621 100644
--- a/extensions/browser/api/storage/storage_api.cc
+++ b/extensions/browser/api/storage/storage_api.cc
@@ -22,8 +22,7 @@ using content::BrowserThread;
 // SettingsFunction
 
 SettingsFunction::SettingsFunction()
-    : settings_namespace_(settings_namespace::INVALID),
-      tried_restoring_storage_(false) {}
+    : settings_namespace_(settings_namespace::INVALID) {}
 
 SettingsFunction::~SettingsFunction() {}
 
@@ -71,11 +70,9 @@ void SettingsFunction::AsyncRunWithStorage(ValueStore* storage) {
 }
 
 ExtensionFunction::ResponseValue SettingsFunction::UseReadResult(
-    ValueStore::ReadResult result,
-    const std::string* key,
-    ValueStore* storage) {
+    ValueStore::ReadResult result) {
   if (!result->status().ok())
-    return HandleError(result->status(), key, storage);
+    return Error(result->status().message);
 
   base::DictionaryValue* dict = new base::DictionaryValue();
   dict->Swap(&result->settings());
@@ -83,11 +80,9 @@ ExtensionFunction::ResponseValue SettingsFunction::UseReadResult(
 }
 
 ExtensionFunction::ResponseValue SettingsFunction::UseWriteResult(
-    ValueStore::WriteResult result,
-    const std::string* key,
-    ValueStore* storage) {
+    ValueStore::WriteResult result) {
   if (!result->status().ok())
-    return HandleError(result->status(), key, storage);
+    return Error(result->status().message);
 
   if (!result->changes().empty()) {
     observers_->Notify(FROM_HERE, &SettingsObserver::OnSettingsChanged,
@@ -98,30 +93,6 @@ ExtensionFunction::ResponseValue SettingsFunction::UseWriteResult(
   return NoArguments();
 }
 
-ExtensionFunction::ResponseValue SettingsFunction::HandleError(
-    const ValueStore::Status& status,
-    const std::string* key,
-    ValueStore* storage) {
-  // If the method failed due to corruption, and we haven't tried to fix it, we
-  // can try to restore the storage and re-run it. Otherwise, the method has
-  // failed.
-  if (status.IsCorrupted() && !tried_restoring_storage_) {
-    tried_restoring_storage_ = true;
-
-    // If the corruption is on a particular key, try to restore that key and
-    // re-run.
-    if (key && storage->RestoreKey(*key))
-      return RunWithStorage(storage);
-
-    // If the full database is corrupted, try to restore the whole thing and
-    // re-run.
-    if (storage->Restore())
-      return RunWithStorage(storage);
-  }
-
-  return Error(status.message);
-}
-
 // Concrete settings functions
 
 namespace {
@@ -175,19 +146,19 @@ ExtensionFunction::ResponseValue StorageStorageAreaGetFunction::RunWithStorage(
 
   switch (input->GetType()) {
     case base::Value::TYPE_NULL:
-      return UseReadResult(storage->Get(), nullptr, storage);
+      return UseReadResult(storage->Get());
 
     case base::Value::TYPE_STRING: {
       std::string as_string;
       input->GetAsString(&as_string);
-      return UseReadResult(storage->Get(as_string), &as_string, storage);
+      return UseReadResult(storage->Get(as_string));
     }
 
     case base::Value::TYPE_LIST: {
       std::vector<std::string> as_string_list;
       AddAllStringValues(*static_cast<base::ListValue*>(input),
                          &as_string_list);
-      return UseReadResult(storage->Get(as_string_list), nullptr, storage);
+      return UseReadResult(storage->Get(as_string_list));
     }
 
     case base::Value::TYPE_DICTIONARY: {
@@ -195,14 +166,13 @@ ExtensionFunction::ResponseValue StorageStorageAreaGetFunction::RunWithStorage(
           static_cast<base::DictionaryValue*>(input);
       ValueStore::ReadResult result = storage->Get(GetKeys(*as_dict));
       if (!result->status().ok()) {
-        return UseReadResult(result.Pass(), nullptr, storage);
+        return UseReadResult(result.Pass());
       }
 
       base::DictionaryValue* with_default_values = as_dict->DeepCopy();
       with_default_values->MergeDictionary(&result->settings());
-      return UseReadResult(
-          ValueStore::MakeReadResult(make_scoped_ptr(with_default_values)),
-          nullptr, storage);
+      return UseReadResult(ValueStore::MakeReadResult(
+          make_scoped_ptr(with_default_values), result->status()));
     }
 
     default:
@@ -251,8 +221,7 @@ ExtensionFunction::ResponseValue StorageStorageAreaSetFunction::RunWithStorage(
   base::DictionaryValue* input = NULL;
   if (!args_->GetDictionary(0, &input))
     return BadMessage();
-  return UseWriteResult(storage->Set(ValueStore::DEFAULTS, *input), nullptr,
-                        storage);
+  return UseWriteResult(storage->Set(ValueStore::DEFAULTS, *input));
 }
 
 void StorageStorageAreaSetFunction::GetQuotaLimitHeuristics(
@@ -270,14 +239,14 @@ StorageStorageAreaRemoveFunction::RunWithStorage(ValueStore* storage) {
     case base::Value::TYPE_STRING: {
       std::string as_string;
       input->GetAsString(&as_string);
-      return UseWriteResult(storage->Remove(as_string), &as_string, storage);
+      return UseWriteResult(storage->Remove(as_string));
     }
 
     case base::Value::TYPE_LIST: {
       std::vector<std::string> as_string_list;
       AddAllStringValues(*static_cast<base::ListValue*>(input),
                          &as_string_list);
-      return UseWriteResult(storage->Remove(as_string_list), nullptr, storage);
+      return UseWriteResult(storage->Remove(as_string_list));
     }
 
     default:
@@ -292,7 +261,7 @@ void StorageStorageAreaRemoveFunction::GetQuotaLimitHeuristics(
 
 ExtensionFunction::ResponseValue
 StorageStorageAreaClearFunction::RunWithStorage(ValueStore* storage) {
-  return UseWriteResult(storage->Clear(), nullptr, storage);
+  return UseWriteResult(storage->Clear());
 }
 
 void StorageStorageAreaClearFunction::GetQuotaLimitHeuristics(
diff --git a/extensions/browser/api/storage/storage_api.h b/extensions/browser/api/storage/storage_api.h
index 0f56fe3..e6b532c 100644
--- a/extensions/browser/api/storage/storage_api.h
+++ b/extensions/browser/api/storage/storage_api.h
@@ -30,52 +30,26 @@ class SettingsFunction : public UIThreadExtensionFunction {
   // The StorageFrontend makes sure this is posted to the appropriate thread.
   virtual ResponseValue RunWithStorage(ValueStore* storage) = 0;
 
-  // Handles the |result| of a read function.
-  // - If the result succeeded, this will set |result_| and return.
-  // - If |result| failed with a ValueStore::CORRUPTION error, this will call
-  //   RestoreStorageAndRetry(), and return that result.
-  // - If the |result| failed with a different error, this will set |error_|
-  //   and return.
-  // - |key| will be non-null only when reading a single value.
-  ResponseValue UseReadResult(ValueStore::ReadResult result,
-                              const std::string* key,
-                              ValueStore* storage);
+  // Convert the |result| of a read function to the appropriate response value.
+  // - If the |result| succeeded this will return a response object argument.
+  // - If the |result| failed will return an error object.
+  ResponseValue UseReadResult(ValueStore::ReadResult result);
 
   // Handles the |result| of a write function.
-  // - If the result succeeded, this will set |result_| and return.
-  // - If |result| failed with a ValueStore::CORRUPTION error, this will call
-  //   RestoreStorageAndRetry(), and return that result.
-  // - If the |result| failed with a different error, this will set |error_|
-  //   and return.
-  // - |key| will be non-null only when writing a single value.
-  // This will also send out a change notification, if appropriate.
-  ResponseValue UseWriteResult(ValueStore::WriteResult result,
-                               const std::string* key,
-                               ValueStore* storage);
+  // - If the |result| succeeded this will send out change notification(s), if
+  //   appropriate, and return no arguments.
+  // - If the |result| failed will return an error object.
+  ResponseValue UseWriteResult(ValueStore::WriteResult result);
 
  private:
   // Called via PostTask from Run. Calls RunWithStorage and then
   // SendResponse with its success value.
   void AsyncRunWithStorage(ValueStore* storage);
 
-  // Called if we encounter a ValueStore error. If the error is due to
-  // corruption, tries to restore the ValueStore and re-run the API function.
-  // If the storage cannot be restored or was due to some other error, then sets
-  // error and returns. This also sets the |tried_restoring_storage_| flag to
-  // ensure we don't enter a loop.
-  // - |key| will be non-null only when reading/writing a single value.
-  ResponseValue HandleError(const ValueStore::Status& error,
-                            const std::string* key,
-                            ValueStore* storage);
-
   // The settings namespace the call was for.  For example, SYNC if the API
   // call was chrome.settings.experimental.sync..., LOCAL if .local, etc.
   settings_namespace::Namespace settings_namespace_;
 
-  // A flag indicating whether or not we have tried to restore storage. We
-  // should only ever try once (per API call) in order to avoid entering a loop.
-  bool tried_restoring_storage_;
-
   // Observers, cached so that it's only grabbed from the UI thread.
   scoped_refptr<SettingsObserverList> observers_;
 };
diff --git a/extensions/browser/api/storage/weak_unlimited_settings_storage.cc b/extensions/browser/api/storage/weak_unlimited_settings_storage.cc
index 1089f23..3de934b 100644
--- a/extensions/browser/api/storage/weak_unlimited_settings_storage.cc
+++ b/extensions/browser/api/storage/weak_unlimited_settings_storage.cc
@@ -64,10 +64,4 @@ ValueStore::WriteResult WeakUnlimitedSettingsStorage::Clear() {
   return delegate_->Clear();
 }
 
-bool WeakUnlimitedSettingsStorage::Restore() { return delegate_->Restore(); }
-
-bool WeakUnlimitedSettingsStorage::RestoreKey(const std::string& key) {
-  return delegate_->RestoreKey(key);
-}
-
 }  // namespace extensions
diff --git a/extensions/browser/api/storage/weak_unlimited_settings_storage.h b/extensions/browser/api/storage/weak_unlimited_settings_storage.h
index 9421287..a92c8a1 100644
--- a/extensions/browser/api/storage/weak_unlimited_settings_storage.h
+++ b/extensions/browser/api/storage/weak_unlimited_settings_storage.h
@@ -5,6 +5,9 @@
 #ifndef EXTENSIONS_BROWSER_API_STORAGE_WEAK_UNLIMITED_SETTINGS_STORAGE_H_
 #define EXTENSIONS_BROWSER_API_STORAGE_WEAK_UNLIMITED_SETTINGS_STORAGE_H_
 
+#include <string>
+#include <vector>
+
 #include "base/compiler_specific.h"
 #include "extensions/browser/value_store/value_store.h"
 
@@ -35,8 +38,6 @@ class WeakUnlimitedSettingsStorage : public ValueStore {
   WriteResult Remove(const std::string& key) override;
   WriteResult Remove(const std::vector<std::string>& keys) override;
   WriteResult Clear() override;
-  bool Restore() override;
-  bool RestoreKey(const std::string& key) override;
 
  private:
   // The delegate storage area, NOT OWNED.
diff --git a/extensions/browser/value_store/leveldb_value_store.cc b/extensions/browser/value_store/leveldb_value_store.cc
index 29e9c33..1c42ca5 100644
--- a/extensions/browser/value_store/leveldb_value_store.cc
+++ b/extensions/browser/value_store/leveldb_value_store.cc
@@ -26,6 +26,7 @@ namespace {
 
 const char kInvalidJson[] = "Invalid JSON";
 const char kCannotSerialize[] = "Cannot serialize value to JSON";
+const char kRestoredDuringOpen[] = "Database corruption repaired during open";
 
 // UMA values used when recovering from a corrupted leveldb.
 // Do not change/delete these values as you will break reporting for older
@@ -58,13 +59,30 @@ class ScopedSnapshot {
   DISALLOW_COPY_AND_ASSIGN(ScopedSnapshot);
 };
 
+ValueStore::StatusCode LevelDbToValueStoreStatus(
+    const leveldb::Status& status) {
+  if (status.ok())
+    return ValueStore::OK;
+  if (status.IsCorruption())
+    return ValueStore::CORRUPTION;
+  return ValueStore::OTHER_ERROR;
+}
+
 }  // namespace
 
 LeveldbValueStore::LeveldbValueStore(const std::string& uma_client_name,
                                      const base::FilePath& db_path)
-    : db_path_(db_path), open_histogram_(nullptr), restore_histogram_(nullptr) {
+    : db_path_(db_path),
+      db_unrecoverable_(false),
+      open_histogram_(nullptr),
+      restore_histogram_(nullptr) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
+  open_options_.max_open_files = 0;  // Use minimum.
+  open_options_.create_if_missing = true;
+  open_options_.paranoid_checks = true;
+  open_options_.reuse_logs = leveldb_env::kDefaultLogReuseOptionValue;
+
   // Used in lieu of UMA_HISTOGRAM_ENUMERATION because the histogram name is
   // not a constant.
   open_histogram_ = base::LinearHistogram::FactoryGet(
@@ -112,28 +130,28 @@ size_t LeveldbValueStore::GetBytesInUse() {
 ValueStore::ReadResult LeveldbValueStore::Get(const std::string& key) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  Status open_error = EnsureDbIsOpen();
-  if (!open_error.ok())
-    return MakeReadResult(open_error);
+  Status status = EnsureDbIsOpen();
+  if (!status.ok())
+    return MakeReadResult(status);
 
   scoped_ptr<base::Value> setting;
-  Status error = ReadFromDb(leveldb::ReadOptions(), key, &setting);
-  if (!error.ok())
-    return MakeReadResult(error);
+  status.Merge(ReadFromDb(leveldb::ReadOptions(), key, &setting));
+  if (!status.ok())
+    return MakeReadResult(status);
 
   base::DictionaryValue* settings = new base::DictionaryValue();
   if (setting)
     settings->SetWithoutPathExpansion(key, setting.release());
-  return MakeReadResult(make_scoped_ptr(settings));
+  return MakeReadResult(make_scoped_ptr(settings), status);
 }
 
 ValueStore::ReadResult LeveldbValueStore::Get(
     const std::vector<std::string>& keys) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  Status open_error = EnsureDbIsOpen();
-  if (!open_error.ok())
-    return MakeReadResult(open_error);
+  Status status = EnsureDbIsOpen();
+  if (!status.ok())
+    return MakeReadResult(status);
 
   leveldb::ReadOptions options;
   scoped_ptr<base::DictionaryValue> settings(new base::DictionaryValue());
@@ -145,22 +163,22 @@ ValueStore::ReadResult LeveldbValueStore::Get(
   for (std::vector<std::string>::const_iterator it = keys.begin();
       it != keys.end(); ++it) {
     scoped_ptr<base::Value> setting;
-    Status error = ReadFromDb(options, *it, &setting);
-    if (!error.ok())
-      return MakeReadResult(error);
+    status.Merge(ReadFromDb(options, *it, &setting));
+    if (!status.ok())
+      return MakeReadResult(status);
     if (setting)
       settings->SetWithoutPathExpansion(*it, setting.release());
   }
 
-  return MakeReadResult(settings.Pass());
+  return MakeReadResult(settings.Pass(), status);
 }
 
 ValueStore::ReadResult LeveldbValueStore::Get() {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  Status open_error = EnsureDbIsOpen();
-  if (!open_error.ok())
-    return MakeReadResult(open_error);
+  Status status = EnsureDbIsOpen();
+  if (!status.ok())
+    return MakeReadResult(status);
 
   base::JSONReader json_reader;
   leveldb::ReadOptions options = leveldb::ReadOptions();
@@ -175,63 +193,69 @@ ValueStore::ReadResult LeveldbValueStore::Get() {
     std::string key = it->key().ToString();
     scoped_ptr<base::Value> value =
         json_reader.ReadToValue(it->value().ToString());
-    if (!value)
-      return MakeReadResult(Status(CORRUPTION, kInvalidJson));
+    if (!value) {
+      return MakeReadResult(Status(
+          CORRUPTION,
+          Delete(key).ok() ? RESTORE_REPAIR_SUCCESS : RESTORE_DELETE_FAILURE,
+          kInvalidJson));
+    }
     settings->SetWithoutPathExpansion(key, value.Pass());
   }
 
   if (it->status().IsNotFound()) {
     NOTREACHED() << "IsNotFound() but iterating over all keys?!";
-    return MakeReadResult(settings.Pass());
+    return MakeReadResult(settings.Pass(), status);
   }
 
-  if (!it->status().ok())
-    return MakeReadResult(ToValueStoreError(it->status()));
+  if (!it->status().ok()) {
+    status.Merge(ToValueStoreError(it->status()));
+    return MakeReadResult(status);
+  }
 
-  return MakeReadResult(settings.Pass());
+  return MakeReadResult(settings.Pass(), status);
 }
 
 ValueStore::WriteResult LeveldbValueStore::Set(
     WriteOptions options, const std::string& key, const base::Value& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  Status open_error = EnsureDbIsOpen();
-  if (!open_error.ok())
-    return MakeWriteResult(open_error);
+  Status status = EnsureDbIsOpen();
+  if (!status.ok())
+    return MakeWriteResult(status);
 
   leveldb::WriteBatch batch;
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
-  Status batch_error = AddToBatch(options, key, value, &batch, changes.get());
-  if (!batch_error.ok())
-    return MakeWriteResult(batch_error);
+  status.Merge(AddToBatch(options, key, value, &batch, changes.get()));
+  if (!status.ok())
+    return MakeWriteResult(status);
 
-  Status write_error = WriteToDb(&batch);
-  return write_error.ok() ? MakeWriteResult(changes.Pass())
-                          : MakeWriteResult(write_error);
+  status.Merge(WriteToDb(&batch));
+  return status.ok() ? MakeWriteResult(changes.Pass(), status)
+                     : MakeWriteResult(status);
 }
 
 ValueStore::WriteResult LeveldbValueStore::Set(
     WriteOptions options, const base::DictionaryValue& settings) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  Status open_error = EnsureDbIsOpen();
-  if (!open_error.ok())
-    return MakeWriteResult(open_error);
+  Status status = EnsureDbIsOpen();
+  if (!status.ok())
+    return MakeWriteResult(status);
 
   leveldb::WriteBatch batch;
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
 
   for (base::DictionaryValue::Iterator it(settings);
        !it.IsAtEnd(); it.Advance()) {
-    Status batch_error =
-        AddToBatch(options, it.key(), it.value(), &batch, changes.get());
-    if (!batch_error.ok())
-      return MakeWriteResult(batch_error);
+    status.Merge(
+        AddToBatch(options, it.key(), it.value(), &batch, changes.get()));
+    if (!status.ok())
+      return MakeWriteResult(status);
   }
 
-  Status write_error = WriteToDb(&batch);
-  return write_error.ok() ? MakeWriteResult(changes.Pass())
-                          : MakeWriteResult(write_error);
+  status.Merge(WriteToDb(&batch));
+  return status.ok() ? MakeWriteResult(changes.Pass(), status)
+                     : MakeWriteResult(status);
 }
 
 ValueStore::WriteResult LeveldbValueStore::Remove(const std::string& key) {
@@ -243,9 +267,9 @@ ValueStore::WriteResult LeveldbValueStore::Remove(
     const std::vector<std::string>& keys) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  Status open_error = EnsureDbIsOpen();
-  if (!open_error.ok())
-    return MakeWriteResult(open_error);
+  Status status = EnsureDbIsOpen();
+  if (!status.ok())
+    return MakeWriteResult(status);
 
   leveldb::WriteBatch batch;
   scoped_ptr<ValueStoreChangeList> changes(new ValueStoreChangeList());
@@ -253,9 +277,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;
-    Status read_error = ReadFromDb(leveldb::ReadOptions(), *it, &old_value);
-    if (!read_error.ok())
-      return MakeWriteResult(read_error);
+    status.Merge(ReadFromDb(leveldb::ReadOptions(), *it, &old_value));
+    if (!status.ok())
+      return MakeWriteResult(status);
 
     if (old_value) {
       changes->push_back(ValueStoreChange(*it, old_value.release(), NULL));
@@ -263,10 +287,12 @@ ValueStore::WriteResult LeveldbValueStore::Remove(
     }
   }
 
-  leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch);
-  if (!status.ok() && !status.IsNotFound())
-    return MakeWriteResult(ToValueStoreError(status));
-  return MakeWriteResult(changes.Pass());
+  leveldb::Status ldb_status = db_->Write(leveldb::WriteOptions(), &batch);
+  if (!ldb_status.ok() && !ldb_status.IsNotFound()) {
+    status.Merge(ToValueStoreError(ldb_status));
+    return MakeWriteResult(status);
+  }
+  return MakeWriteResult(changes.Pass(), status);
 }
 
 ValueStore::WriteResult LeveldbValueStore::Clear() {
@@ -283,56 +309,11 @@ ValueStore::WriteResult LeveldbValueStore::Clear() {
     std::string next_key = base::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));
+    changes->push_back(ValueStoreChange(next_key, next_value.release(), NULL));
   }
 
   DeleteDbFile();
-  return MakeWriteResult(changes.Pass());
-}
-
-bool LeveldbValueStore::Restore() {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-
-  // Possible to have a corrupted open database, so first close it.
-  db_.reset();
-
-  leveldb::Options options;
-  options.create_if_missing = true;
-  options.paranoid_checks = true;
-
-  // Repair can drop an unbounded number of leveldb tables (key/value sets)
-  leveldb::Status status = leveldb::RepairDB(db_path_.AsUTF8Unsafe(), options);
-  if (status.ok()) {
-    restore_histogram_->Add(LEVELDB_RESTORE_REPAIR_SUCCESS);
-    return true;
-  }
-
-  if (DeleteDbFile()) {
-    restore_histogram_->Add(LEVELDB_RESTORE_DELETE_SUCCESS);
-    return true;
-  }
-
-  restore_histogram_->Add(LEVELDB_RESTORE_DELETE_FAILURE);
-  return false;
-}
-
-bool LeveldbValueStore::RestoreKey(const std::string& key) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-
-  ReadResult result = Get(key);
-  if (result->status().IsCorrupted()) {
-    leveldb::WriteBatch batch;
-    batch.Delete(key);
-    ValueStore::Status error = WriteToDb(&batch);
-    // If we can't delete the key, the restore failed.
-    if (!error.ok())
-      return false;
-    result = Get(key);
-  }
-
-  // The restore succeeded if there is no corruption error.
-  return !result->status().IsCorrupted();
+  return MakeWriteResult(changes.Pass(), read_result->status());
 }
 
 bool LeveldbValueStore::WriteToDbForTest(leveldb::WriteBatch* batch) {
@@ -371,34 +352,120 @@ bool LeveldbValueStore::OnMemoryDump(
   return true;
 }
 
+ValueStore::BackingStoreRestoreStatus LeveldbValueStore::LogRestoreStatus(
+    BackingStoreRestoreStatus restore_status) {
+  switch (restore_status) {
+    case RESTORE_NONE:
+      NOTREACHED();
+      break;
+    case RESTORE_DELETE_SUCCESS:
+      restore_histogram_->Add(LEVELDB_RESTORE_DELETE_SUCCESS);
+      break;
+    case RESTORE_DELETE_FAILURE:
+      restore_histogram_->Add(LEVELDB_RESTORE_DELETE_FAILURE);
+      break;
+    case RESTORE_REPAIR_SUCCESS:
+      restore_histogram_->Add(LEVELDB_RESTORE_REPAIR_SUCCESS);
+      break;
+  }
+  return restore_status;
+}
+
+ValueStore::BackingStoreRestoreStatus LeveldbValueStore::FixCorruption(
+    const std::string* key) {
+  leveldb::Status s;
+  if (key && db_) {
+    s = Delete(*key);
+    // Deleting involves writing to the log, so it's possible to have a
+    // perfectly OK database but still have a delete fail.
+    if (s.ok())
+      return LogRestoreStatus(RESTORE_REPAIR_SUCCESS);
+    else if (s.IsIOError())
+      return LogRestoreStatus(RESTORE_DELETE_FAILURE);
+    // Any other kind of failure triggers a db repair.
+  }
+
+  // Make sure database is closed.
+  db_.reset();
+
+  // First try the less lossy repair.
+  BackingStoreRestoreStatus restore_status = RESTORE_NONE;
+
+  leveldb::Options repair_options;
+  repair_options.create_if_missing = true;
+  repair_options.paranoid_checks = true;
+
+  // RepairDB can drop an unbounded number of leveldb tables (key/value sets).
+  s = leveldb::RepairDB(db_path_.AsUTF8Unsafe(), repair_options);
+
+  leveldb::DB* db = nullptr;
+  if (s.ok()) {
+    restore_status = RESTORE_REPAIR_SUCCESS;
+    s = leveldb::DB::Open(open_options_, db_path_.AsUTF8Unsafe(), &db);
+  }
+
+  if (!s.ok()) {
+    if (DeleteDbFile()) {
+      restore_status = RESTORE_DELETE_SUCCESS;
+      s = leveldb::DB::Open(open_options_, db_path_.AsUTF8Unsafe(), &db);
+    } else {
+      restore_status = RESTORE_DELETE_FAILURE;
+    }
+  }
+
+  if (s.ok())
+    db_.reset(db);
+  else
+    db_unrecoverable_ = true;
+
+  if (s.ok() && key) {
+    s = Delete(*key);
+    if (s.ok()) {
+      restore_status = RESTORE_REPAIR_SUCCESS;
+    } else if (s.IsIOError()) {
+      restore_status = RESTORE_DELETE_FAILURE;
+    } else {
+      db_.reset(db);
+      if (!DeleteDbFile())
+        db_unrecoverable_ = true;
+      restore_status = RESTORE_DELETE_FAILURE;
+    }
+  }
+
+  // Only log for the final and most extreme form of database restoration.
+  LogRestoreStatus(restore_status);
+
+  return restore_status;
+}
+
 ValueStore::Status LeveldbValueStore::EnsureDbIsOpen() {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   if (db_)
     return Status();
 
-  leveldb::Options options;
-  options.max_open_files = 0;  // Use minimum.
-  options.create_if_missing = true;
-  options.reuse_logs = leveldb_env::kDefaultLogReuseOptionValue;
+  if (db_unrecoverable_) {
+    return ValueStore::Status(ValueStore::CORRUPTION,
+                              ValueStore::RESTORE_DELETE_FAILURE,
+                              "Database corrupted");
+  }
 
   leveldb::DB* db = NULL;
-  leveldb::Status status =
-      leveldb::DB::Open(options, db_path_.AsUTF8Unsafe(), &db);
-  if (open_histogram_)
-    open_histogram_->Add(leveldb_env::GetLevelDBStatusUMAValue(status));
-  if (status.IsCorruption()) {
-    // Returning a corruption error should result in Restore() being called.
-    // However, since once a leveldb becomes corrupt it's unusable without
-    // some kind of repair or delete, so do that right now.
-    Restore();
+  leveldb::Status ldb_status =
+      leveldb::DB::Open(open_options_, db_path_.AsUTF8Unsafe(), &db);
+  open_histogram_->Add(leveldb_env::GetLevelDBStatusUMAValue(ldb_status));
+  Status status = ToValueStoreError(ldb_status);
+  if (ldb_status.ok()) {
+    db_.reset(db);
+  } else if (ldb_status.IsCorruption()) {
+    status.restore_status = FixCorruption(nullptr);
+    if (status.restore_status != RESTORE_DELETE_FAILURE) {
+      status.code = OK;
+      status.message = kRestoredDuringOpen;
+    }
   }
-  if (!status.ok())
-    return ToValueStoreError(status);
 
-  CHECK(db);
-  db_.reset(db);
-  return Status();
+  return status;
 }
 
 ValueStore::Status LeveldbValueStore::ReadFromDb(
@@ -422,7 +489,7 @@ ValueStore::Status LeveldbValueStore::ReadFromDb(
 
   scoped_ptr<base::Value> value = base::JSONReader().ReadToValue(value_as_json);
   if (!value)
-    return Status(CORRUPTION, kInvalidJson);
+    return Status(CORRUPTION, FixCorruption(&key), kInvalidJson);
 
   *setting = value.Pass();
   return Status();
@@ -438,11 +505,9 @@ ValueStore::Status LeveldbValueStore::AddToBatch(
 
   if (!(options & NO_GENERATE_CHANGES)) {
     scoped_ptr<base::Value> old_value;
-    Status read_error = ReadFromDb(leveldb::ReadOptions(), key, &old_value);
-    if (read_error.IsCorrupted())
-      batch->Delete(key);
-    else if (!read_error.ok())
-      return read_error;
+    Status status = ReadFromDb(leveldb::ReadOptions(), key, &old_value);
+    if (!status.ok())
+      return status;
     if (!old_value || !old_value->Equals(&value)) {
       changes->push_back(
           ValueStoreChange(key, old_value.release(), value.DeepCopy()));
@@ -463,7 +528,7 @@ ValueStore::Status LeveldbValueStore::AddToBatch(
 
 ValueStore::Status LeveldbValueStore::WriteToDb(leveldb::WriteBatch* batch) {
   leveldb::Status status = db_->Write(leveldb::WriteOptions(), batch);
-  return status.ok() ? Status() : ToValueStoreError(status);
+  return ToValueStoreError(status);
 }
 
 bool LeveldbValueStore::IsEmpty() {
@@ -479,6 +544,16 @@ bool LeveldbValueStore::IsEmpty() {
   return is_empty;
 }
 
+leveldb::Status LeveldbValueStore::Delete(const std::string& key) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  DCHECK(db_.get());
+
+  leveldb::WriteBatch batch;
+  batch.Delete(key);
+
+  return db_->Write(leveldb::WriteOptions(), &batch);
+}
+
 bool LeveldbValueStore::DeleteDbFile() {
   db_.reset();  // release any lock on the directory
   if (!base::DeleteFile(db_path_, true /* recursive */)) {
@@ -491,7 +566,6 @@ bool LeveldbValueStore::DeleteDbFile() {
 
 ValueStore::Status LeveldbValueStore::ToValueStoreError(
     const leveldb::Status& status) {
-  CHECK(!status.ok());
   CHECK(!status.IsNotFound());  // not an error
 
   std::string message = status.ToString();
@@ -500,5 +574,5 @@ ValueStore::Status LeveldbValueStore::ToValueStoreError(
   base::ReplaceSubstringsAfterOffset(
       &message, 0u, db_path_.AsUTF8Unsafe(), "...");
 
-  return Status(CORRUPTION, message);
+  return Status(LevelDbToValueStoreStatus(status), message);
 }
diff --git a/extensions/browser/value_store/leveldb_value_store.h b/extensions/browser/value_store/leveldb_value_store.h
index 81956fb..6b34e8a 100644
--- a/extensions/browser/value_store/leveldb_value_store.h
+++ b/extensions/browser/value_store/leveldb_value_store.h
@@ -51,8 +51,6 @@ class LeveldbValueStore : public ValueStore,
   WriteResult Remove(const std::string& key) override;
   WriteResult Remove(const std::vector<std::string>& keys) override;
   WriteResult Clear() override;
-  bool Restore() override;
-  bool RestoreKey(const std::string& key) override;
 
   // Write directly to the backing levelDB. Only used for testing to cause
   // corruption in the database.
@@ -63,6 +61,15 @@ class LeveldbValueStore : public ValueStore,
                     base::trace_event::ProcessMemoryDump* pmd) override;
 
  private:
+  // Fix the |key| or database. If |key| is not null and the database is open
+  // then the key will be deleted. Otherwise the database will be repaired, and
+  // failing that will be deleted.
+  BackingStoreRestoreStatus FixCorruption(const std::string* key);
+
+  // Log, to UMA, the status of an attempt to restore a database.
+  BackingStoreRestoreStatus LogRestoreStatus(
+      BackingStoreRestoreStatus restore_status);
+
   // Tries to open the database if it hasn't been opened already.
   ValueStore::Status EnsureDbIsOpen();
 
@@ -83,9 +90,12 @@ class LeveldbValueStore : public ValueStore,
   // Commits the changes in |batch| to the database.
   ValueStore::Status WriteToDb(leveldb::WriteBatch* batch);
 
-  // Converts an error leveldb::Status to a ValueStore::Status.
+  // Converts an error leveldb::Status to a ValueStore::Error.
   ValueStore::Status ToValueStoreError(const leveldb::Status& status);
 
+  // Delete a value (identified by |key|) from the value store.
+  leveldb::Status Delete(const std::string& key);
+
   // Removes the on-disk database at |db_path_|. Any file system locks should
   // be released before calling this method.
   bool DeleteDbFile();
@@ -95,9 +105,12 @@ class LeveldbValueStore : public ValueStore,
 
   // The location of the leveldb backend.
   const base::FilePath db_path_;
+  leveldb::Options open_options_;
 
   // leveldb backend.
   scoped_ptr<leveldb::DB> db_;
+  // Database is corrupt - restoration failed.
+  bool db_unrecoverable_;
   base::HistogramBase* open_histogram_;
   base::HistogramBase* restore_histogram_;
 
diff --git a/extensions/browser/value_store/leveldb_value_store_unittest.cc b/extensions/browser/value_store/leveldb_value_store_unittest.cc
index d5a28ac..bf83706 100644
--- a/extensions/browser/value_store/leveldb_value_store_unittest.cc
+++ b/extensions/browser/value_store/leveldb_value_store_unittest.cc
@@ -39,18 +39,20 @@ class LeveldbValueStoreUnitTest : public testing::Test {
  protected:
   void SetUp() override {
     ASSERT_TRUE(database_dir_.CreateUniqueTempDir());
-    OpenStore();
+    CreateStore();
     ASSERT_TRUE(store_->Get()->status().ok());
   }
 
   void TearDown() override {
+    if (!store_)
+      return;
     store_->Clear();
-    store_.reset();
+    CloseStore();
   }
 
   void CloseStore() { store_.reset(); }
 
-  void OpenStore() {
+  void CreateStore() {
     store_.reset(
         new LeveldbValueStore(kDatabaseUMAClientName, database_path()));
   }
@@ -82,15 +84,15 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreKeyTest) {
   batch.Put(kCorruptKey, "[{(.*+\"\'\\");
   ASSERT_TRUE(store()->WriteToDbForTest(&batch));
 
-  // Verify corruption.
+  // Verify corruption (the first Get will return corruption).
   ValueStore::ReadResult result = store()->Get(kCorruptKey);
   ASSERT_FALSE(result->status().ok());
   ASSERT_EQ(ValueStore::CORRUPTION, result->status().code);
 
-  // Restore and verify.
-  ASSERT_TRUE(store()->RestoreKey(kCorruptKey));
+  // Verify restored (was deleted in the first Get).
   result = store()->Get(kCorruptKey);
-  EXPECT_TRUE(result->status().ok());
+  EXPECT_TRUE(result->status().ok()) << "Get result not OK: "
+                                     << result->status().message;
   EXPECT_TRUE(result->settings().empty());
 
   // Verify that the valid pair is still present.
@@ -126,18 +128,19 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreDoesMinimumNecessary) {
   batch.Put(kCorruptKey2, kCorruptValue);
   ASSERT_TRUE(store()->WriteToDbForTest(&batch));
 
-  // Verify that we broke it, and then fix it.
+  // Verify that we broke it and that it was repaired by the value store.
   ValueStore::ReadResult result = store()->Get();
   ASSERT_FALSE(result->status().ok());
   ASSERT_EQ(ValueStore::CORRUPTION, result->status().code);
-
-  ASSERT_TRUE(store()->Restore());
+  ASSERT_EQ(ValueStore::RESTORE_REPAIR_SUCCESS,
+            result->status().restore_status);
 
   // We should still have all valid pairs present in the database.
   std::string value_string;
   for (size_t i = 0; i < kNotCorruptKeysSize; ++i) {
     result = store()->Get(kNotCorruptKeys[i]);
     EXPECT_TRUE(result->status().ok());
+    ASSERT_EQ(ValueStore::RESTORE_NONE, result->status().restore_status);
     EXPECT_TRUE(result->settings().HasKey(kNotCorruptKeys[i]));
     EXPECT_TRUE(
         result->settings().GetString(kNotCorruptKeys[i], &value_string));
@@ -172,19 +175,14 @@ TEST_F(LeveldbValueStoreUnitTest, RestoreFullDatabase) {
   for (base::FilePath file = enumerator.Next(); !file.empty();
        file = enumerator.Next()) {
     // WriteFile() failure is a result of -1.
-    ASSERT_NE(base::WriteFile(file, kLolCats.c_str(), kLolCats.length()),
-              -1);
+    ASSERT_NE(base::WriteFile(file, kLolCats.c_str(), kLolCats.length()), -1);
   }
-  OpenStore();
+  CreateStore();
 
-  // We should definitely have an error.
+  // We couldn't recover anything, but we should be in a sane state again.
   ValueStore::ReadResult result = store()->Get();
-  ASSERT_FALSE(result->status().ok());
-  ASSERT_EQ(ValueStore::CORRUPTION, result->status().code);
-
-  ASSERT_TRUE(store()->Restore());
-  result = store()->Get();
+  ASSERT_EQ(ValueStore::RESTORE_REPAIR_SUCCESS,
+            result->status().restore_status);
   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 73536f3..500e6e8 100644
--- a/extensions/browser/value_store/testing_value_store.cc
+++ b/extensions/browser/value_store/testing_value_store.cc
@@ -57,14 +57,14 @@ ValueStore::ReadResult TestingValueStore::Get(
       settings->SetWithoutPathExpansion(*it, value->DeepCopy());
     }
   }
-  return MakeReadResult(make_scoped_ptr(settings));
+  return MakeReadResult(make_scoped_ptr(settings), status_);
 }
 
 ValueStore::ReadResult TestingValueStore::Get() {
   read_count_++;
   if (!status_.ok())
     return MakeReadResult(status_);
-  return MakeReadResult(make_scoped_ptr(storage_.DeepCopy()));
+  return MakeReadResult(make_scoped_ptr(storage_.DeepCopy()), status_);
 }
 
 ValueStore::WriteResult TestingValueStore::Set(
@@ -94,7 +94,7 @@ ValueStore::WriteResult TestingValueStore::Set(
       storage_.SetWithoutPathExpansion(it.key(), it.value().DeepCopy());
     }
   }
-  return MakeWriteResult(changes.Pass());
+  return MakeWriteResult(changes.Pass(), status_);
 }
 
 ValueStore::WriteResult TestingValueStore::Remove(const std::string& key) {
@@ -115,7 +115,7 @@ ValueStore::WriteResult TestingValueStore::Remove(
       changes->push_back(ValueStoreChange(*it, old_value.release(), NULL));
     }
   }
-  return MakeWriteResult(changes.Pass());
+  return MakeWriteResult(changes.Pass(), status_);
 }
 
 ValueStore::WriteResult TestingValueStore::Clear() {
@@ -126,11 +126,3 @@ ValueStore::WriteResult TestingValueStore::Clear() {
   }
   return Remove(keys);
 }
-
-bool TestingValueStore::Restore() {
-  return true;
-}
-
-bool TestingValueStore::RestoreKey(const std::string& key) {
-  return true;
-}
diff --git a/extensions/browser/value_store/testing_value_store.h b/extensions/browser/value_store/testing_value_store.h
index 9559649..f439694 100644
--- a/extensions/browser/value_store/testing_value_store.h
+++ b/extensions/browser/value_store/testing_value_store.h
@@ -45,10 +45,6 @@ class TestingValueStore : public ValueStore {
   WriteResult Remove(const std::string& key) override;
   WriteResult Remove(const std::vector<std::string>& keys) override;
   WriteResult Clear() override;
-  // TestingValueStores can't get corrupted (they're all in-memory), so these
-  // just return true.
-  bool Restore() override;
-  bool RestoreKey(const std::string& key) override;
 
  private:
   base::DictionaryValue storage_;
diff --git a/extensions/browser/value_store/value_store.cc b/extensions/browser/value_store/value_store.cc
index 218956c..88ebe2a 100644
--- a/extensions/browser/value_store/value_store.cc
+++ b/extensions/browser/value_store/value_store.cc
@@ -8,17 +8,33 @@
 
 // Implementation of Status.
 
-ValueStore::Status::Status() : code(OK) {}
+ValueStore::Status::Status() : code(OK), restore_status(RESTORE_NONE) {}
 
 ValueStore::Status::Status(StatusCode code, const std::string& message)
-    : code(code), message(message) {}
+    : Status(code, RESTORE_NONE, message) {}
+
+ValueStore::Status::Status(StatusCode code,
+                           BackingStoreRestoreStatus restore_status,
+                           const std::string& message)
+    : code(code), restore_status(restore_status), message(message) {}
 
 ValueStore::Status::~Status() {}
 
+void ValueStore::Status::Merge(const Status& status) {
+  if (code == OK)
+    code = status.code;
+  if (message.empty() && !status.message.empty())
+    message = status.message;
+  if (restore_status == RESTORE_NONE)
+    restore_status = status.restore_status;
+}
+
 // Implementation of ReadResultType.
 
 ValueStore::ReadResultType::ReadResultType(
-    scoped_ptr<base::DictionaryValue> settings) : settings_(settings.Pass()) {
+    scoped_ptr<base::DictionaryValue> settings,
+    const Status& status)
+    : settings_(settings.Pass()), status_(status) {
   CHECK(settings_);
 }
 
@@ -30,8 +46,9 @@ ValueStore::ReadResultType::~ReadResultType() {}
 // Implementation of WriteResultType.
 
 ValueStore::WriteResultType::WriteResultType(
-    scoped_ptr<ValueStoreChangeList> changes)
-    : changes_(changes.Pass()) {
+    scoped_ptr<ValueStoreChangeList> changes,
+    const Status& status)
+    : changes_(changes.Pass()), status_(status) {
   CHECK(changes_);
 }
 
diff --git a/extensions/browser/value_store/value_store.h b/extensions/browser/value_store/value_store.h
index 9ac652a..ca9d48c 100644
--- a/extensions/browser/value_store/value_store.h
+++ b/extensions/browser/value_store/value_store.h
@@ -39,9 +39,23 @@ class ValueStore {
     OTHER_ERROR,
   };
 
+  enum BackingStoreRestoreStatus {
+    // No restore attempted.
+    RESTORE_NONE,
+    // Corrupted backing store successfully deleted.
+    RESTORE_DELETE_SUCCESS,
+    // Corrupted backing store cannot be deleted.
+    RESTORE_DELETE_FAILURE,
+    // Corrupted backing store successfully repaired.
+    RESTORE_REPAIR_SUCCESS,
+  };
+
   // The status (result) of an operation on a ValueStore.
   struct Status {
     Status();
+    Status(StatusCode code,
+           BackingStoreRestoreStatus restore_status,
+           const std::string& message);
     Status(StatusCode code, const std::string& message);
     ~Status();
 
@@ -49,9 +63,16 @@ class ValueStore {
 
     bool IsCorrupted() const { return code == CORRUPTION; }
 
+    // Merge |status| into this object. Any members (either |code|,
+    // |restore_status|, or |message| in |status| will be used, but only if this
+    // object's members are at their default value.
+    void Merge(const Status& status);
+
     // The status code.
     StatusCode code;
 
+    BackingStoreRestoreStatus restore_status;
+
     // Message associated with the status (error) if there is one.
     std::string message;
   };
@@ -59,7 +80,8 @@ class ValueStore {
   // The result of a read operation (Get).
   class ReadResultType {
    public:
-    explicit ReadResultType(scoped_ptr<base::DictionaryValue> settings);
+    ReadResultType(scoped_ptr<base::DictionaryValue> settings,
+                   const Status& status);
     explicit ReadResultType(const Status& status);
     ~ReadResultType();
 
@@ -86,7 +108,8 @@ class ValueStore {
   // The result of a write operation (Set/Remove/Clear).
   class WriteResultType {
    public:
-    explicit WriteResultType(scoped_ptr<ValueStoreChangeList> changes);
+    WriteResultType(scoped_ptr<ValueStoreChangeList> changes,
+                    const Status& status);
     explicit WriteResultType(const Status& status);
     ~WriteResultType();
 
@@ -122,17 +145,17 @@ class ValueStore {
   virtual ~ValueStore() {}
 
   // Helpers for making a Read/WriteResult.
-  template<typename T>
-  static ReadResult MakeReadResult(scoped_ptr<T> arg) {
-    return ReadResult(new ReadResultType(arg.Pass()));
+  template <typename T>
+  static ReadResult MakeReadResult(scoped_ptr<T> arg, const Status& status) {
+    return ReadResult(new ReadResultType(arg.Pass(), status));
   }
   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()));
+  template <typename T>
+  static WriteResult MakeWriteResult(scoped_ptr<T> arg, const Status& status) {
+    return WriteResult(new WriteResultType(arg.Pass(), status));
   }
   static WriteResult MakeWriteResult(const Status& status) {
     return WriteResult(new WriteResultType(status));
@@ -177,27 +200,6 @@ class ValueStore {
 
   // Clears the storage.
   virtual WriteResult Clear() = 0;
-
-  // In the event of corruption, the ValueStore should be able to restore
-  // itself. This means deleting local corrupted files. If only a few keys are
-  // corrupted, then some of the database may be saved. If the full database is
-  // corrupted, this will erase it in its entirety.
-  // Returns true on success, false on failure. The only way this will fail is
-  // if we also cannot delete the database file.
-  // Note: This method may be expensive; some implementations may need to read
-  // the entire database to restore. Use sparingly.
-  // Note: This method (and the following RestoreKey()) are rude, and do not
-  // make any logs, track changes, or other generally polite things. Please do
-  // not use these as substitutes for Clear() and Remove().
-  virtual bool Restore() = 0;
-
-  // Similar to Restore(), but for only a particular key. If the key is corrupt,
-  // this will forcefully remove the key. It does not look at the database on
-  // the whole, which makes it faster, but does not guarantee there is no
-  // additional corruption.
-  // Returns true on success, and false on failure. If false, the next step is
-  // probably to Restore() the whole database.
-  virtual bool RestoreKey(const std::string& key) = 0;
 };
 
 #endif  // EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_H_