Extension Settings API: make it so that when leveldb storage areas fail to be

opened/created, all operations fail from then on rather than falling back to an
in-memory implementation.

Slight refactor to SettingsFrontend/Backend to inject a factory for creating
SettingsStorage objects, for testing when storage area construction fails.

BUG=103514
TEST=*ExtensionSettings*

Review URL: http://codereview.chromium.org/8497065

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

22 files changed, 356 insertions, 92 deletions

diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 2499bd4..bd34f24 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -55,6 +55,7 @@
 #include "chrome/browser/extensions/external_extension_provider_interface.h"
 #include "chrome/browser/extensions/installed_loader.h"
 #include "chrome/browser/extensions/pending_extension_manager.h"
+#include "chrome/browser/extensions/settings/settings_frontend.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/history/history_extension_api.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
@@ -362,7 +363,7 @@ ExtensionService::ExtensionService(Profile* profile,
                                    bool extensions_enabled)
     : profile_(profile),
       extension_prefs_(extension_prefs),
-      settings_frontend_(profile),
+      settings_frontend_(extensions::SettingsFrontend::Create(profile)),
       pending_extension_manager_(*ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       install_directory_(install_directory),
       extensions_enabled_(extensions_enabled),
@@ -1123,7 +1124,7 @@ ExtensionPrefs* ExtensionService::extension_prefs() {
 }
 
 extensions::SettingsFrontend* ExtensionService::settings_frontend() {
-  return &settings_frontend_;
+  return settings_frontend_.get();
 }
 
 ExtensionContentSettingsStore*
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 0a1ec62..dbc88ac 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -26,7 +26,6 @@
 #include "chrome/browser/extensions/extension_permissions_api.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
-#include "chrome/browser/extensions/settings/settings_frontend.h"
 #include "chrome/browser/extensions/extension_sync_data.h"
 #include "chrome/browser/extensions/extension_toolbar_model.h"
 #include "chrome/browser/extensions/extension_warning_set.h"
@@ -74,6 +73,7 @@ class ExtensionInputMethodEventRouter;
 
 namespace extensions {
 class ComponentLoader;
+class SettingsFrontend;
 }
 
 // This is an interface class to encapsulate the dependencies that
@@ -661,7 +661,7 @@ class ExtensionService
   ExtensionPrefs* extension_prefs_;
 
   // Settings for the owning profile.
-  extensions::SettingsFrontend settings_frontend_;
+  scoped_ptr<extensions::SettingsFrontend> settings_frontend_;
 
   // The current list of installed extensions.
   // TODO(aa): This should use chrome/common/extensions/extension_set.h.
diff --git a/chrome/browser/extensions/settings/failing_settings_storage.cc b/chrome/browser/extensions/settings/failing_settings_storage.cc
new file mode 100644
index 0000000..92d5a96
--- /dev/null
+++ b/chrome/browser/extensions/settings/failing_settings_storage.cc
@@ -0,0 +1,61 @@
+// Copyright (c) 2011 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/extensions/settings/failing_settings_storage.h"
+
+namespace extensions {
+
+namespace {
+
+const char* kGenericErrorMessage = "Failed to initialize settings";
+
+SettingsStorage::ReadResult ReadResultError() {
+  return SettingsStorage::ReadResult(kGenericErrorMessage);
+}
+
+SettingsStorage::WriteResult WriteResultError() {
+  return SettingsStorage::WriteResult(kGenericErrorMessage);
+}
+
+}  // namespace
+
+SettingsStorage::ReadResult FailingSettingsStorage::Get(
+    const std::string& key) {
+  return ReadResultError();
+}
+
+SettingsStorage::ReadResult FailingSettingsStorage::Get(
+    const std::vector<std::string>& keys) {
+  return ReadResultError();
+}
+
+SettingsStorage::ReadResult FailingSettingsStorage::Get() {
+  return ReadResultError();
+}
+
+SettingsStorage::WriteResult FailingSettingsStorage::Set(
+    const std::string& key, const Value& value) {
+  return WriteResultError();
+}
+
+SettingsStorage::WriteResult FailingSettingsStorage::Set(
+    const DictionaryValue& settings) {
+  return WriteResultError();
+}
+
+SettingsStorage::WriteResult FailingSettingsStorage::Remove(
+    const std::string& key) {
+  return WriteResultError();
+}
+
+SettingsStorage::WriteResult FailingSettingsStorage::Remove(
+    const std::vector<std::string>& keys) {
+  return WriteResultError();
+}
+
+SettingsStorage::WriteResult FailingSettingsStorage::Clear() {
+  return WriteResultError();
+}
+
+}  // namespace extensions
diff --git a/chrome/browser/extensions/settings/failing_settings_storage.h b/chrome/browser/extensions/settings/failing_settings_storage.h
new file mode 100644
index 0000000..db5c8c3
--- /dev/null
+++ b/chrome/browser/extensions/settings/failing_settings_storage.h
@@ -0,0 +1,35 @@
+// Copyright (c) 2011 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_EXTENSIONS_SETTINGS_FAILING_SETTINGS_STORAGE_H_
+#define CHROME_BROWSER_EXTENSIONS_SETTINGS_FAILING_SETTINGS_STORAGE_H_
+#pragma once
+
+#include "base/compiler_specific.h"
+#include "chrome/browser/extensions/settings/settings_storage.h"
+
+namespace extensions {
+
+// Settings storage area which fails every request.
+class FailingSettingsStorage : public SettingsStorage {
+ public:
+  FailingSettingsStorage() {}
+
+  // SettingsStorage implementation.
+  virtual ReadResult Get(const std::string& key) OVERRIDE;
+  virtual ReadResult Get(const std::vector<std::string>& keys) OVERRIDE;
+  virtual ReadResult Get() OVERRIDE;
+  virtual WriteResult Set(const std::string& key, const Value& value) OVERRIDE;
+  virtual WriteResult Set(const DictionaryValue& values) OVERRIDE;
+  virtual WriteResult Remove(const std::string& key) OVERRIDE;
+  virtual WriteResult Remove(const std::vector<std::string>& keys) OVERRIDE;
+  virtual WriteResult Clear() OVERRIDE;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FailingSettingsStorage);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_SETTINGS_FAILING_SETTINGS_STORAGE_H_
diff --git a/chrome/browser/extensions/settings/in_memory_settings_storage_unittest.cc b/chrome/browser/extensions/settings/in_memory_settings_storage_unittest.cc
index f2e32c0..c19cc9d7 100644
--- a/chrome/browser/extensions/settings/in_memory_settings_storage_unittest.cc
+++ b/chrome/browser/extensions/settings/in_memory_settings_storage_unittest.cc
@@ -19,7 +19,7 @@ SettingsStorage* Param(
 
 INSTANTIATE_TEST_CASE_P(
     InMemorySettingsStorage,
-    SettingsStorageTest,
+    ExtensionSettingsStorageTest,
     testing::Values(&Param));
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/settings/settings_api.cc b/chrome/browser/extensions/settings/settings_api.cc
index 1a7916a..a927a01 100644
--- a/chrome/browser/extensions/settings/settings_api.cc
+++ b/chrome/browser/extensions/settings/settings_api.cc
@@ -2,10 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/extensions/settings/settings_api.h"
+
 #include "base/bind.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/settings/settings_api.h"
+#include "chrome/browser/extensions/settings/settings_frontend.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -35,10 +37,7 @@ void SettingsFunction::RunWithStorageOnFileThread(
     scoped_refptr<SettingsObserverList> observers,
     SettingsStorage* storage) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  bool success = false;
-  if (storage) {
-    success = RunWithStorage(observers.get(), storage);
-  }
+  bool success = RunWithStorage(observers.get(), storage);
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
diff --git a/chrome/browser/extensions/settings/settings_apitest.cc b/chrome/browser/extensions/settings/settings_apitest.cc
index 588bbba..2d99f85 100644
--- a/chrome/browser/extensions/settings/settings_apitest.cc
+++ b/chrome/browser/extensions/settings/settings_apitest.cc
@@ -7,6 +7,7 @@
 #include "base/json/json_writer.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/settings/settings_frontend.h"
 #include "chrome/browser/extensions/settings/settings_sync_util.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/profiles/profile.h"
diff --git a/chrome/browser/extensions/settings/settings_backend.cc b/chrome/browser/extensions/settings/settings_backend.cc
index 78e776a..d56a7d3 100644
--- a/chrome/browser/extensions/settings/settings_backend.cc
+++ b/chrome/browser/extensions/settings/settings_backend.cc
@@ -11,20 +11,18 @@
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/extensions/settings/settings_leveldb_storage.h"
+#include "chrome/browser/extensions/settings/failing_settings_storage.h"
 #include "chrome/browser/extensions/settings/settings_storage_cache.h"
+#include "chrome/browser/extensions/settings/settings_storage_factory.h"
 #include "chrome/browser/extensions/settings/settings_storage_quota_enforcer.h"
 #include "chrome/browser/extensions/settings/settings_sync_util.h"
-#include "chrome/browser/extensions/settings/in_memory_settings_storage.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
-#include "third_party/leveldatabase/src/include/leveldb/iterator.h"
-#include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
-
-namespace extensions {
 
 using content::BrowserThread;
 
+namespace extensions {
+
 namespace {
 
 // Total quota for all settings per extension, in bytes.  100K should be enough
@@ -41,9 +39,11 @@ const size_t kMaxSettingKeys = 512;
 }  // namespace
 
 SettingsBackend::SettingsBackend(
+    SettingsStorageFactory* storage_factory,
     const FilePath& base_path,
     const scoped_refptr<SettingsObserverList>& observers)
-    : base_path_(base_path),
+    : storage_factory_(storage_factory),
+      base_path_(base_path),
       observers_(observers),
       sync_type_(syncable::UNSPECIFIED),
       sync_processor_(NULL) {
@@ -66,35 +66,27 @@ SettingsBackend::GetOrCreateStorageWithSyncData(
     const std::string& extension_id, const DictionaryValue& sync_data) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  linked_ptr<SyncableSettingsStorage> syncable_storage =
-      storage_objs_[extension_id];
-  if (syncable_storage.get()) {
-    return syncable_storage.get();
+  StorageObjMap::iterator maybe_storage = storage_objs_.find(extension_id);
+  if (maybe_storage != storage_objs_.end()) {
+    return maybe_storage->second.get();
   }
 
-  SettingsStorage* storage =
-      SettingsLeveldbStorage::Create(base_path_, extension_id);
+  SettingsStorage* storage = storage_factory_->Create(base_path_, extension_id);
   if (storage) {
     storage = new SettingsStorageCache(storage);
+    // It's fine to create the quota enforcer underneath the sync layer, since
+    // sync will only go ahead if each underlying storage operation succeeds.
+    storage = new SettingsStorageQuotaEnforcer(
+        kTotalQuotaBytes, kQuotaPerSettingBytes, kMaxSettingKeys, storage);
   } else {
-    // Failed to create a leveldb storage area, create an in-memory one.
-    // It's ok for these to be synced, it just means that on next starting up
-    // extensions will see the "old" settings, then overwritten (and notified)
-    // when the sync changes come through.
-    storage = new InMemorySettingsStorage();
+    storage = new FailingSettingsStorage();
   }
 
-  // It's fine to create the quota enforcer underneath the sync later, since
-  // sync will only go ahead if each underlying storage operation is successful.
-  storage = new SettingsStorageQuotaEnforcer(
-      kTotalQuotaBytes, kQuotaPerSettingBytes, kMaxSettingKeys, storage);
-
-  syncable_storage =
-      linked_ptr<SyncableSettingsStorage>(
-          new SyncableSettingsStorage(
-              observers_,
-              extension_id,
-              storage));
+  linked_ptr<SyncableSettingsStorage> syncable_storage(
+      new SyncableSettingsStorage(
+          observers_,
+          extension_id,
+          storage));
   if (sync_processor_) {
     // TODO(kalman): do something if StartSyncing fails.
     ignore_result(syncable_storage->StartSyncing(
@@ -154,8 +146,7 @@ static void AddAllSyncData(
     SyncDataList* dst) {
   for (DictionaryValue::Iterator it(src); it.HasNext(); it.Advance()) {
     dst->push_back(
-        settings_sync_util::CreateData(
-            extension_id, it.key(), it.value()));
+        settings_sync_util::CreateData(extension_id, it.key(), it.value()));
   }
 }
 
diff --git a/chrome/browser/extensions/settings/settings_backend.h b/chrome/browser/extensions/settings/settings_backend.h
index 58f1d02..1c6366f 100644
--- a/chrome/browser/extensions/settings/settings_backend.h
+++ b/chrome/browser/extensions/settings/settings_backend.h
@@ -12,6 +12,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/observer_list_threadsafe.h"
 #include "base/task.h"
+#include "chrome/browser/extensions/settings/settings_leveldb_storage.h"
 #include "chrome/browser/extensions/settings/settings_observer.h"
 #include "chrome/browser/extensions/settings/syncable_settings_storage.h"
 #include "chrome/browser/sync/api/syncable_service.h"
@@ -23,10 +24,13 @@ namespace extensions {
 // Lives entirely on the FILE thread.
 class SettingsBackend : public SyncableService {
  public:
+  // |storage_factory| is use to create leveldb storage areas.
   // |base_path| is the base of the extension settings directory, so the
   // databases will be at base_path/extension_id.
   // |observers| is the list of observers to settings changes.
   explicit SettingsBackend(
+      // Ownership NOT taken.
+      SettingsStorageFactory* storage_factory,
       const FilePath& base_path,
       const scoped_refptr<SettingsObserverList>& observers);
 
@@ -68,6 +72,9 @@ class SettingsBackend : public SyncableService {
   // installed extensions.
   std::set<std::string> GetKnownExtensionIDs() const;
 
+  // The Factory to use for creating leveldb storage areas.  Not owned.
+  SettingsStorageFactory* const storage_factory_;
+
   // The base file path to create any leveldb databases at.
   const FilePath base_path_;
 
diff --git a/chrome/browser/extensions/settings/settings_frontend.cc b/chrome/browser/extensions/settings/settings_frontend.cc
index 31d79dd..1117e55 100644
--- a/chrome/browser/extensions/settings/settings_frontend.cc
+++ b/chrome/browser/extensions/settings/settings_frontend.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/settings/settings_backend.h"
+#include "chrome/browser/extensions/settings/settings_leveldb_storage.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
@@ -22,17 +23,23 @@ namespace {
 
 struct Backends {
   Backends(
+      // Ownership taken.
+      SettingsStorageFactory* storage_factory,
       const FilePath& profile_path,
       const scoped_refptr<SettingsObserverList>& observers)
-    : extensions_backend_(
+    : storage_factory_(storage_factory),
+      extensions_backend_(
+          storage_factory,
           profile_path.AppendASCII(
               ExtensionService::kExtensionSettingsDirectoryName),
           observers),
       apps_backend_(
+          storage_factory,
           profile_path.AppendASCII(
               ExtensionService::kAppSettingsDirectoryName),
           observers) {}
 
+  scoped_ptr<SettingsStorageFactory> storage_factory_;
   SettingsBackend extensions_backend_;
   SettingsBackend apps_backend_;
 };
@@ -82,7 +89,7 @@ void DeleteStorageOnFileThread(
 
 }  // namespace
 
-// DefaultObserver.
+// DefaultObserver
 
 SettingsFrontend::DefaultObserver::DefaultObserver(Profile* profile)
     : profile_(profile) {}
@@ -101,12 +108,18 @@ void SettingsFrontend::DefaultObserver::OnSettingsChanged(
       GURL());
 }
 
+// Core
+
 class SettingsFrontend::Core
     : public base::RefCountedThreadSafe<Core> {
  public:
   explicit Core(
+      // Ownership taken.
+      SettingsStorageFactory* storage_factory,
       const scoped_refptr<SettingsObserverList>& observers)
-      : observers_(observers), backends_(NULL) {
+      : storage_factory_(storage_factory),
+        observers_(observers),
+        backends_(NULL) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   }
 
@@ -118,7 +131,9 @@ class SettingsFrontend::Core
   void InitOnFileThread(const FilePath& profile_path) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
     DCHECK(!backends_);
-    backends_ = new Backends(profile_path, observers_);
+    backends_ =
+        new Backends(
+            storage_factory_.release(), profile_path, observers_);
   }
 
   // Runs |callback| with both the extensions and apps settings on the FILE
@@ -153,6 +168,9 @@ class SettingsFrontend::Core
 
   friend class base::RefCountedThreadSafe<Core>;
 
+  // Leveldb storage area factory.  Ownership passed to Backends on Init.
+  scoped_ptr<SettingsStorageFactory> storage_factory_;
+
   // Observers to settings changes (thread safe).
   scoped_refptr<SettingsObserverList> observers_;
 
@@ -162,11 +180,25 @@ class SettingsFrontend::Core
   DISALLOW_COPY_AND_ASSIGN(Core);
 };
 
-SettingsFrontend::SettingsFrontend(Profile* profile)
+// SettingsFrontend
+
+/* static */
+SettingsFrontend* SettingsFrontend::Create(Profile* profile) {
+  return new SettingsFrontend(new SettingsLeveldbStorage::Factory(), profile);
+}
+
+/* static */
+SettingsFrontend* SettingsFrontend::Create(
+    SettingsStorageFactory* storage_factory, Profile* profile) {
+  return new SettingsFrontend(storage_factory, profile);
+}
+
+SettingsFrontend::SettingsFrontend(
+    SettingsStorageFactory* storage_factory, Profile* profile)
     : profile_(profile),
       observers_(new SettingsObserverList()),
       default_observer_(profile),
-      core_(new SettingsFrontend::Core(observers_)) {
+      core_(new SettingsFrontend::Core(storage_factory, observers_)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!profile->IsOffTheRecord());
 
diff --git a/chrome/browser/extensions/settings/settings_frontend.h b/chrome/browser/extensions/settings/settings_frontend.h
index 93b24d0..ed4e12b 100644
--- a/chrome/browser/extensions/settings/settings_frontend.h
+++ b/chrome/browser/extensions/settings/settings_frontend.h
@@ -11,6 +11,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list_threadsafe.h"
+#include "chrome/browser/extensions/settings/settings_leveldb_storage.h"
 #include "chrome/browser/extensions/settings/settings_observer.h"
 #include "chrome/browser/sync/api/syncable_service.h"
 #include "content/public/browser/notification_observer.h"
@@ -28,7 +29,15 @@ class SettingsStorage;
 // All public methods must be called on the UI thread.
 class SettingsFrontend {
  public:
-  explicit SettingsFrontend(Profile* profile);
+  // Creates with the default factory.  Ownership of |profile| not taken.
+  static SettingsFrontend* Create(Profile* profile);
+
+  static SettingsFrontend* Create(
+      // Ownership taken.
+      SettingsStorageFactory* storage_factory,
+      // Owership NOT taken.
+      Profile* profile);
+
   virtual ~SettingsFrontend();
 
   typedef base::Callback<void(SyncableService*)> SyncableServiceCallback;
@@ -68,6 +77,12 @@ class SettingsFrontend {
     Profile* const profile_;
   };
 
+  SettingsFrontend(
+      // Ownership taken.
+      SettingsStorageFactory* storage_factory,
+      // Ownership NOT taken.
+      Profile* profile);
+
   // The (non-incognito) Profile this Frontend belongs to.
   Profile* const profile_;
 
diff --git a/chrome/browser/extensions/settings/settings_frontend_unittest.cc b/chrome/browser/extensions/settings/settings_frontend_unittest.cc
index 4c0063a..18a97bc 100644
--- a/chrome/browser/extensions/settings/settings_frontend_unittest.cc
+++ b/chrome/browser/extensions/settings/settings_frontend_unittest.cc
@@ -21,16 +21,56 @@ using content::BrowserThread;
 
 using namespace settings_test_util;
 
-class SettingsFrontendTest : public testing::Test {
+namespace {
+
+// SettingsStorageFactory which acts as a wrapper for other factories.
+class ScopedSettingsStorageFactory : public SettingsStorageFactory {
+ public:
+  explicit ScopedSettingsStorageFactory(SettingsStorageFactory* delegate)
+      : delegate_(delegate) {
+    DCHECK(delegate);
+  }
+
+  virtual ~ScopedSettingsStorageFactory() {}
+
+  void Reset(SettingsStorageFactory* delegate) {
+    DCHECK(delegate);
+    delegate_.reset(delegate);
+  }
+
+  virtual SettingsStorage* Create(
+      const FilePath& base_path, const std::string& extension_id) OVERRIDE {
+    return delegate_->Create(base_path, extension_id);
+  }
+
+ private:
+  scoped_ptr<SettingsStorageFactory> delegate_;
+};
+
+// A SettingsStorageFactory which always returns NULL.
+class NullSettingsStorageFactory : public SettingsStorageFactory {
  public:
-   SettingsFrontendTest()
+  virtual ~NullSettingsStorageFactory() {}
+
+  // SettingsStorageFactory implementation.
+  virtual SettingsStorage* Create(
+      const FilePath& base_path, const std::string& extension_id) OVERRIDE {
+    return NULL;
+  }
+};
+
+}
+
+class ExtensionSettingsFrontendTest : public testing::Test {
+ public:
+   ExtensionSettingsFrontendTest()
       : ui_thread_(BrowserThread::UI, MessageLoop::current()),
         file_thread_(BrowserThread::FILE, MessageLoop::current()) {}
 
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     profile_.reset(new MockProfile(temp_dir_.path()));
-    frontend_.reset(new SettingsFrontend(profile_.get()));
+    ResetFrontend();
   }
 
   virtual void TearDown() OVERRIDE {
@@ -39,10 +79,20 @@ class SettingsFrontendTest : public testing::Test {
   }
 
  protected:
+  void ResetFrontend() {
+    storage_factory_ =
+        new ScopedSettingsStorageFactory(
+            new SettingsLeveldbStorage::Factory());
+    frontend_.reset(SettingsFrontend::Create(storage_factory_, profile_.get()));
+  }
+
   ScopedTempDir temp_dir_;
   scoped_ptr<MockProfile> profile_;
   scoped_ptr<SettingsFrontend> frontend_;
 
+  // Owned by |frontend_|.
+  ScopedSettingsStorageFactory* storage_factory_;
+
  private:
   MessageLoop message_loop_;
   content::TestBrowserThread ui_thread_;
@@ -53,7 +103,7 @@ class SettingsFrontendTest : public testing::Test {
 // alternating in each test.
 // TODO(kalman): explicitly test the two interact correctly.
 
-TEST_F(SettingsFrontendTest, SettingsPreservedAcrossReconstruction) {
+TEST_F(ExtensionSettingsFrontendTest, SettingsPreservedAcrossReconstruction) {
   const std::string id = "ext";
   profile_->GetMockExtensionService()->AddExtension(
       id, Extension::TYPE_EXTENSION);
@@ -74,7 +124,7 @@ TEST_F(SettingsFrontendTest, SettingsPreservedAcrossReconstruction) {
     EXPECT_FALSE(result.settings().empty());
   }
 
-  frontend_.reset(new SettingsFrontend(profile_.get()));
+  ResetFrontend();
   storage = GetStorage(id, frontend_.get());
 
   {
@@ -84,7 +134,7 @@ TEST_F(SettingsFrontendTest, SettingsPreservedAcrossReconstruction) {
   }
 }
 
-TEST_F(SettingsFrontendTest, SettingsClearedOnUninstall) {
+TEST_F(ExtensionSettingsFrontendTest, SettingsClearedOnUninstall) {
   const std::string id = "ext";
   profile_->GetMockExtensionService()->AddExtension(
       id, Extension::TYPE_PACKAGED_APP);
@@ -110,7 +160,7 @@ TEST_F(SettingsFrontendTest, SettingsClearedOnUninstall) {
   }
 }
 
-TEST_F(SettingsFrontendTest, LeveldbDatabaseDeletedFromDiskOnClear) {
+TEST_F(ExtensionSettingsFrontendTest, LeveldbDatabaseDeletedFromDiskOnClear) {
   const std::string id = "ext";
   profile_->GetMockExtensionService()->AddExtension(
       id, Extension::TYPE_EXTENSION);
@@ -140,4 +190,34 @@ TEST_F(SettingsFrontendTest, LeveldbDatabaseDeletedFromDiskOnClear) {
   //EXPECT_FALSE(file_util::PathExists(temp_dir_.path()));
 }
 
+TEST_F(ExtensionSettingsFrontendTest,
+    LeveldbCreationFailureFailsAllOperations) {
+  const StringValue bar("bar");
+  const std::string id = "ext";
+  profile_->GetMockExtensionService()->AddExtension(
+      id, Extension::TYPE_EXTENSION);
+
+  storage_factory_->Reset(new NullSettingsStorageFactory());
+
+  SettingsStorage* storage = GetStorage(id, frontend_.get());
+  ASSERT_TRUE(storage != NULL);
+
+  EXPECT_TRUE(storage->Get().HasError());
+  EXPECT_TRUE(storage->Clear().HasError());
+  EXPECT_TRUE(storage->Set("foo", bar).HasError());
+  EXPECT_TRUE(storage->Remove("foo").HasError());
+
+  // For simplicity: just always fail those requests, even if the leveldb
+  // storage areas start working.
+  storage_factory_->Reset(new SettingsLeveldbStorage::Factory());
+
+  storage = GetStorage(id, frontend_.get());
+  ASSERT_TRUE(storage != NULL);
+
+  EXPECT_TRUE(storage->Get().HasError());
+  EXPECT_TRUE(storage->Clear().HasError());
+  EXPECT_TRUE(storage->Set("foo", bar).HasError());
+  EXPECT_TRUE(storage->Remove("foo").HasError());
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/settings/settings_leveldb_storage.cc b/chrome/browser/extensions/settings/settings_leveldb_storage.cc
index 19e3b83..77971ca 100644
--- a/chrome/browser/extensions/settings/settings_leveldb_storage.cc
+++ b/chrome/browser/extensions/settings/settings_leveldb_storage.cc
@@ -47,10 +47,10 @@ class ScopedSnapshot {
 
 }  // namespace
 
-/* static */
-SettingsLeveldbStorage* SettingsLeveldbStorage::Create(
-      const FilePath& base_path,
-      const std::string& extension_id) {
+// SettingsLeveldbStorage::Factory
+
+SettingsStorage* SettingsLeveldbStorage::Factory::Create(
+    const FilePath& base_path, const std::string& extension_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   FilePath path = base_path.AppendASCII(extension_id);
 
@@ -72,6 +72,8 @@ SettingsLeveldbStorage* SettingsLeveldbStorage::Create(
   return new SettingsLeveldbStorage(path, db);
 }
 
+// SettingsLeveldbStorage
+
 SettingsLeveldbStorage::SettingsLeveldbStorage(
     const FilePath& db_path, leveldb::DB* db)
     : db_path_(db_path), db_(db) {
diff --git a/chrome/browser/extensions/settings/settings_leveldb_storage.h b/chrome/browser/extensions/settings/settings_leveldb_storage.h
index 95b0e12..7c3abe4 100644
--- a/chrome/browser/extensions/settings/settings_leveldb_storage.h
+++ b/chrome/browser/extensions/settings/settings_leveldb_storage.h
@@ -14,21 +14,31 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/task.h"
 #include "chrome/browser/extensions/settings/settings_storage.h"
+#include "chrome/browser/extensions/settings/settings_storage_factory.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 
-// Extension settings storage object, backed by a leveldb database.
-//
 namespace extensions {
 
+// Extension settings storage object, backed by a leveldb database.
+//
 // No caching is done; that should be handled by wrapping with an
 // SettingsStorageCache.
 // All methods must be run on the FILE thread.
 class SettingsLeveldbStorage : public SettingsStorage {
  public:
-  // Tries to create a leveldb storage area for an extension at a base path.
-  // Returns NULL if creation fails.
-  static SettingsLeveldbStorage* Create(
-      const FilePath& base_path, const std::string& extension_id);
+  // Factory for creating SettingsLeveldbStorage instances.
+  class Factory : public SettingsStorageFactory {
+   public:
+    Factory() {}
+    virtual ~Factory() {}
+
+    // SettingsStorageFactory implementation.
+    virtual SettingsStorage* Create(
+        const FilePath& base_path, const std::string& extension_id) OVERRIDE;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Factory);
+  };
 
   // Must be deleted on the FILE thread.
   virtual ~SettingsLeveldbStorage();
diff --git a/chrome/browser/extensions/settings/settings_leveldb_storage_unittest.cc b/chrome/browser/extensions/settings/settings_leveldb_storage_unittest.cc
index 3b9e25c..8e7ec39 100644
--- a/chrome/browser/extensions/settings/settings_leveldb_storage_unittest.cc
+++ b/chrome/browser/extensions/settings/settings_leveldb_storage_unittest.cc
@@ -12,14 +12,14 @@ namespace {
 
 SettingsStorage* Param(
     const FilePath& file_path, const std::string& extension_id) {
-  return SettingsLeveldbStorage::Create(file_path, extension_id);
+  return SettingsLeveldbStorage::Factory().Create(file_path, extension_id);
 }
 
 }  // namespace
 
 INSTANTIATE_TEST_CASE_P(
     SettingsLeveldbStorage,
-    SettingsStorageTest,
+    ExtensionSettingsStorageTest,
     testing::Values(&Param));
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/settings/settings_storage_cache_unittest.cc b/chrome/browser/extensions/settings/settings_storage_cache_unittest.cc
index 74e9e46..5524e9e 100644
--- a/chrome/browser/extensions/settings/settings_storage_cache_unittest.cc
+++ b/chrome/browser/extensions/settings/settings_storage_cache_unittest.cc
@@ -21,7 +21,7 @@ SettingsStorage* Param(
 
 INSTANTIATE_TEST_CASE_P(
     SettingsStorageCache,
-    SettingsStorageTest,
+    ExtensionSettingsStorageTest,
     testing::Values(&Param));
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/settings/settings_storage_factory.h b/chrome/browser/extensions/settings/settings_storage_factory.h
new file mode 100644
index 0000000..b39f45f
--- /dev/null
+++ b/chrome/browser/extensions/settings/settings_storage_factory.h
@@ -0,0 +1,28 @@
+// Copyright (c) 2011 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_EXTENSIONS_SETTINGS_SETTINGS_STORAGE_FACTORY_H_
+#define CHROME_BROWSER_EXTENSIONS_SETTINGS_SETTINGS_STORAGE_FACTORY_H_
+#pragma once
+
+class FilePath;
+
+namespace extensions {
+
+class SettingsStorage;
+
+// Factory for creating SettingStorage instances.
+class SettingsStorageFactory {
+ public:
+  virtual ~SettingsStorageFactory() {}
+
+  // Create a new SettingsLeveldbStorage area.  Return NULL to indicate
+  // failure.  Must be called on the FILE thread.
+  virtual SettingsStorage* Create(
+      const FilePath& base_path, const std::string& extension_id) = 0;
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_SETTINGS_SETTINGS_STORAGE_FACTORY_H_
diff --git a/chrome/browser/extensions/settings/settings_storage_unittest.cc b/chrome/browser/extensions/settings/settings_storage_unittest.cc
index 9c42032..8f78abe 100644
--- a/chrome/browser/extensions/settings/settings_storage_unittest.cc
+++ b/chrome/browser/extensions/settings/settings_storage_unittest.cc
@@ -134,7 +134,7 @@ testing::AssertionResult ChangesEq(
   return testing::AssertionSuccess();
 }
 
-SettingsStorageTest::SettingsStorageTest()
+ExtensionSettingsStorageTest::ExtensionSettingsStorageTest()
     : key1_("foo"),
       key2_("bar"),
       key3_("baz"),
@@ -176,26 +176,26 @@ SettingsStorageTest::SettingsStorageTest()
   dict123_->Set(key3_, val3_->DeepCopy());
 }
 
-SettingsStorageTest::~SettingsStorageTest() {}
+ExtensionSettingsStorageTest::~ExtensionSettingsStorageTest() {}
 
-void SettingsStorageTest::SetUp() {
+void ExtensionSettingsStorageTest::SetUp() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   storage_.reset((GetParam())(temp_dir_.path(), "fakeExtension"));
   ASSERT_TRUE(storage_.get());
 }
 
-void SettingsStorageTest::TearDown() {
+void ExtensionSettingsStorageTest::TearDown() {
   storage_.reset();
 }
 
-TEST_P(SettingsStorageTest, GetWhenEmpty) {
+TEST_P(ExtensionSettingsStorageTest, GetWhenEmpty) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(key1_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(empty_list_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get(list123_));
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, GetWithSingleValue) {
+TEST_P(ExtensionSettingsStorageTest, GetWithSingleValue) {
   {
     SettingChangeList changes;
     changes.push_back(SettingChange(key1_, NULL, val1_->DeepCopy()));
@@ -210,7 +210,7 @@ TEST_P(SettingsStorageTest, GetWithSingleValue) {
   EXPECT_PRED_FORMAT2(SettingsEq, *dict1_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, GetWithMultipleValues) {
+TEST_P(ExtensionSettingsStorageTest, GetWithMultipleValues) {
   {
     SettingChangeList changes;
     changes.push_back(SettingChange(key1_, NULL, val1_->DeepCopy()));
@@ -225,7 +225,7 @@ TEST_P(SettingsStorageTest, GetWithMultipleValues) {
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, RemoveWhenEmpty) {
+TEST_P(ExtensionSettingsStorageTest, RemoveWhenEmpty) {
   EXPECT_PRED_FORMAT2(ChangesEq,
       SettingChangeList(), storage_->Remove(key1_));
 
@@ -234,7 +234,7 @@ TEST_P(SettingsStorageTest, RemoveWhenEmpty) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, RemoveWithSingleValue) {
+TEST_P(ExtensionSettingsStorageTest, RemoveWithSingleValue) {
   storage_->Set(*dict1_);
   {
     SettingChangeList changes;
@@ -249,7 +249,7 @@ TEST_P(SettingsStorageTest, RemoveWithSingleValue) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, RemoveWithMultipleValues) {
+TEST_P(ExtensionSettingsStorageTest, RemoveWithMultipleValues) {
   storage_->Set(*dict123_);
   {
     SettingChangeList changes;
@@ -283,7 +283,7 @@ TEST_P(SettingsStorageTest, RemoveWithMultipleValues) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, SetWhenOverwriting) {
+TEST_P(ExtensionSettingsStorageTest, SetWhenOverwriting) {
   storage_->Set(key1_, *val2_);
   {
     SettingChangeList changes;
@@ -303,7 +303,7 @@ TEST_P(SettingsStorageTest, SetWhenOverwriting) {
   EXPECT_PRED_FORMAT2(SettingsEq, *dict12_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, ClearWhenEmpty) {
+TEST_P(ExtensionSettingsStorageTest, ClearWhenEmpty) {
   EXPECT_PRED_FORMAT2(ChangesEq,
       SettingChangeList(), storage_->Clear());
 
@@ -313,7 +313,7 @@ TEST_P(SettingsStorageTest, ClearWhenEmpty) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, ClearWhenNotEmpty) {
+TEST_P(ExtensionSettingsStorageTest, ClearWhenNotEmpty) {
   storage_->Set(*dict12_);
   {
     SettingChangeList changes;
@@ -330,7 +330,7 @@ TEST_P(SettingsStorageTest, ClearWhenNotEmpty) {
 
 // Dots should be allowed in key names; they shouldn't be interpreted as
 // indexing into a dictionary.
-TEST_P(SettingsStorageTest, DotsInKeyNames) {
+TEST_P(ExtensionSettingsStorageTest, DotsInKeyNames) {
   std::string dot_key("foo.bar");
   StringValue dot_value("baz.qux");
   std::vector<std::string> dot_list;
@@ -380,7 +380,7 @@ TEST_P(SettingsStorageTest, DotsInKeyNames) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get());
 }
 
-TEST_P(SettingsStorageTest, DotsInKeyNamesWithDicts) {
+TEST_P(ExtensionSettingsStorageTest, DotsInKeyNamesWithDicts) {
   DictionaryValue outer_dict;
   DictionaryValue* inner_dict = new DictionaryValue();
   outer_dict.Set("foo", inner_dict);
@@ -397,7 +397,7 @@ TEST_P(SettingsStorageTest, DotsInKeyNamesWithDicts) {
   EXPECT_PRED_FORMAT2(SettingsEq, *empty_dict_, storage_->Get("foo.bar"));
 }
 
-TEST_P(SettingsStorageTest, ComplexChangedKeysScenarios) {
+TEST_P(ExtensionSettingsStorageTest, ComplexChangedKeysScenarios) {
   // Test:
   //   - Setting over missing/changed/same keys, combinations.
   //   - Removing over missing and present keys, combinations.
diff --git a/chrome/browser/extensions/settings/settings_storage_unittest.h b/chrome/browser/extensions/settings/settings_storage_unittest.h
index 4cb2ab8..dc2079f 100644
--- a/chrome/browser/extensions/settings/settings_storage_unittest.h
+++ b/chrome/browser/extensions/settings/settings_storage_unittest.h
@@ -28,11 +28,11 @@ typedef SettingsStorage* (*SettingsStorageTestParam)(
 // Test fixture for SettingsStorage tests.  Tests are defined in
 // settings_storage_unittest.cc with configurations for both cached
 // and non-cached leveldb storage, and cached no-op storage.
-class SettingsStorageTest
+class ExtensionSettingsStorageTest
   : public testing::TestWithParam<SettingsStorageTestParam> {
  public:
-  SettingsStorageTest();
-  virtual ~SettingsStorageTest();
+  ExtensionSettingsStorageTest();
+  virtual ~ExtensionSettingsStorageTest();
 
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
diff --git a/chrome/browser/extensions/settings/settings_sync_unittest.cc b/chrome/browser/extensions/settings/settings_sync_unittest.cc
index ae8e25f..5cf9039 100644
--- a/chrome/browser/extensions/settings/settings_sync_unittest.cc
+++ b/chrome/browser/extensions/settings/settings_sync_unittest.cc
@@ -136,7 +136,7 @@ class ExtensionSettingsSyncTest : public testing::Test {
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     profile_.reset(new MockProfile(temp_dir_.path()));
-    frontend_.reset(new SettingsFrontend(profile_.get()));
+    frontend_.reset(SettingsFrontend::Create(profile_.get()));
   }
 
   virtual void TearDown() OVERRIDE {
diff --git a/chrome/browser/extensions/settings/settings_test_util.cc b/chrome/browser/extensions/settings/settings_test_util.cc
index 38869ac..11eb12f 100644
--- a/chrome/browser/extensions/settings/settings_test_util.cc
+++ b/chrome/browser/extensions/settings/settings_test_util.cc
@@ -6,14 +6,14 @@
 
 #include "base/file_path.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/browser/extensions/settings/settings_frontend.h"
 
 namespace extensions {
 
 namespace settings_test_util {
 
 // Intended as a StorageCallback from GetStorage.
-static void AssignStorage(
-    SettingsStorage** dst, SettingsStorage* src) {
+static void AssignStorage(SettingsStorage** dst, SettingsStorage* src) {
   *dst = src;
 }
 
diff --git a/chrome/browser/extensions/settings/settings_test_util.h b/chrome/browser/extensions/settings/settings_test_util.h
index bad77a5..536093c 100644
--- a/chrome/browser/extensions/settings/settings_test_util.h
+++ b/chrome/browser/extensions/settings/settings_test_util.h
@@ -8,18 +8,20 @@
 
 #include <string>
 
-#include "base/memory/scoped_ptr.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/test_extension_service.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/test/base/testing_profile.h"
 
-class SettingsFrontend;
 
 namespace extensions {
 
+class SettingsFrontend;
+class SettingsStorage;
+
 // Utilities for extension settings API tests.
 namespace settings_test_util {