Add a Restore() method to ValueStore and make StorageAPI use it

chrome/browser/extensions/api/storage/storage_api_unittest.cc

Index: chrome/browser/extensions/api/storage/storage_api_unittest.cc
diff --git a/chrome/browser/extensions/api/storage/storage_api_unittest.cc b/chrome/browser/extensions/api/storage/storage_api_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ddad8322674ce5e5d69650ca060dac1e22b8bb27
--- /dev/null
+++ b/chrome/browser/extensions/api/storage/storage_api_unittest.cc
@@ -0,0 +1,111 @@
+// 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 "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/extensions/api/storage/settings_storage_quota_enforcer.h"
+#include "chrome/browser/extensions/api/storage/settings_test_util.h"
+#include "chrome/browser/extensions/api/storage/storage_api.h"
+#include "chrome/browser/extensions/extension_api_unittest.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/test_extension_system.h"
+#include "chrome/browser/value_store/leveldb_value_store.h"
+#include "chrome/browser/value_store/value_store.h"
+#include "extensions/browser/event_router.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/common/id_util.h"
+#include "extensions/common/manifest.h"
+#include "extensions/common/test_util.h"
+#include "third_party/leveldatabase/src/include/leveldb/db.h"
+#include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
+
+namespace extensions {
+
+class StorageApiUnittest : public ExtensionApiUnittest {
+ public:
+  virtual void SetUp() OVERRIDE {
+    ExtensionApiUnittest::SetUp();
+    TestExtensionSystem* extension_system =
+        static_cast<TestExtensionSystem*>(ExtensionSystem::Get(profile()));
+    ExtensionService* extension_service = extension_system->extension_service();
+    if (!extension_service) {
+      extension_service = extension_system->CreateExtensionService(
+          CommandLine::ForCurrentProcess(), base::FilePath(), false);
+      ASSERT_TRUE(extension_service);
+    }
+
+    if (!extension_system->event_router()) {
+      extension_system->SetEventRouter(scoped_ptr<EventRouter>(
+          new EventRouter(profile(), ExtensionPrefs::Get(profile()))));
+    }
+
+    scoped_refptr<Extension> extension =
+        test_util::CreateExtensionWithID(id_util::GenerateId("ext"));
+    set_extension(extension);
+    extension_service->AddExtension(extension.get());
+  }
+
+ protected:
+  // Runs the storage.set() API function with local storage.
+  void RunSetFunction(const std::string& key, const std::string& value) {
+    RunFunction(
+        new StorageStorageAreaSetFunction(),
+        base::StringPrintf(
+            "[\"local\", {\"%s\": \"%s\"}]", key.c_str(), value.c_str()));
+  }
+
+  // Runs the storage.get() API function with the local storage, and populates
+  // |value| with the string result. Returns true on success, and false on
+  // failure.
+  bool RunGetFunction(const std::string& key, std::string* value) {

[not at google - send to devlin, 2014/02/19 21:25:29]

return a testing::AssertionResult here. It would look something like

testing::AssertionResult RunGetFunction(...) {
  scoped_ptr<base::Value> result = ...;
  base::DictionaryValue* dict = NULL;
  if (!result->GetAsDictionary(&dict))
    return testing::AssertionFailure() << result << " was not a dictionary";
  if (!dict->GetString(key, value))
    return testing::AssertionFailure() << dict << " did not contain key \"" <<
key << "\"";
  return testing::AssertionSuccess();

testing::AssertionResults are super cool.

[Devlin, 2014/02/19 23:12:28]

Done.

+    scoped_ptr<base::Value> result = RunFunctionAndReturnValue(
+        new StorageStorageAreaGetFunction(),
+        base::StringPrintf("[\"local\", \"%s\"]", key.c_str()));
+    base::DictionaryValue* dict = NULL;
+    if (!result->GetAsDictionary(&dict) || !dict->GetString(key, value))
+      return false;
+    return true;
+  }
+};
+
+TEST_F(StorageApiUnittest, RestoreCorruptedStorage) {
+  const char kKey[] = "key";
+  const char kValue[] = "value";
+  std::string result;
+
+  // Do a simple set/get combo to make sure the API works.
+  RunSetFunction(kKey, kValue);
+  EXPECT_TRUE(RunGetFunction(kKey, &result));
+  EXPECT_EQ(kValue, result);
+
+  // Corrupt the store. This is not as pretty as ideal, because we use knowledge
+  // of the underlying structure, but there's no real good way to corrupt a
+  // store other than directly modifying the files.
+  ValueStore* store =
+      settings_test_util::GetStorage(extension()->id(),
+                                     settings_namespace::LOCAL,
+                                     ExtensionSystem::Get(profile())
+                                         ->extension_service()
+                                         ->settings_frontend());
+  ASSERT_TRUE(store);
+  SettingsStorageQuotaEnforcer* quota_store =
+      static_cast<SettingsStorageQuotaEnforcer*>(store);
+  LeveldbValueStore* leveldb_store =
+      static_cast<LeveldbValueStore*>(quota_store->get_delegate_for_test());

[not at google - send to devlin, 2014/02/19 21:25:29]

this does make me very uneasy given there is no safety whatsoever here and there
are 2 assumptions about structure.

as we talked about in person, a safer way would be to directly corrupt the
on-disk database but that would require making sure it's closed after setting
those values, and the test infrastructure may not be factored in a way that
supports it.

also, it would be nice to test the full Restore end-to-end but I think that
would block on the database being closed (since you'd want to do what you do
with the leveldb storage tests and delete random files).

[Devlin, 2014/02/19 23:12:28]

Per long in-person conversation, there's no pretty way of doing this.  This will
do until we clean more of this up.

+  leveldb::WriteBatch batch;
+  batch.Put(kKey, "[{(.*+\"\'\\");
+  EXPECT_TRUE(leveldb_store->WriteToDbForTest(&batch));
+  EXPECT_TRUE(leveldb_store->Get(kKey)->IsCorrupted());
+
+  // Running another set should end up working (even though it will restore the
+  // store behind the scenes).
+  RunSetFunction(kKey, kValue);
+  EXPECT_TRUE(RunGetFunction(kKey, &result));
+  EXPECT_EQ(kValue, result);
+}
+
+}  // namespace extensions