Unrevert 141514: Remove CachingValueStore in favor of an async ValueStoreFrontend.

After actually seeing the use cases for the value store in extension code, I
realized that an async version would be more appropriate. This one will use
less memory as well, since we won't need to keep the cache in memory.

BUG=123366,132203
TEST=no

Changes: Fixed a use-after-free bug.

Review URL: https://chromiumcodereview.appspot.com/10533116

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141745 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 230 insertions, 279 deletions

diff --git a/chrome/browser/value_store/caching_value_store.cc b/chrome/browser/value_store/caching_value_store.cc
deleted file mode 100644
index d0d5899..0000000
--- a/chrome/browser/value_store/caching_value_store.cc
+++ /dev/null
@@ -1,141 +0,0 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/value_store/caching_value_store.h"
-
-#include "chrome/browser/value_store/leveldb_value_store.h"
-#include "content/public/browser/browser_thread.h"
-
-using content::BrowserThread;
-
-class CachingValueStore::Backend : public base::RefCountedThreadSafe<Backend> {
- public:
-  typedef base::Callback<void(scoped_ptr<base::DictionaryValue>)> ReadyCallback;
-  Backend() : storage_(NULL) {}
-
-  void Init(const FilePath& db_path, ReadyCallback callback) {
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        base::Bind(&CachingValueStore::Backend::InitOnFileThread,
-                   this, db_path, callback));
-  }
-
-  void Set(const std::string& key, scoped_ptr<base::Value> value) {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-    // We've already checked that the value has changed, so don't bother
-    // checking again. Also, skip changes to avoid wasted copies.
-    storage_->Set(ValueStore::IGNORE_QUOTA |
-                      ValueStore::NO_GENERATE_CHANGES |
-                      ValueStore::NO_CHECK_OLD_VALUE,
-                  key, *value.get());
-  }
-
-  void Remove(const std::string& key) {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-    storage_->Remove(key);
-  }
-
- private:
-  friend class base::RefCountedThreadSafe<Backend>;
-
-  virtual ~Backend() {
-    if (BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
-      delete storage_;
-    } else {
-      BrowserThread::DeleteSoon(BrowserThread::FILE, FROM_HERE, storage_);
-    }
-  }
-
-  void InitOnFileThread(const FilePath& db_path, ReadyCallback callback) {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-    DCHECK(!storage_);
-    storage_ = LeveldbValueStore::Create(db_path);
-
-    ValueStore::ReadResult result = storage_->Get();
-    if (result->HasError()) {
-      callback.Run(scoped_ptr<base::DictionaryValue>(NULL));
-    } else {
-      callback.Run(result->settings().Pass());
-    }
-  }
-
-  // The actual ValueStore that handles persisting the data to disk. Used
-  // exclusively on the FILE thread.
-  LeveldbValueStore* storage_;
-
-  DISALLOW_COPY_AND_ASSIGN(Backend);
-};
-
-CachingValueStore::CachingValueStore(const FilePath& db_path)
-    : backend_(new Backend()) {
-  backend_->Init(db_path,
-                 base::Bind(&CachingValueStore::OnBackendReady, AsWeakPtr()));
-}
-
-CachingValueStore::~CachingValueStore() {
-}
-
-bool CachingValueStore::Get(const std::string& key,
-                            const base::Value** result) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(IsInitialized());
-
-  base::Value* value = NULL;
-  if (cache_->GetWithoutPathExpansion(key, &value)) {
-    *result = value;
-    return true;
-  }
-  return false;
-}
-
-void CachingValueStore::Set(const std::string& key, base::Value* value) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(IsInitialized());
-
-  scoped_ptr<base::Value> new_value(value);
-  base::Value* old_value = NULL;
-  if (!cache_->GetWithoutPathExpansion(key, &old_value) ||
-      !value->Equals(old_value)) {
-    cache_->SetWithoutPathExpansion(key, new_value.release());
-
-    scoped_ptr<base::Value> passed_value(value->DeepCopy());
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        base::Bind(&CachingValueStore::Backend::Set,
-                   backend_, key, base::Passed(passed_value.Pass())));
-  }
-}
-
-void CachingValueStore::Remove(const std::string& key) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(IsInitialized());
-
-  if (cache_->RemoveWithoutPathExpansion(key, NULL)) {
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        base::Bind(&CachingValueStore::Backend::Remove,
-                   backend_, key));
-  }
-}
-
-bool CachingValueStore::IsInitialized() {
-  DCHECK(CalledOnValidThread());
-  return !!cache_.get();
-}
-
-void CachingValueStore::AddObserver(Observer* observer) {
-  DCHECK(CalledOnValidThread());
-  observers_.AddObserver(observer);
-}
-
-void CachingValueStore::RemoveObserver(Observer* observer) {
-  DCHECK(CalledOnValidThread());
-  observers_.RemoveObserver(observer);
-}
-
-void CachingValueStore::OnBackendReady(
-    scoped_ptr<base::DictionaryValue> values) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(!cache_.get());
-  cache_.swap(values);
-
-  FOR_EACH_OBSERVER(Observer, observers_, OnInitializationComplete());
-}
diff --git a/chrome/browser/value_store/caching_value_store.h b/chrome/browser/value_store/caching_value_store.h
deleted file mode 100644
index 8b3662b4..0000000
--- a/chrome/browser/value_store/caching_value_store.h
+++ /dev/null
@@ -1,74 +0,0 @@
-// Copyright (c) 2012 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 CHROME_BROWSER_VALUE_STORE_CACHING_VALUE_STORE_H_
-#define CHROME_BROWSER_VALUE_STORE_CACHING_VALUE_STORE_H_
-#pragma once
-
-#include <string>
-
-#include "base/file_path.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/memory/weak_ptr.h"
-#include "base/observer_list.h"
-#include "base/threading/non_thread_safe.h"
-#include "base/values.h"
-
-// Value store area with caching, backed by a LeveldbValueStore on the FILE
-// thread. The key/value store is flat with no path expansion, meaning "foo"
-// and "foo.bar" are completely separate keys.
-class CachingValueStore
-    : public base::SupportsWeakPtr<CachingValueStore>,
-      public base::NonThreadSafe {
- public:
-  class Observer {
-   public:
-    virtual ~Observer() {}
-
-    // Notification that the Store is ready to use.
-    virtual void OnInitializationComplete() = 0;
-  };
-
-  explicit CachingValueStore(const FilePath& db_path);
-  ~CachingValueStore();
-
-  // Retrieves a value from the cache, returning true if value with the given
-  // key exists. The returned value is a reference to the value in the cache,
-  // i.e. no copies are made.
-  bool Get(const std::string& key, const base::Value** result);
-
-  // Sets a value with the given key. Ownership of |value| is transferred to
-  // the store.
-  void Set(const std::string& key, base::Value* value);
-
-  // Removes the value with the given key.
-  void Remove(const std::string& key);
-
-  // Returns true if the store has finished initializing.
-  bool IsInitialized();
-
-  void AddObserver(Observer* observer);
-  void RemoveObserver(Observer* observer);
-
- private:
-  class Backend;
-
-  // Called when our backend finishes reading the database.
-  void OnBackendReady(scoped_ptr<base::DictionaryValue> values);
-
-  // A cache of the value store. This is always up-to-date, with changes
-  // persisted to disk as they are made.
-  scoped_ptr<base::DictionaryValue> cache_;
-
-  // A helper class to manage lifetime of the backing ValueStore, which lives
-  // on the FILE thread.
-  scoped_refptr<Backend> backend_;
-
-  ObserverList<Observer, true> observers_;
-
-  DISALLOW_COPY_AND_ASSIGN(CachingValueStore);
-};
-
-#endif  // CHROME_BROWSER_VALUE_STORE_CACHING_VALUE_STORE_H_
diff --git a/chrome/browser/value_store/failing_value_store.cc b/chrome/browser/value_store/failing_value_store.cc
index 8b6826a..56e509b 100644
--- a/chrome/browser/value_store/failing_value_store.cc
+++ b/chrome/browser/value_store/failing_value_store.cc
@@ -20,59 +20,59 @@ ValueStore::WriteResult WriteResultError() {
 
 }  // namespace
 
-size_t FailingSettingsStorage::GetBytesInUse(const std::string& key) {
+size_t FailingValueStore::GetBytesInUse(const std::string& key) {
   // Let SettingsStorageQuotaEnforcer implement this.
   NOTREACHED() << "Not implemented";
   return 0;
 }
 
-size_t FailingSettingsStorage::GetBytesInUse(
+size_t FailingValueStore::GetBytesInUse(
     const std::vector<std::string>& keys) {
   // Let SettingsStorageQuotaEnforcer implement this.
   NOTREACHED() << "Not implemented";
   return 0;
 }
 
-size_t FailingSettingsStorage::GetBytesInUse() {
+size_t FailingValueStore::GetBytesInUse() {
   // Let SettingsStorageQuotaEnforcer implement this.
   NOTREACHED() << "Not implemented";
   return 0;
 }
 
-ValueStore::ReadResult FailingSettingsStorage::Get(
+ValueStore::ReadResult FailingValueStore::Get(
     const std::string& key) {
   return ReadResultError();
 }
 
-ValueStore::ReadResult FailingSettingsStorage::Get(
+ValueStore::ReadResult FailingValueStore::Get(
     const std::vector<std::string>& keys) {
   return ReadResultError();
 }
 
-ValueStore::ReadResult FailingSettingsStorage::Get() {
+ValueStore::ReadResult FailingValueStore::Get() {
   return ReadResultError();
 }
 
-ValueStore::WriteResult FailingSettingsStorage::Set(
+ValueStore::WriteResult FailingValueStore::Set(
     WriteOptions options, const std::string& key, const Value& value) {
   return WriteResultError();
 }
 
-ValueStore::WriteResult FailingSettingsStorage::Set(
+ValueStore::WriteResult FailingValueStore::Set(
     WriteOptions options, const DictionaryValue& settings) {
   return WriteResultError();
 }
 
-ValueStore::WriteResult FailingSettingsStorage::Remove(
+ValueStore::WriteResult FailingValueStore::Remove(
     const std::string& key) {
   return WriteResultError();
 }
 
-ValueStore::WriteResult FailingSettingsStorage::Remove(
+ValueStore::WriteResult FailingValueStore::Remove(
     const std::vector<std::string>& keys) {
   return WriteResultError();
 }
 
-ValueStore::WriteResult FailingSettingsStorage::Clear() {
+ValueStore::WriteResult FailingValueStore::Clear() {
   return WriteResultError();
 }
diff --git a/chrome/browser/value_store/failing_value_store.h b/chrome/browser/value_store/failing_value_store.h
index f1887bc..5f07359 100644
--- a/chrome/browser/value_store/failing_value_store.h
+++ b/chrome/browser/value_store/failing_value_store.h
@@ -10,9 +10,9 @@
 #include "chrome/browser/value_store/value_store.h"
 
 // Settings storage area which fails every request.
-class FailingSettingsStorage : public ValueStore {
+class FailingValueStore : public ValueStore {
  public:
-  FailingSettingsStorage() {}
+  FailingValueStore() {}
 
   // ValueStore implementation.
   virtual size_t GetBytesInUse(const std::string& key) OVERRIDE;
@@ -32,7 +32,7 @@ class FailingSettingsStorage : public ValueStore {
   virtual WriteResult Clear() OVERRIDE;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(FailingSettingsStorage);
+  DISALLOW_COPY_AND_ASSIGN(FailingValueStore);
 };
 
 #endif  // CHROME_BROWSER_VALUE_STORE_FAILING_VALUE_STORE_H_
diff --git a/chrome/browser/value_store/testing_value_store.cc b/chrome/browser/value_store/testing_value_store.cc
index 337d999..21473e5 100644
--- a/chrome/browser/value_store/testing_value_store.cc
+++ b/chrome/browser/value_store/testing_value_store.cc
@@ -8,7 +8,7 @@
 
 namespace {
 
-const char* kGenericErrorMessage = "TestingSettingsStorage configured to error";
+const char* kGenericErrorMessage = "TestingValueStore configured to error";
 
 ValueStore::ReadResult ReadResultError() {
   return ValueStore::MakeReadResult(kGenericErrorMessage);
@@ -26,40 +26,40 @@ std::vector<std::string> CreateVector(const std::string& string) {
 
 }  // namespace
 
-TestingSettingsStorage::TestingSettingsStorage()
+TestingValueStore::TestingValueStore()
     : fail_all_requests_(false) {}
 
-TestingSettingsStorage::~TestingSettingsStorage() {}
+TestingValueStore::~TestingValueStore() {}
 
-void TestingSettingsStorage::SetFailAllRequests(bool fail_all_requests) {
+void TestingValueStore::SetFailAllRequests(bool fail_all_requests) {
   fail_all_requests_ = fail_all_requests;
 }
 
-size_t TestingSettingsStorage::GetBytesInUse(const std::string& key) {
+size_t TestingValueStore::GetBytesInUse(const std::string& key) {
   // Let SettingsStorageQuotaEnforcer implement this.
   NOTREACHED() << "Not implemented";
   return 0;
 }
 
-size_t TestingSettingsStorage::GetBytesInUse(
+size_t TestingValueStore::GetBytesInUse(
     const std::vector<std::string>& keys) {
   // Let SettingsStorageQuotaEnforcer implement this.
   NOTREACHED() << "Not implemented";
   return 0;
 }
 
-size_t TestingSettingsStorage::GetBytesInUse() {
+size_t TestingValueStore::GetBytesInUse() {
   // Let SettingsStorageQuotaEnforcer implement this.
   NOTREACHED() << "Not implemented";
   return 0;
 }
 
-ValueStore::ReadResult TestingSettingsStorage::Get(
+ValueStore::ReadResult TestingValueStore::Get(
     const std::string& key) {
   return Get(CreateVector(key));
 }
 
-ValueStore::ReadResult TestingSettingsStorage::Get(
+ValueStore::ReadResult TestingValueStore::Get(
     const std::vector<std::string>& keys) {
   if (fail_all_requests_) {
     return ReadResultError();
@@ -76,21 +76,21 @@ ValueStore::ReadResult TestingSettingsStorage::Get(
   return MakeReadResult(settings);
 }
 
-ValueStore::ReadResult TestingSettingsStorage::Get() {
+ValueStore::ReadResult TestingValueStore::Get() {
   if (fail_all_requests_) {
     return ReadResultError();
   }
   return MakeReadResult(storage_.DeepCopy());
 }
 
-ValueStore::WriteResult TestingSettingsStorage::Set(
+ValueStore::WriteResult TestingValueStore::Set(
     WriteOptions options, const std::string& key, const Value& value) {
   DictionaryValue settings;
   settings.SetWithoutPathExpansion(key, value.DeepCopy());
   return Set(options, settings);
 }
 
-ValueStore::WriteResult TestingSettingsStorage::Set(
+ValueStore::WriteResult TestingValueStore::Set(
     WriteOptions options, const DictionaryValue& settings) {
   if (fail_all_requests_) {
     return WriteResultError();
@@ -112,12 +112,12 @@ ValueStore::WriteResult TestingSettingsStorage::Set(
   return MakeWriteResult(changes.release());
 }
 
-ValueStore::WriteResult TestingSettingsStorage::Remove(
+ValueStore::WriteResult TestingValueStore::Remove(
     const std::string& key) {
   return Remove(CreateVector(key));
 }
 
-ValueStore::WriteResult TestingSettingsStorage::Remove(
+ValueStore::WriteResult TestingValueStore::Remove(
     const std::vector<std::string>& keys) {
   if (fail_all_requests_) {
     return WriteResultError();
@@ -135,7 +135,7 @@ ValueStore::WriteResult TestingSettingsStorage::Remove(
   return MakeWriteResult(changes.release());
 }
 
-ValueStore::WriteResult TestingSettingsStorage::Clear() {
+ValueStore::WriteResult TestingValueStore::Clear() {
   if (fail_all_requests_) {
     return WriteResultError();
   }
diff --git a/chrome/browser/value_store/testing_value_store.h b/chrome/browser/value_store/testing_value_store.h
index 5b3f1f1..8da7c53 100644
--- a/chrome/browser/value_store/testing_value_store.h
+++ b/chrome/browser/value_store/testing_value_store.h
@@ -11,10 +11,10 @@
 
 // ValueStore for testing, with an in-memory storage but the ability to
 // optionally fail all operations.
-class TestingSettingsStorage : public ValueStore {
+class TestingValueStore : public ValueStore {
  public:
-  TestingSettingsStorage();
-  virtual ~TestingSettingsStorage();
+  TestingValueStore();
+  virtual ~TestingValueStore();
 
   // Sets whether to fail all requests (default is false).
   void SetFailAllRequests(bool fail_all_requests);
@@ -41,7 +41,7 @@ class TestingSettingsStorage : public ValueStore {
 
   bool fail_all_requests_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestingSettingsStorage);
+  DISALLOW_COPY_AND_ASSIGN(TestingValueStore);
 };
 
 #endif  // CHROME_BROWSER_VALUE_STORE_TESTING_VALUE_STORE_H_
diff --git a/chrome/browser/value_store/testing_value_store_unittest.cc b/chrome/browser/value_store/testing_value_store_unittest.cc
index f311bc1..eaaca40 100644
--- a/chrome/browser/value_store/testing_value_store_unittest.cc
+++ b/chrome/browser/value_store/testing_value_store_unittest.cc
@@ -11,13 +11,13 @@ namespace extensions {
 namespace {
 
 ValueStore* Param(const FilePath& file_path) {
-  return new TestingSettingsStorage();
+  return new TestingValueStore();
 }
 
 }  // namespace
 
 INSTANTIATE_TEST_CASE_P(
-    TestingSettingsStorage,
+    TestingValueStore,
     ValueStoreTest,
     testing::Values(&Param));
 
diff --git a/chrome/browser/value_store/value_store_frontend.cc b/chrome/browser/value_store/value_store_frontend.cc
new file mode 100644
index 0000000..7a87cba
--- /dev/null
+++ b/chrome/browser/value_store/value_store_frontend.cc
@@ -0,0 +1,114 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/value_store/value_store_frontend.h"
+
+#include "chrome/browser/value_store/failing_value_store.h"
+#include "chrome/browser/value_store/leveldb_value_store.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> {
+ public:
+  explicit Backend(const FilePath& db_path) : storage_(NULL) {
+  }
+
+  void Init(const FilePath& db_path) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    DCHECK(!storage_);
+    storage_ = LeveldbValueStore::Create(db_path);
+    if (!storage_)
+      storage_ = new FailingValueStore();
+  }
+
+  void Get(const std::string& key,
+           const ValueStoreFrontend::ReadCallback& callback) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    ValueStore::ReadResult result = storage_->Get(key);
+
+    // Extract the value from the ReadResult and pass ownership of it to the
+    // callback.
+    base::Value* value = NULL;
+    if (!result->HasError())
+      result->settings()->RemoveWithoutPathExpansion(key, &value);
+
+    scoped_ptr<base::Value> passed_value(value);
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(&ValueStoreFrontend::Backend::RunCallback,
+                   this, callback, base::Passed(passed_value.Pass())));
+  }
+
+  void Set(const std::string& key, scoped_ptr<base::Value> value) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    // We don't need the old value, so skip generating changes.
+    storage_->Set(ValueStore::IGNORE_QUOTA | ValueStore::NO_GENERATE_CHANGES,
+                  key, *value.get());
+  }
+
+  void Remove(const std::string& key) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    storage_->Remove(key);
+  }
+
+ private:
+  friend class base::RefCountedThreadSafe<Backend>;
+
+  virtual ~Backend() {
+    if (BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
+      delete storage_;
+    } else {
+      BrowserThread::DeleteSoon(BrowserThread::FILE, FROM_HERE, storage_);
+    }
+  }
+
+  void RunCallback(const ValueStoreFrontend::ReadCallback& callback,
+                   scoped_ptr<base::Value> value) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    callback.Run(value.Pass());
+  }
+
+  // The actual ValueStore that handles persisting the data to disk. Used
+  // exclusively on the FILE thread.
+  ValueStore* storage_;
+
+  DISALLOW_COPY_AND_ASSIGN(Backend);
+};
+
+ValueStoreFrontend::ValueStoreFrontend(const FilePath& db_path)
+    : backend_(new Backend(db_path)) {
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ValueStoreFrontend::Backend::Init,
+                 backend_, db_path));
+}
+
+ValueStoreFrontend::~ValueStoreFrontend() {
+  DCHECK(CalledOnValidThread());
+}
+
+void ValueStoreFrontend::Get(const std::string& key,
+                             const ReadCallback& callback) {
+  DCHECK(CalledOnValidThread());
+
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ValueStoreFrontend::Backend::Get,
+                 backend_, key, callback));
+}
+
+void ValueStoreFrontend::Set(const std::string& key,
+                             scoped_ptr<base::Value> value) {
+  DCHECK(CalledOnValidThread());
+
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ValueStoreFrontend::Backend::Set,
+                 backend_, key, base::Passed(value.Pass())));
+}
+
+void ValueStoreFrontend::Remove(const std::string& key) {
+  DCHECK(CalledOnValidThread());
+
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ValueStoreFrontend::Backend::Remove,
+                 backend_, key));
+}
diff --git a/chrome/browser/value_store/value_store_frontend.h b/chrome/browser/value_store/value_store_frontend.h
new file mode 100644
index 0000000..f6027da
--- /dev/null
+++ b/chrome/browser/value_store/value_store_frontend.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2012 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 CHROME_BROWSER_VALUE_STORE_VALUE_STORE_FRONTEND_H_
+#define CHROME_BROWSER_VALUE_STORE_VALUE_STORE_FRONTEND_H_
+#pragma once
+
+#include <string>
+
+#include "base/callback.h"
+#include "base/file_path.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/non_thread_safe.h"
+#include "base/values.h"
+
+// A frontend for a LeveldbValueStore, for use on the UI thread.
+class ValueStoreFrontend
+    : public base::SupportsWeakPtr<ValueStoreFrontend>,
+      public base::NonThreadSafe {
+ public:
+  typedef base::Callback<void(scoped_ptr<base::Value>)> ReadCallback;
+
+  explicit ValueStoreFrontend(const FilePath& db_path);
+  ~ValueStoreFrontend();
+
+  // Retrieves a value from the database asynchronously, passing a copy to
+  // |callback| when ready. NULL is passed if no matching entry is found.
+  void Get(const std::string& key, const ReadCallback& callback);
+
+  // Sets a value with the given key.
+  void Set(const std::string& key, scoped_ptr<base::Value> value);
+
+  // Removes the value with the given key.
+  void Remove(const std::string& key);
+
+ private:
+  class Backend;
+
+  // A helper class to manage lifetime of the backing ValueStore, which lives
+  // on the FILE thread.
+  scoped_refptr<Backend> backend_;
+
+  DISALLOW_COPY_AND_ASSIGN(ValueStoreFrontend);
+};
+
+#endif  // CHROME_BROWSER_VALUE_STORE_VALUE_STORE_FRONTEND_H_
diff --git a/chrome/browser/value_store/caching_value_store_unittest.cc b/chrome/browser/value_store/value_store_frontend_unittest.cc
index eaa8093..efec3ed 100644
--- a/chrome/browser/value_store/caching_value_store_unittest.cc
+++ b/chrome/browser/value_store/value_store_frontend_unittest.cc
@@ -7,7 +7,7 @@
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
-#include "chrome/browser/value_store/caching_value_store.h"
+#include "chrome/browser/value_store/value_store_frontend.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -16,11 +16,9 @@ using content::BrowserThread;
 
 // Test suite for CachingValueStore, using a test database with a few simple
 // entries.
-class CachingValueStoreTest
-    : public testing::Test,
-      public CachingValueStore::Observer {
+class ValueStoreFrontendTest : public testing::Test {
  public:
-  CachingValueStoreTest()
+  ValueStoreFrontendTest()
       : ui_thread_(BrowserThread::UI, MessageLoop::current()),
         file_thread_(BrowserThread::FILE, MessageLoop::current()) {
   }
@@ -39,26 +37,29 @@ class CachingValueStoreTest
 
   virtual void TearDown() {
     MessageLoop::current()->RunAllPending();  // wait for storage to delete
-    storage_->RemoveObserver(this);
     storage_.reset();
   }
 
-  // CachingValueStore::Observer
-  virtual void OnInitializationComplete() {
-    MessageLoop::current()->Quit();
-  }
-
   // Reset the value store, reloading the DB from disk.
   void ResetStorage() {
-    if (storage_.get())
-      storage_->RemoveObserver(this);
-    storage_.reset(new CachingValueStore(db_path_));
-    storage_->AddObserver(this);
-    MessageLoop::current()->Run();  // wait for OnInitializationComplete
+    storage_.reset(new ValueStoreFrontend(db_path_));
+  }
+
+  bool Get(const std::string& key, scoped_ptr<base::Value>* output) {
+    storage_->Get(key, base::Bind(&ValueStoreFrontendTest::GetAndWait,
+                                  base::Unretained(this), output));
+    MessageLoop::current()->Run();  // wait for GetAndWait
+    return !!output->get();
   }
 
  protected:
-  scoped_ptr<CachingValueStore> storage_;
+  void GetAndWait(scoped_ptr<base::Value>* output,
+                  scoped_ptr<base::Value> result) {
+    *output = result.Pass();
+    MessageLoop::current()->Quit();
+  }
+
+  scoped_ptr<ValueStoreFrontend> storage_;
   ScopedTempDir temp_dir_;
   FilePath db_path_;
   MessageLoop message_loop_;
@@ -66,48 +67,50 @@ class CachingValueStoreTest
   content::TestBrowserThread file_thread_;
 };
 
-TEST_F(CachingValueStoreTest, GetExistingData) {
-  const base::Value* value = NULL;
-  ASSERT_FALSE(storage_->Get("key0", &value));
+TEST_F(ValueStoreFrontendTest, GetExistingData) {
+  scoped_ptr<base::Value> value(NULL);
+  ASSERT_FALSE(Get("key0", &value));
 
   // Test existing keys in the DB.
   {
-    ASSERT_TRUE(storage_->Get("key1", &value));
+    ASSERT_TRUE(Get("key1", &value));
     std::string result;
     ASSERT_TRUE(value->GetAsString(&result));
     EXPECT_EQ("value1", result);
   }
 
   {
-    ASSERT_TRUE(storage_->Get("key2", &value));
+    ASSERT_TRUE(Get("key2", &value));
     int result;
     ASSERT_TRUE(value->GetAsInteger(&result));
     EXPECT_EQ(2, result);
   }
 }
 
-TEST_F(CachingValueStoreTest, ChangesPersistAfterReload) {
-  storage_->Set("key0", base::Value::CreateIntegerValue(0));
-  storage_->Set("key1", base::Value::CreateStringValue("new1"));
+TEST_F(ValueStoreFrontendTest, ChangesPersistAfterReload) {
+  storage_->Set("key0",
+      scoped_ptr<base::Value>(base::Value::CreateIntegerValue(0)));
+  storage_->Set("key1",
+      scoped_ptr<base::Value>(base::Value::CreateStringValue("new1")));
   storage_->Remove("key2");
 
   // Reload the DB and test our changes.
   ResetStorage();
 
-  const base::Value* value = NULL;
+  scoped_ptr<base::Value> value(NULL);
   {
-    ASSERT_TRUE(storage_->Get("key0", &value));
+    ASSERT_TRUE(Get("key0", &value));
     int result;
     ASSERT_TRUE(value->GetAsInteger(&result));
     EXPECT_EQ(0, result);
   }
 
   {
-    ASSERT_TRUE(storage_->Get("key1", &value));
+    ASSERT_TRUE(Get("key1", &value));
     std::string result;
     ASSERT_TRUE(value->GetAsString(&result));
     EXPECT_EQ("new1", result);
   }
 
-  ASSERT_FALSE(storage_->Get("key2", &value));
+  ASSERT_FALSE(Get("key2", &value));
 }