diff options
50 files changed, 1123 insertions, 507 deletions
diff --git a/chrome/browser/extensions/api/chrome_extensions_api_client.cc b/chrome/browser/extensions/api/chrome_extensions_api_client.cc index 3f447c2..b9b2b09 100644 --- a/chrome/browser/extensions/api/chrome_extensions_api_client.cc +++ b/chrome/browser/extensions/api/chrome_extensions_api_client.cc @@ -51,7 +51,7 @@ ChromeExtensionsAPIClient::~ChromeExtensionsAPIClient() {} void ChromeExtensionsAPIClient::AddAdditionalValueStoreCaches( content::BrowserContext* context, - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<base::ObserverListThreadSafe<SettingsObserver>>& observers, std::map<settings_namespace::Namespace, ValueStoreCache*>* caches) { diff --git a/chrome/browser/extensions/api/chrome_extensions_api_client.h b/chrome/browser/extensions/api/chrome_extensions_api_client.h index c12603e..4333730 100644 --- a/chrome/browser/extensions/api/chrome_extensions_api_client.h +++ b/chrome/browser/extensions/api/chrome_extensions_api_client.h @@ -20,7 +20,7 @@ class ChromeExtensionsAPIClient : public ExtensionsAPIClient { // ExtensionsApiClient implementation. void AddAdditionalValueStoreCaches( content::BrowserContext* context, - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<base::ObserverListThreadSafe<SettingsObserver>>& observers, std::map<settings_namespace::Namespace, ValueStoreCache*>* caches) diff --git a/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc b/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc index c53bb74..63d6326 100644 --- a/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc +++ b/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc @@ -226,8 +226,6 @@ TEST_F(RulesRegistryWithCacheTest, OnExtensionUninstalled) { TEST_F(RulesRegistryWithCacheTest, DeclarativeRulesStored) { ExtensionPrefs* extension_prefs = env_.GetExtensionPrefs(); - // The value store is first created during GetExtensionService. - TestingValueStore* store = env_.GetExtensionSystem()->value_store(); const std::string event_name("testEvent"); const std::string rules_stored_key( @@ -251,15 +249,21 @@ TEST_F(RulesRegistryWithCacheTest, DeclarativeRulesStored) { EXPECT_TRUE(cache_delegate->GetDeclarativeRulesStored(extension1_->id())); // 2. Test writing behavior. - int write_count = store->write_count(); + // The ValueStore is lazily created when reading/writing. None done yet so + // verify that not yet created. + TestingValueStore* store = env_.GetExtensionSystem()->value_store(); + // No write yet so no store should have been created. + ASSERT_FALSE(store); scoped_ptr<base::ListValue> value(new base::ListValue); value->AppendBoolean(true); cache_delegate->WriteToStorage(extension1_->id(), std::move(value)); EXPECT_TRUE(cache_delegate->GetDeclarativeRulesStored(extension1_->id())); base::RunLoop().RunUntilIdle(); - EXPECT_EQ(write_count + 1, store->write_count()); - write_count = store->write_count(); + store = env_.GetExtensionSystem()->value_store(); + ASSERT_TRUE(store); + EXPECT_EQ(1, store->write_count()); + int write_count = store->write_count(); value.reset(new base::ListValue); cache_delegate->WriteToStorage(extension1_->id(), std::move(value)); @@ -378,14 +382,15 @@ TEST_F(RulesRegistryWithCacheTest, ConcurrentStoringOfRules) { // write a rules update for extension A, just because it is immediately // followed by a rules update for extension B. extensions::TestExtensionSystem* system = env_.GetExtensionSystem(); - TestingValueStore* store = system->value_store(); + // ValueStore created lazily, assure none currently exists. + ASSERT_TRUE(system->value_store() == nullptr); - int write_count = store->write_count(); + int write_count = 0; EXPECT_EQ("", AddRule(extension1_->id(), kRuleId)); EXPECT_EQ("", AddRule(extension2_->id(), kRule2Id)); env_.GetExtensionSystem()->SetReady(); base::RunLoop().RunUntilIdle(); - EXPECT_EQ(write_count + 2, store->write_count()); + EXPECT_EQ(write_count + 2, system->value_store()->write_count()); } } // namespace extensions diff --git a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc index 13130c1..78c4643 100644 --- a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc +++ b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc @@ -26,12 +26,12 @@ #include "components/policy/core/common/schema_map.h" #include "components/policy/core/common/schema_registry.h" #include "content/public/browser/browser_thread.h" -#include "extensions/browser/api/storage/settings_storage_factory.h" #include "extensions/browser/extension_prefs.h" #include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_registry_observer.h" #include "extensions/browser/extension_system.h" #include "extensions/browser/value_store/value_store_change.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "extensions/common/api/storage.h" #include "extensions/common/constants.h" #include "extensions/common/extension.h" @@ -60,6 +60,10 @@ const char kLoadSchemasBackgroundTaskTokenName[] = const char kLegacyBrowserSupportExtensionId[] = "heildphpnddilhkemkielfhnkaagiabh"; +// Only extension settings are stored in the managed namespace - not apps. +const ValueStoreFactory::ModelType kManagedModelType = + ValueStoreFactory::ModelType::EXTENSION; + } // namespace // This helper observes initialization of all the installed extensions and @@ -234,16 +238,14 @@ void ManagedValueStoreCache::ExtensionTracker::Register( ManagedValueStoreCache::ManagedValueStoreCache( BrowserContext* context, - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<SettingsObserverList>& observers) : profile_(Profile::FromBrowserContext(context)), policy_service_( policy::ProfilePolicyConnectorFactory::GetForBrowserContext(context) ->policy_service()), storage_factory_(factory), - observers_(observers), - base_path_(profile_->GetPath().AppendASCII( - extensions::kManagedSettingsDirectoryName)) { + observers_(observers) { DCHECK_CURRENTLY_ON(BrowserThread::UI); policy_service_->AddObserver(policy::POLICY_DOMAIN_EXTENSIONS, this); @@ -361,17 +363,18 @@ PolicyValueStore* ManagedValueStoreCache::GetStoreFor( // Create the store now, and serve the cached policy until the PolicyService // sends updated values. PolicyValueStore* store = new PolicyValueStore( - extension_id, - observers_, - make_scoped_ptr(storage_factory_->Create(base_path_, extension_id))); + extension_id, observers_, + storage_factory_->CreateSettingsStore(settings_namespace::MANAGED, + kManagedModelType, extension_id)); store_map_[extension_id] = make_linked_ptr(store); return store; } bool ManagedValueStoreCache::HasStore(const std::string& extension_id) const { - // TODO(joaodasilva): move this check to a ValueStore method. - return base::DirectoryExists(base_path_.AppendASCII(extension_id)); + // Note: Currently only manage extensions (not apps). + return storage_factory_->HasSettings(settings_namespace::MANAGED, + kManagedModelType, extension_id); } } // namespace extensions diff --git a/chrome/browser/extensions/api/storage/managed_value_store_cache.h b/chrome/browser/extensions/api/storage/managed_value_store_cache.h index aab9968..b76793e 100644 --- a/chrome/browser/extensions/api/storage/managed_value_store_cache.h +++ b/chrome/browser/extensions/api/storage/managed_value_store_cache.h @@ -31,7 +31,7 @@ class PolicyMap; namespace extensions { class PolicyValueStore; -class SettingsStorageFactory; +class ValueStoreFactory; // A ValueStoreCache that manages a PolicyValueStore for each extension that // uses the storage.managed namespace. This class observes policy changes and @@ -44,7 +44,7 @@ class ManagedValueStoreCache : public ValueStoreCache, // |observers| is the list of SettingsObservers to notify when a ValueStore // changes. ManagedValueStoreCache(content::BrowserContext* context, - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<SettingsObserverList>& observers); ~ManagedValueStoreCache() override; @@ -91,9 +91,8 @@ class ManagedValueStoreCache : public ValueStoreCache, scoped_ptr<ExtensionTracker> extension_tracker_; // These live on the FILE thread. - scoped_refptr<SettingsStorageFactory> storage_factory_; + scoped_refptr<ValueStoreFactory> storage_factory_; scoped_refptr<SettingsObserverList> observers_; - base::FilePath base_path_; // All the PolicyValueStores live on the FILE thread, and |store_map_| can be // accessed only on the FILE thread as well. diff --git a/chrome/browser/extensions/api/storage/settings_sync_unittest.cc b/chrome/browser/extensions/api/storage/settings_sync_unittest.cc index 3aff3d7..cf55beb 100644 --- a/chrome/browser/extensions/api/storage/settings_sync_unittest.cc +++ b/chrome/browser/extensions/api/storage/settings_sync_unittest.cc @@ -19,14 +19,13 @@ #include "chrome/browser/extensions/api/storage/syncable_settings_storage.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread.h" -#include "extensions/browser/api/storage/leveldb_settings_storage_factory.h" -#include "extensions/browser/api/storage/settings_storage_factory.h" #include "extensions/browser/api/storage/settings_test_util.h" #include "extensions/browser/api/storage/storage_frontend.h" #include "extensions/browser/event_router.h" #include "extensions/browser/event_router_factory.h" #include "extensions/browser/extension_system.h" #include "extensions/browser/mock_extension_system.h" +#include "extensions/browser/value_store/test_value_store_factory.h" #include "extensions/browser/value_store/testing_value_store.h" #include "extensions/common/manifest.h" #include "sync/api/sync_change_processor.h" @@ -167,38 +166,6 @@ class MockSyncChangeProcessor : public syncer::SyncChangeProcessor { bool fail_all_requests_; }; -// SettingsStorageFactory which always returns TestingValueStore objects, -// and allows individually created objects to be returned. -class TestingValueStoreFactory : public SettingsStorageFactory { - public: - TestingValueStore* GetExisting(const std::string& extension_id) { - DCHECK(created_.count(extension_id)); - return created_[extension_id]; - } - - // SettingsStorageFactory implementation. - ValueStore* Create(const base::FilePath& base_path, - const std::string& extension_id) override { - TestingValueStore* new_storage = new TestingValueStore(); - DCHECK(!created_.count(extension_id)); - created_[extension_id] = new_storage; - return new_storage; - } - - // Testing value stores don't actually create a real database. Don't delete - // any files. - void DeleteDatabaseIfExists(const base::FilePath& base_path, - const std::string& extension_id) override {} - - private: - // SettingsStorageFactory is refcounted. - ~TestingValueStoreFactory() override {} - - // None of these storage areas are owned by this factory, so care must be - // taken when calling GetExisting. - std::map<std::string, TestingValueStore*> created_; -}; - scoped_ptr<KeyedService> MockExtensionSystemFactoryFunction( content::BrowserContext* context) { return make_scoped_ptr(new MockExtensionSystem(context)); @@ -215,7 +182,7 @@ class ExtensionSettingsSyncTest : public testing::Test { ExtensionSettingsSyncTest() : ui_thread_(BrowserThread::UI, base::MessageLoop::current()), file_thread_(BrowserThread::FILE, base::MessageLoop::current()), - storage_factory_(new util::ScopedSettingsStorageFactory()), + storage_factory_(new TestValueStoreFactory()), sync_processor_(new MockSyncChangeProcessor), sync_processor_wrapper_(new syncer::SyncChangeProcessorWrapperForTest( sync_processor_.get())) {} @@ -223,7 +190,7 @@ class ExtensionSettingsSyncTest : public testing::Test { void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); profile_.reset(new TestingProfile(temp_dir_.path())); - storage_factory_->Reset(new LeveldbSettingsStorageFactory()); + storage_factory_->Reset(); frontend_ = StorageFrontend::CreateForTesting(storage_factory_, profile_.get()); @@ -279,6 +246,13 @@ class ExtensionSettingsSyncTest : public testing::Test { return as_map; } + // This class uses it's TestingValueStore in such a way that it always mints + // new TestingValueStore instances. + TestingValueStore* GetExisting(const ExtensionId& extension_id) { + return static_cast<TestingValueStore*>( + storage_factory_->GetExisting(extension_id)); + } + // Need these so that the DCHECKs for running on FILE or UI threads pass. base::MessageLoop message_loop_; content::TestBrowserThread ui_thread_; @@ -287,7 +261,7 @@ class ExtensionSettingsSyncTest : public testing::Test { base::ScopedTempDir temp_dir_; scoped_ptr<TestingProfile> profile_; scoped_ptr<StorageFrontend> frontend_; - scoped_refptr<util::ScopedSettingsStorageFactory> storage_factory_; + scoped_refptr<TestValueStoreFactory> storage_factory_; scoped_ptr<MockSyncChangeProcessor> sync_processor_; scoped_ptr<syncer::SyncChangeProcessorWrapperForTest> sync_processor_wrapper_; }; @@ -709,14 +683,12 @@ TEST_F(ExtensionSettingsSyncTest, FailingStartSyncingDisablesSync) { // There is a bit of a convoluted method to get storage areas that can fail; // hand out TestingValueStore object then toggle them failing/succeeding // as necessary. - TestingValueStoreFactory* testing_factory = new TestingValueStoreFactory(); - storage_factory_->Reset(testing_factory); ValueStore* good = AddExtensionAndGetStorage("good", type); ValueStore* bad = AddExtensionAndGetStorage("bad", type); // Make bad fail for incoming sync changes. - testing_factory->GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); + GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); { syncer::SyncDataList sync_data; sync_data.push_back(settings_sync_util::CreateData( @@ -728,7 +700,7 @@ TEST_F(ExtensionSettingsSyncTest, FailingStartSyncingDisablesSync) { model_type, sync_data, std::move(sync_processor_wrapper_), make_scoped_ptr(new syncer::SyncErrorFactoryMock())); } - testing_factory->GetExisting("bad")->set_status_code(ValueStore::OK); + GetExisting("bad")->set_status_code(ValueStore::OK); { base::DictionaryValue dict; @@ -809,7 +781,7 @@ TEST_F(ExtensionSettingsSyncTest, FailingStartSyncingDisablesSync) { } // Failing ProcessSyncChanges shouldn't go to the storage. - testing_factory->GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); + GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); { syncer::SyncChangeList change_list; change_list.push_back(settings_sync_util::CreateUpdate( @@ -819,7 +791,7 @@ TEST_F(ExtensionSettingsSyncTest, FailingStartSyncingDisablesSync) { "bad", "foo", fooValue, model_type)); GetSyncableService(model_type)->ProcessSyncChanges(FROM_HERE, change_list); } - testing_factory->GetExisting("bad")->set_status_code(ValueStore::OK); + GetExisting("bad")->set_status_code(ValueStore::OK); { base::DictionaryValue dict; @@ -897,9 +869,6 @@ TEST_F(ExtensionSettingsSyncTest, FailingProcessChangesDisablesSync) { base::StringValue fooValue("fooValue"); base::StringValue barValue("barValue"); - TestingValueStoreFactory* testing_factory = new TestingValueStoreFactory(); - storage_factory_->Reset(testing_factory); - ValueStore* good = AddExtensionAndGetStorage("good", type); ValueStore* bad = AddExtensionAndGetStorage("bad", type); @@ -930,7 +899,7 @@ TEST_F(ExtensionSettingsSyncTest, FailingProcessChangesDisablesSync) { } // Now fail ProcessSyncChanges for bad. - testing_factory->GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); + GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); { syncer::SyncChangeList change_list; change_list.push_back(settings_sync_util::CreateAdd( @@ -939,7 +908,7 @@ TEST_F(ExtensionSettingsSyncTest, FailingProcessChangesDisablesSync) { "bad", "bar", barValue, model_type)); GetSyncableService(model_type)->ProcessSyncChanges(FROM_HERE, change_list); } - testing_factory->GetExisting("bad")->set_status_code(ValueStore::OK); + GetExisting("bad")->set_status_code(ValueStore::OK); { base::DictionaryValue dict; @@ -992,9 +961,6 @@ TEST_F(ExtensionSettingsSyncTest, FailingGetAllSyncDataDoesntStopSync) { base::StringValue fooValue("fooValue"); base::StringValue barValue("barValue"); - TestingValueStoreFactory* testing_factory = new TestingValueStoreFactory(); - storage_factory_->Reset(testing_factory); - ValueStore* good = AddExtensionAndGetStorage("good", type); ValueStore* bad = AddExtensionAndGetStorage("bad", type); @@ -1003,14 +969,14 @@ TEST_F(ExtensionSettingsSyncTest, FailingGetAllSyncDataDoesntStopSync) { // Even though bad will fail to get all sync data, sync data should still // include that from good. - testing_factory->GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); + GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); { syncer::SyncDataList all_sync_data = GetSyncableService(model_type)->GetAllSyncData(model_type); EXPECT_EQ(1u, all_sync_data.size()); EXPECT_EQ("good/foo", syncer::SyncDataLocal(all_sync_data[0]).GetTag()); } - testing_factory->GetExisting("bad")->set_status_code(ValueStore::OK); + GetExisting("bad")->set_status_code(ValueStore::OK); // Sync shouldn't be disabled for good (nor bad -- but this is unimportant). GetSyncableService(model_type) @@ -1043,9 +1009,6 @@ TEST_F(ExtensionSettingsSyncTest, FailureToReadChangesToPushDisablesSync) { base::StringValue fooValue("fooValue"); base::StringValue barValue("barValue"); - TestingValueStoreFactory* testing_factory = new TestingValueStoreFactory(); - storage_factory_->Reset(testing_factory); - ValueStore* good = AddExtensionAndGetStorage("good", type); ValueStore* bad = AddExtensionAndGetStorage("bad", type); @@ -1054,13 +1017,13 @@ TEST_F(ExtensionSettingsSyncTest, FailureToReadChangesToPushDisablesSync) { // good will successfully push foo:fooValue to sync, but bad will fail to // get them so won't. - testing_factory->GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); + GetExisting("bad")->set_status_code(ValueStore::CORRUPTION); GetSyncableService(model_type) ->MergeDataAndStartSyncing( model_type, syncer::SyncDataList(), std::move(sync_processor_wrapper_), make_scoped_ptr(new syncer::SyncErrorFactoryMock())); - testing_factory->GetExisting("bad")->set_status_code(ValueStore::OK); + GetExisting("bad")->set_status_code(ValueStore::OK); EXPECT_EQ(syncer::SyncChange::ACTION_ADD, sync_processor_->GetOnlyChange("good", "foo")->change_type()); @@ -1139,9 +1102,6 @@ TEST_F(ExtensionSettingsSyncTest, FailureToPushLocalStateDisablesSync) { base::StringValue fooValue("fooValue"); base::StringValue barValue("barValue"); - TestingValueStoreFactory* testing_factory = new TestingValueStoreFactory(); - storage_factory_->Reset(testing_factory); - ValueStore* good = AddExtensionAndGetStorage("good", type); ValueStore* bad = AddExtensionAndGetStorage("bad", type); @@ -1224,9 +1184,6 @@ TEST_F(ExtensionSettingsSyncTest, FailureToPushLocalChangeDisablesSync) { base::StringValue fooValue("fooValue"); base::StringValue barValue("barValue"); - TestingValueStoreFactory* testing_factory = new TestingValueStoreFactory(); - storage_factory_->Reset(testing_factory); - ValueStore* good = AddExtensionAndGetStorage("good", type); ValueStore* bad = AddExtensionAndGetStorage("bad", type); diff --git a/chrome/browser/extensions/api/storage/sync_storage_backend.cc b/chrome/browser/extensions/api/storage/sync_storage_backend.cc index bf1a0bd..ea17e60 100644 --- a/chrome/browser/extensions/api/storage/sync_storage_backend.cc +++ b/chrome/browser/extensions/api/storage/sync_storage_backend.cc @@ -6,7 +6,6 @@ #include <utility> -#include "base/files/file_enumerator.h" #include "base/logging.h" #include "chrome/browser/extensions/api/storage/settings_sync_processor.h" #include "chrome/browser/extensions/api/storage/settings_sync_util.h" @@ -34,17 +33,27 @@ scoped_ptr<base::DictionaryValue> EmptyDictionaryValue() { return make_scoped_ptr(new base::DictionaryValue()); } +ValueStoreFactory::ModelType ToFactoryModelType(syncer::ModelType sync_type) { + switch (sync_type) { + case syncer::APP_SETTINGS: + return ValueStoreFactory::ModelType::APP; + case syncer::EXTENSION_SETTINGS: + return ValueStoreFactory::ModelType::EXTENSION; + default: + NOTREACHED(); + } + return ValueStoreFactory::ModelType::EXTENSION; +} + } // namespace SyncStorageBackend::SyncStorageBackend( - const scoped_refptr<SettingsStorageFactory>& storage_factory, - const base::FilePath& base_path, + const scoped_refptr<ValueStoreFactory>& storage_factory, const SettingsStorageQuotaEnforcer::Limits& quota, const scoped_refptr<SettingsObserverList>& observers, syncer::ModelType sync_type, const syncer::SyncableService::StartSyncFlare& flare) : storage_factory_(storage_factory), - base_path_(base_path), quota_(quota), observers_(observers), sync_type_(sync_type), @@ -73,7 +82,9 @@ SyncableSettingsStorage* SyncStorageBackend::GetOrCreateStorageWithSyncData( scoped_ptr<SettingsStorageQuotaEnforcer> storage( new SettingsStorageQuotaEnforcer( - quota_, storage_factory_->Create(base_path_, extension_id))); + quota_, storage_factory_->CreateSettingsStore( + settings_namespace::SYNC, ToFactoryModelType(sync_type_), + extension_id))); // It's fine to create the quota enforcer underneath the sync layer, since // sync will only go ahead if each underlying storage operation succeeds. @@ -109,7 +120,8 @@ void SyncStorageBackend::DeleteStorage(const std::string& extension_id) { storage_objs_.erase(extension_id); } -std::set<std::string> SyncStorageBackend::GetKnownExtensionIDs() const { +std::set<std::string> SyncStorageBackend::GetKnownExtensionIDs( + ValueStoreFactory::ModelType model_type) const { DCHECK_CURRENTLY_ON(BrowserThread::FILE); std::set<std::string> result; @@ -119,18 +131,9 @@ std::set<std::string> SyncStorageBackend::GetKnownExtensionIDs() const { result.insert(storage_obj.first); } - // Leveldb databases are directories inside |base_path_|. - base::FileEnumerator extension_dirs( - base_path_, false, base::FileEnumerator::DIRECTORIES); - while (!extension_dirs.Next().empty()) { - base::FilePath extension_dir = extension_dirs.GetInfo().GetName(); - DCHECK(!extension_dir.IsAbsolute()); - // Extension IDs are created as std::strings so they *should* be ASCII. - std::string maybe_as_ascii(extension_dir.MaybeAsASCII()); - if (!maybe_as_ascii.empty()) { - result.insert(maybe_as_ascii); - } - } + std::set<std::string> disk_ids = storage_factory_->GetKnownExtensionIDs( + settings_namespace::SYNC, model_type); + result.insert(disk_ids.begin(), disk_ids.end()); return result; } @@ -138,14 +141,11 @@ std::set<std::string> SyncStorageBackend::GetKnownExtensionIDs() const { syncer::SyncDataList SyncStorageBackend::GetAllSyncData(syncer::ModelType type) const { DCHECK_CURRENTLY_ON(BrowserThread::FILE); - // Ignore the type, it's just for sanity checking; assume that whatever base - // path we're constructed with is correct for the sync type. - DCHECK(type == syncer::EXTENSION_SETTINGS || type == syncer::APP_SETTINGS); - // For all extensions, get all their settings. This has the effect // of bringing in the entire state of extension settings in memory; sad. syncer::SyncDataList all_sync_data; - std::set<std::string> known_extension_ids(GetKnownExtensionIDs()); + std::set<std::string> known_extension_ids( + GetKnownExtensionIDs(ToFactoryModelType(type))); for (std::set<std::string>::const_iterator it = known_extension_ids.begin(); it != known_extension_ids.end(); diff --git a/chrome/browser/extensions/api/storage/sync_storage_backend.h b/chrome/browser/extensions/api/storage/sync_storage_backend.h index 584a354..fe95c81 100644 --- a/chrome/browser/extensions/api/storage/sync_storage_backend.h +++ b/chrome/browser/extensions/api/storage/sync_storage_backend.h @@ -16,8 +16,8 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "extensions/browser/api/storage/settings_observer.h" -#include "extensions/browser/api/storage/settings_storage_factory.h" #include "extensions/browser/api/storage/settings_storage_quota_enforcer.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "sync/api/syncable_service.h" namespace syncer { @@ -28,6 +28,7 @@ namespace extensions { class SettingsSyncProcessor; class SyncableSettingsStorage; +class ValueStoreFactory; // Manages ValueStore objects for extensions, including routing // changes from sync to them. @@ -35,16 +36,12 @@ class SyncableSettingsStorage; class SyncStorageBackend : public syncer::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. - SyncStorageBackend( - const scoped_refptr<SettingsStorageFactory>& storage_factory, - const base::FilePath& base_path, - const SettingsStorageQuotaEnforcer::Limits& quota, - const scoped_refptr<SettingsObserverList>& observers, - syncer::ModelType sync_type, - const syncer::SyncableService::StartSyncFlare& flare); + SyncStorageBackend(const scoped_refptr<ValueStoreFactory>& storage_factory, + const SettingsStorageQuotaEnforcer::Limits& quota, + const scoped_refptr<SettingsObserverList>& observers, + syncer::ModelType sync_type, + const syncer::SyncableService::StartSyncFlare& flare); ~SyncStorageBackend() override; @@ -72,17 +69,15 @@ class SyncStorageBackend : public syncer::SyncableService { // Gets all extension IDs known to extension settings. This may not be all // installed extensions. - std::set<std::string> GetKnownExtensionIDs() const; + std::set<std::string> GetKnownExtensionIDs( + ValueStoreFactory::ModelType model_type) const; // Creates a new SettingsSyncProcessor for an extension. scoped_ptr<SettingsSyncProcessor> CreateSettingsSyncProcessor( const std::string& extension_id) const; // The Factory to use for creating new ValueStores. - const scoped_refptr<SettingsStorageFactory> storage_factory_; - - // The base file path to use when creating new ValueStores. - const base::FilePath base_path_; + const scoped_refptr<ValueStoreFactory> storage_factory_; // Quota limits (see SettingsStorageQuotaEnforcer). const SettingsStorageQuotaEnforcer::Limits quota_; diff --git a/chrome/browser/extensions/api/storage/sync_value_store_cache.cc b/chrome/browser/extensions/api/storage/sync_value_store_cache.cc index 738c496..13dbc46 100644 --- a/chrome/browser/extensions/api/storage/sync_value_store_cache.cc +++ b/chrome/browser/extensions/api/storage/sync_value_store_cache.cc @@ -6,18 +6,11 @@ #include <stddef.h> -#include "base/bind.h" -#include "base/callback.h" -#include "base/files/file_path.h" -#include "base/sequenced_task_runner.h" #include "chrome/browser/extensions/api/storage/sync_storage_backend.h" #include "chrome/browser/sync/glue/sync_start_util.h" #include "content/public/browser/browser_thread.h" -#include "extensions/browser/api/storage/settings_storage_quota_enforcer.h" -#include "extensions/browser/api/storage/storage_frontend.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "extensions/common/api/storage.h" -#include "extensions/common/constants.h" -#include "extensions/common/extension.h" using content::BrowserThread; @@ -38,7 +31,7 @@ SettingsStorageQuotaEnforcer::Limits GetSyncQuotaLimits() { } // namespace SyncValueStoreCache::SyncValueStoreCache( - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<SettingsObserverList>& observers, const base::FilePath& profile_path) : initialized_(false) { @@ -91,21 +84,19 @@ void SyncValueStoreCache::DeleteStorageSoon(const std::string& extension_id) { } void SyncValueStoreCache::InitOnFileThread( - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<SettingsObserverList>& observers, const base::FilePath& profile_path) { DCHECK_CURRENTLY_ON(BrowserThread::FILE); DCHECK(!initialized_); app_backend_.reset(new SyncStorageBackend( factory, - profile_path.AppendASCII(kSyncAppSettingsDirectoryName), GetSyncQuotaLimits(), observers, syncer::APP_SETTINGS, sync_start_util::GetFlareForSyncableService(profile_path))); extension_backend_.reset(new SyncStorageBackend( factory, - profile_path.AppendASCII(kSyncExtensionSettingsDirectoryName), GetSyncQuotaLimits(), observers, syncer::EXTENSION_SETTINGS, diff --git a/chrome/browser/extensions/api/storage/sync_value_store_cache.h b/chrome/browser/extensions/api/storage/sync_value_store_cache.h index d44a6fd..6de8c1d 100644 --- a/chrome/browser/extensions/api/storage/sync_value_store_cache.h +++ b/chrome/browser/extensions/api/storage/sync_value_store_cache.h @@ -23,17 +23,16 @@ class SyncableService; namespace extensions { -class SettingsStorageFactory; class SyncStorageBackend; +class ValueStoreFactory; // ValueStoreCache for the SYNC namespace. It owns a backend for apps and // another for extensions. Each backend takes care of persistence and syncing. class SyncValueStoreCache : public ValueStoreCache { public: - SyncValueStoreCache( - const scoped_refptr<SettingsStorageFactory>& factory, - const scoped_refptr<SettingsObserverList>& observers, - const base::FilePath& profile_path); + SyncValueStoreCache(const scoped_refptr<ValueStoreFactory>& factory, + const scoped_refptr<SettingsObserverList>& observers, + const base::FilePath& profile_path); ~SyncValueStoreCache() override; syncer::SyncableService* GetSyncableService(syncer::ModelType type) const; @@ -45,7 +44,7 @@ class SyncValueStoreCache : public ValueStoreCache { void DeleteStorageSoon(const std::string& extension_id) override; private: - void InitOnFileThread(const scoped_refptr<SettingsStorageFactory>& factory, + void InitOnFileThread(const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<SettingsObserverList>& observers, const base::FilePath& profile_path); diff --git a/chrome/browser/extensions/extension_system_impl.cc b/chrome/browser/extensions/extension_system_impl.cc index af180ba..8e24a2b 100644 --- a/chrome/browser/extensions/extension_system_impl.cc +++ b/chrome/browser/extensions/extension_system_impl.cc @@ -50,6 +50,7 @@ #include "extensions/browser/service_worker_manager.h" #include "extensions/browser/state_store.h" #include "extensions/browser/uninstall_ping_sender.h" +#include "extensions/browser/value_store/value_store_factory_impl.h" #include "extensions/common/constants.h" #include "extensions/common/manifest_url_handlers.h" @@ -71,13 +72,6 @@ using content::BrowserThread; -// Statistics are logged to UMA with this string as part of histogram name. They -// can all be found under Extensions.Database.Open.<client>. Changing this needs -// to synchronize with histograms.xml, AND will also become incompatible with -// older browsers still reporting the previous values. -const char kStateDatabaseUMAClientName[] = "State"; -const char kRulesDatabaseUMAClientName[] = "Rules"; - namespace extensions { namespace { @@ -107,18 +101,18 @@ ExtensionSystemImpl::Shared::~Shared() { } void ExtensionSystemImpl::Shared::InitPrefs() { + store_factory_ = new ValueStoreFactoryImpl(profile_->GetPath()); + // Two state stores. The latter, which contains declarative rules, must be // loaded immediately so that the rules are ready before we issue network // requests. state_store_.reset(new StateStore( - profile_, kStateDatabaseUMAClientName, - profile_->GetPath().AppendASCII(extensions::kStateStoreName), true)); + profile_, store_factory_, ValueStoreFrontend::BackendType::STATE, true)); state_store_notification_observer_.reset( new StateStoreNotificationObserver(state_store_.get())); rules_store_.reset(new StateStore( - profile_, kRulesDatabaseUMAClientName, - profile_->GetPath().AppendASCII(extensions::kRulesStoreName), false)); + profile_, store_factory_, ValueStoreFrontend::BackendType::RULES, false)); #if defined(OS_CHROMEOS) const user_manager::User* user = @@ -281,6 +275,11 @@ StateStore* ExtensionSystemImpl::Shared::rules_store() { return rules_store_.get(); } +scoped_refptr<ValueStoreFactory> ExtensionSystemImpl::Shared::store_factory() + const { + return store_factory_; +} + ExtensionService* ExtensionSystemImpl::Shared::extension_service() { return extension_service_.get(); } @@ -374,6 +373,10 @@ StateStore* ExtensionSystemImpl::rules_store() { return shared_->rules_store(); } +scoped_refptr<ValueStoreFactory> ExtensionSystemImpl::store_factory() { + return shared_->store_factory(); +} + InfoMap* ExtensionSystemImpl::info_map() { return shared_->info_map(); } const OneShotEvent& ExtensionSystemImpl::ready() const { diff --git a/chrome/browser/extensions/extension_system_impl.h b/chrome/browser/extensions/extension_system_impl.h index bff34f3..cb50c79 100644 --- a/chrome/browser/extensions/extension_system_impl.h +++ b/chrome/browser/extensions/extension_system_impl.h @@ -13,6 +13,7 @@ #include "extensions/common/one_shot_event.h" class Profile; +class ValueStore; namespace extensions { @@ -20,6 +21,8 @@ class ExtensionSystemSharedFactory; class NavigationObserver; class StateStoreNotificationObserver; class UninstallPingSender; +class ValueStoreFactory; +class ValueStoreFactoryImpl; // The ExtensionSystem for ProfileImpl and OffTheRecordProfileImpl. // Implementation details: non-shared services are owned by @@ -43,6 +46,7 @@ class ExtensionSystemImpl : public ExtensionSystem { SharedUserScriptMaster* shared_user_script_master() override; // shared StateStore* state_store() override; // shared StateStore* rules_store() override; // shared + scoped_refptr<ValueStoreFactory> store_factory() override; // shared InfoMap* info_map() override; // shared QuotaService* quota_service() override; // shared AppSorting* app_sorting() override; // shared @@ -83,6 +87,7 @@ class ExtensionSystemImpl : public ExtensionSystem { StateStore* state_store(); StateStore* rules_store(); + scoped_refptr<ValueStoreFactory> store_factory() const; ExtensionService* extension_service(); RuntimeData* runtime_data(); ManagementPolicy* management_policy(); @@ -103,6 +108,7 @@ class ExtensionSystemImpl : public ExtensionSystem { scoped_ptr<StateStoreNotificationObserver> state_store_notification_observer_; scoped_ptr<StateStore> rules_store_; + scoped_refptr<ValueStoreFactoryImpl> store_factory_; scoped_ptr<NavigationObserver> navigation_observer_; scoped_ptr<ServiceWorkerManager> service_worker_manager_; // Shared memory region manager for scripts statically declared in extension diff --git a/chrome/browser/extensions/test_extension_system.cc b/chrome/browser/extensions/test_extension_system.cc index 19f17ce..f116596 100644 --- a/chrome/browser/extensions/test_extension_system.cc +++ b/chrome/browser/extensions/test_extension_system.cc @@ -25,6 +25,7 @@ #include "extensions/browser/quota_service.h" #include "extensions/browser/runtime_data.h" #include "extensions/browser/state_store.h" +#include "extensions/browser/value_store/test_value_store_factory.h" #include "extensions/browser/value_store/testing_value_store.h" using content::BrowserThread; @@ -33,11 +34,10 @@ namespace extensions { TestExtensionSystem::TestExtensionSystem(Profile* profile) : profile_(profile), - value_store_(NULL), + store_factory_(new TestValueStoreFactory()), info_map_(new InfoMap()), quota_service_(new QuotaService()), - app_sorting_(new ChromeAppSorting(profile_)) { -} + app_sorting_(new ChromeAppSorting(profile_)) {} TestExtensionSystem::~TestExtensionSystem() { } @@ -51,11 +51,8 @@ ExtensionService* TestExtensionSystem::CreateExtensionService( const base::CommandLine* command_line, const base::FilePath& install_directory, bool autoupdate_enabled) { - // The ownership of |value_store_| is immediately transferred to state_store_, - // but we keep a naked pointer to the TestingValueStore. - scoped_ptr<TestingValueStore> value_store(new TestingValueStore()); - value_store_ = value_store.get(); - state_store_.reset(new StateStore(profile_, std::move(value_store))); + state_store_.reset(new StateStore( + profile_, store_factory_, ValueStoreFrontend::BackendType::RULES, false)); management_policy_.reset(new ManagementPolicy()); management_policy_->RegisterProviders( ExtensionManagementFactory::GetForBrowserContext(profile_) @@ -105,6 +102,10 @@ StateStore* TestExtensionSystem::rules_store() { return state_store_.get(); } +scoped_refptr<ValueStoreFactory> TestExtensionSystem::store_factory() { + return store_factory_; +} + InfoMap* TestExtensionSystem::info_map() { return info_map_.get(); } QuotaService* TestExtensionSystem::quota_service() { @@ -135,7 +136,9 @@ void TestExtensionSystem::InstallUpdate(const std::string& extension_id, } TestingValueStore* TestExtensionSystem::value_store() { - return value_store_; + // These tests use TestingValueStore in a way that ensures it only ever mints + // instances of TestingValueStore. + return static_cast<TestingValueStore*>(store_factory_->LastCreatedStore()); } // static diff --git a/chrome/browser/extensions/test_extension_system.h b/chrome/browser/extensions/test_extension_system.h index b169095..1306507 100644 --- a/chrome/browser/extensions/test_extension_system.h +++ b/chrome/browser/extensions/test_extension_system.h @@ -23,6 +23,8 @@ class BrowserContext; namespace extensions { +class TestValueStoreFactory; + // Test ExtensionSystem, for use with TestingProfile. class TestExtensionSystem : public ExtensionSystem { public: @@ -50,6 +52,7 @@ class TestExtensionSystem : public ExtensionSystem { SharedUserScriptMaster* shared_user_script_master() override; StateStore* state_store() override; StateStore* rules_store() override; + scoped_refptr<ValueStoreFactory> store_factory() override; TestingValueStore* value_store(); InfoMap* info_map() override; QuotaService* quota_service() override; @@ -78,8 +81,7 @@ class TestExtensionSystem : public ExtensionSystem { private: scoped_ptr<StateStore> state_store_; - // A pointer to the TestingValueStore owned by |state_store_|. - TestingValueStore* value_store_; + scoped_refptr<TestValueStoreFactory> store_factory_; scoped_ptr<ManagementPolicy> management_policy_; scoped_ptr<RuntimeData> runtime_data_; scoped_ptr<ExtensionService> extension_service_; diff --git a/extensions/browser/api/extensions_api_client.cc b/extensions/browser/api/extensions_api_client.cc index 7eb174f..bfc08d3 100644 --- a/extensions/browser/api/extensions_api_client.cc +++ b/extensions/browser/api/extensions_api_client.cc @@ -28,7 +28,7 @@ ExtensionsAPIClient* ExtensionsAPIClient::Get() { return g_instance; } void ExtensionsAPIClient::AddAdditionalValueStoreCaches( content::BrowserContext* context, - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<base::ObserverListThreadSafe<SettingsObserver>>& observers, std::map<settings_namespace::Namespace, ValueStoreCache*>* caches) {} diff --git a/extensions/browser/api/extensions_api_client.h b/extensions/browser/api/extensions_api_client.h index ca50ba8..d68174d 100644 --- a/extensions/browser/api/extensions_api_client.h +++ b/extensions/browser/api/extensions_api_client.h @@ -40,8 +40,8 @@ class MimeHandlerViewGuest; class MimeHandlerViewGuestDelegate; class RulesCacheDelegate; class SettingsObserver; -class SettingsStorageFactory; class ValueStoreCache; +class ValueStoreFactory; class VirtualKeyboardDelegate; class WebRequestEventRouterDelegate; class WebViewGuest; @@ -69,7 +69,7 @@ class ExtensionsAPIClient { // to |caches|. By default adds nothing. virtual void AddAdditionalValueStoreCaches( content::BrowserContext* context, - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, const scoped_refptr<base::ObserverListThreadSafe<SettingsObserver>>& observers, std::map<settings_namespace::Namespace, ValueStoreCache*>* caches); diff --git a/extensions/browser/api/storage/leveldb_settings_storage_factory.cc b/extensions/browser/api/storage/leveldb_settings_storage_factory.cc deleted file mode 100644 index ef7d302..0000000 --- a/extensions/browser/api/storage/leveldb_settings_storage_factory.cc +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "extensions/browser/api/storage/leveldb_settings_storage_factory.h" - -#include "base/files/file_util.h" -#include "base/logging.h" -#include "extensions/browser/value_store/leveldb_value_store.h" - -namespace extensions { - -namespace { - -// Statistics are logged to UMA with this string as part of histogram name. They -// can all be found under Extensions.Database.Open.<client>. Changing this needs -// to synchronize with histograms.xml, AND will also become incompatible with -// older browsers still reporting the previous values. -const char kDatabaseUMAClientName[] = "Settings"; - -base::FilePath GetDatabasePath(const base::FilePath& base_path, - const std::string& extension_id) { - return base_path.AppendASCII(extension_id); -} - -} // namespace - -ValueStore* LeveldbSettingsStorageFactory::Create( - const base::FilePath& base_path, - const std::string& extension_id) { - return new LeveldbValueStore(kDatabaseUMAClientName, - GetDatabasePath(base_path, extension_id)); -} - -void LeveldbSettingsStorageFactory::DeleteDatabaseIfExists( - const base::FilePath& base_path, - const std::string& extension_id) { - base::FilePath path = GetDatabasePath(base_path, extension_id); - if (base::PathExists(path)) - base::DeleteFile(path, true /* recursive */); -} - -} // namespace extensions diff --git a/extensions/browser/api/storage/leveldb_settings_storage_factory.h b/extensions/browser/api/storage/leveldb_settings_storage_factory.h deleted file mode 100644 index 6f9cc56..0000000 --- a/extensions/browser/api/storage/leveldb_settings_storage_factory.h +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef EXTENSIONS_BROWSER_API_STORAGE_LEVELDB_SETTINGS_STORAGE_FACTORY_H_ -#define EXTENSIONS_BROWSER_API_STORAGE_LEVELDB_SETTINGS_STORAGE_FACTORY_H_ - -#include "extensions/browser/api/storage/settings_storage_factory.h" - -namespace extensions { - -// Factory for creating LeveldbValueStore instances. -class LeveldbSettingsStorageFactory : public SettingsStorageFactory { - public: - ValueStore* Create(const base::FilePath& base_path, - const std::string& extension_id) override; - - void DeleteDatabaseIfExists(const base::FilePath& base_path, - const std::string& extension_id) override; - - private: - // SettingsStorageFactory is refcounted. - ~LeveldbSettingsStorageFactory() override {} -}; - -} // namespace extensions - -#endif // EXTENSIONS_BROWSER_API_STORAGE_LEVELDB_SETTINGS_STORAGE_FACTORY_H_ diff --git a/extensions/browser/api/storage/local_value_store_cache.cc b/extensions/browser/api/storage/local_value_store_cache.cc index 9c9ab15..920d130 100644 --- a/extensions/browser/api/storage/local_value_store_cache.cc +++ b/extensions/browser/api/storage/local_value_store_cache.cc @@ -8,18 +8,10 @@ #include <limits> -#include "base/bind.h" -#include "base/callback.h" -#include "base/files/file_path.h" #include "content/public/browser/browser_thread.h" -#include "extensions/browser/api/storage/settings_storage_factory.h" -#include "extensions/browser/api/storage/settings_storage_quota_enforcer.h" #include "extensions/browser/api/storage/weak_unlimited_settings_storage.h" -#include "extensions/browser/value_store/value_store.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "extensions/common/api/storage.h" -#include "extensions/common/constants.h" -#include "extensions/common/extension.h" -#include "extensions/common/permissions/api_permission.h" #include "extensions/common/permissions/permissions_data.h" using content::BrowserThread; @@ -40,13 +32,8 @@ SettingsStorageQuotaEnforcer::Limits GetLocalQuotaLimits() { } // namespace LocalValueStoreCache::LocalValueStoreCache( - const scoped_refptr<SettingsStorageFactory>& factory, - const base::FilePath& profile_path) - : storage_factory_(factory), - extension_base_path_( - profile_path.AppendASCII(kLocalExtensionSettingsDirectoryName)), - app_base_path_(profile_path.AppendASCII(kLocalAppSettingsDirectoryName)), - quota_(GetLocalQuotaLimits()) { + const scoped_refptr<ValueStoreFactory>& factory) + : storage_factory_(factory), quota_(GetLocalQuotaLimits()) { DCHECK_CURRENTLY_ON(BrowserThread::UI); } @@ -59,7 +46,7 @@ void LocalValueStoreCache::RunWithValueStoreForExtension( scoped_refptr<const Extension> extension) { DCHECK_CURRENTLY_ON(BrowserThread::FILE); - ValueStore* storage = GetStorage(extension); + ValueStore* storage = GetStorage(extension.get()); // A neat way to implement unlimited storage; if the extension has the // unlimited storage permission, force through all calls to Set(). @@ -75,21 +62,26 @@ void LocalValueStoreCache::RunWithValueStoreForExtension( void LocalValueStoreCache::DeleteStorageSoon(const std::string& extension_id) { DCHECK_CURRENTLY_ON(BrowserThread::FILE); storage_map_.erase(extension_id); - storage_factory_->DeleteDatabaseIfExists(app_base_path_, extension_id); - storage_factory_->DeleteDatabaseIfExists(extension_base_path_, extension_id); + storage_factory_->DeleteSettings(settings_namespace::LOCAL, + ValueStoreFactory::ModelType::APP, + extension_id); + storage_factory_->DeleteSettings(settings_namespace::LOCAL, + ValueStoreFactory::ModelType::EXTENSION, + extension_id); } -ValueStore* LocalValueStoreCache::GetStorage( - scoped_refptr<const Extension> extension) { +ValueStore* LocalValueStoreCache::GetStorage(const Extension* extension) { StorageMap::iterator iter = storage_map_.find(extension->id()); if (iter != storage_map_.end()) return iter->second.get(); - const base::FilePath& file_path = - extension->is_app() ? app_base_path_ : extension_base_path_; + ValueStoreFactory::ModelType model_type = + extension->is_app() ? ValueStoreFactory::ModelType::APP + : ValueStoreFactory::ModelType::EXTENSION; + scoped_ptr<ValueStore> store = storage_factory_->CreateSettingsStore( + settings_namespace::LOCAL, model_type, extension->id()); linked_ptr<SettingsStorageQuotaEnforcer> storage( - new SettingsStorageQuotaEnforcer( - quota_, storage_factory_->Create(file_path, extension->id()))); + new SettingsStorageQuotaEnforcer(quota_, std::move(store))); DCHECK(storage.get()); storage_map_[extension->id()] = storage; diff --git a/extensions/browser/api/storage/local_value_store_cache.h b/extensions/browser/api/storage/local_value_store_cache.h index 6385ff1..c3303a5 100644 --- a/extensions/browser/api/storage/local_value_store_cache.h +++ b/extensions/browser/api/storage/local_value_store_cache.h @@ -6,7 +6,6 @@ #define EXTENSIONS_BROWSER_API_STORAGE_LOCAL_VALUE_STORE_CACHE_H_ #include "base/compiler_specific.h" -#include "base/files/file_path.h" #include "base/macros.h" #include "base/memory/linked_ptr.h" #include "base/memory/ref_counted.h" @@ -16,14 +15,14 @@ namespace extensions { -class SettingsStorageFactory; +class ValueStoreFactory; // ValueStoreCache for the LOCAL namespace. It owns a backend for apps and // another for extensions. Each backend takes care of persistence. class LocalValueStoreCache : public ValueStoreCache { public: - LocalValueStoreCache(const scoped_refptr<SettingsStorageFactory>& factory, - const base::FilePath& profile_path); + explicit LocalValueStoreCache( + const scoped_refptr<ValueStoreFactory>& factory); ~LocalValueStoreCache() override; // ValueStoreCache implementation: @@ -35,16 +34,10 @@ class LocalValueStoreCache : public ValueStoreCache { private: typedef std::map<std::string, linked_ptr<ValueStore> > StorageMap; - ValueStore* GetStorage(scoped_refptr<const Extension> extension); + ValueStore* GetStorage(const Extension* extension); // The Factory to use for creating new ValueStores. - const scoped_refptr<SettingsStorageFactory> storage_factory_; - - // The base path to use for extensions when creating new ValueStores. - const base::FilePath extension_base_path_; - - // The base path to use for apps when creating new ValueStores. - const base::FilePath app_base_path_; + const scoped_refptr<ValueStoreFactory> storage_factory_; // Quota limits (see SettingsStorageQuotaEnforcer). const SettingsStorageQuotaEnforcer::Limits quota_; diff --git a/extensions/browser/api/storage/settings_quota_unittest.cc b/extensions/browser/api/storage/settings_quota_unittest.cc index c89669c..3681401 100644 --- a/extensions/browser/api/storage/settings_quota_unittest.cc +++ b/extensions/browser/api/storage/settings_quota_unittest.cc @@ -54,7 +54,8 @@ class ExtensionSettingsQuotaTest : public testing::Test { ASSERT_TRUE(storage_.get() == NULL); SettingsStorageQuotaEnforcer::Limits limits = { quota_bytes, quota_bytes_per_item, max_items }; - storage_.reset(new SettingsStorageQuotaEnforcer(limits, delegate_)); + storage_.reset( + new SettingsStorageQuotaEnforcer(limits, make_scoped_ptr(delegate_))); } // Returns whether the settings in |storage_| and |delegate_| are the same as diff --git a/extensions/browser/api/storage/settings_storage_factory.h b/extensions/browser/api/storage/settings_storage_factory.h deleted file mode 100644 index 14f6187..0000000 --- a/extensions/browser/api/storage/settings_storage_factory.h +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef EXTENSIONS_BROWSER_API_STORAGE_SETTINGS_STORAGE_FACTORY_H_ -#define EXTENSIONS_BROWSER_API_STORAGE_SETTINGS_STORAGE_FACTORY_H_ - -#include <string> - -#include "base/files/file_path.h" -#include "base/memory/ref_counted.h" - -class ValueStore; - -namespace extensions { - -// Factory for creating SettingStorage instances. -// -// Refcouted because it's just too messy to distribute these objects between -// ValueStoreCache instances any other way. -class SettingsStorageFactory - : public base::RefCountedThreadSafe<SettingsStorageFactory> { - public: - // Creates a new ValueStore area for an extension under |base_path|. - // Return NULL to indicate failure. Must be called on the FILE thread. - virtual ValueStore* Create(const base::FilePath& base_path, - const std::string& extension_id) = 0; - - // Deletes the database for the extension, if one exists. - // Note: it is important to delete references to the database if any are - // held, because ValueStores will create themselves if there is no file. - virtual void DeleteDatabaseIfExists(const base::FilePath& base_path, - const std::string& extension_id) = 0; - - protected: - friend class base::RefCountedThreadSafe<SettingsStorageFactory>; - virtual ~SettingsStorageFactory() {} -}; - -} // namespace extensions - -#endif // EXTENSIONS_BROWSER_API_STORAGE_SETTINGS_STORAGE_FACTORY_H_ diff --git a/extensions/browser/api/storage/settings_storage_quota_enforcer.cc b/extensions/browser/api/storage/settings_storage_quota_enforcer.cc index 9a9e9b7..7d3703c 100644 --- a/extensions/browser/api/storage/settings_storage_quota_enforcer.cc +++ b/extensions/browser/api/storage/settings_storage_quota_enforcer.cc @@ -70,10 +70,11 @@ ValueStore::Status QuotaExceededError(Resource resource) { } // namespace -SettingsStorageQuotaEnforcer::SettingsStorageQuotaEnforcer(const Limits& limits, - ValueStore* delegate) +SettingsStorageQuotaEnforcer::SettingsStorageQuotaEnforcer( + const Limits& limits, + scoped_ptr<ValueStore> delegate) : limits_(limits), - delegate_(delegate), + delegate_(std::move(delegate)), used_total_(0), usage_calculated_(false) {} diff --git a/extensions/browser/api/storage/settings_storage_quota_enforcer.h b/extensions/browser/api/storage/settings_storage_quota_enforcer.h index 48a5e89..33de5b7 100644 --- a/extensions/browser/api/storage/settings_storage_quota_enforcer.h +++ b/extensions/browser/api/storage/settings_storage_quota_enforcer.h @@ -13,6 +13,7 @@ #include "base/compiler_specific.h" #include "base/macros.h" +#include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "extensions/browser/value_store/value_store.h" @@ -33,7 +34,8 @@ class SettingsStorageQuotaEnforcer : public ValueStore { size_t max_items; }; - SettingsStorageQuotaEnforcer(const Limits& limits, ValueStore* delegate); + SettingsStorageQuotaEnforcer(const Limits& limits, + scoped_ptr<ValueStore> delegate); ~SettingsStorageQuotaEnforcer() override; diff --git a/extensions/browser/api/storage/settings_test_util.cc b/extensions/browser/api/storage/settings_test_util.cc index c3dc4a5..4442541 100644 --- a/extensions/browser/api/storage/settings_test_util.cc +++ b/extensions/browser/api/storage/settings_test_util.cc @@ -118,34 +118,6 @@ scoped_refptr<const Extension> AddExtensionWithIdAndPermissions( return extension; } -// ScopedSettingsFactory - -ScopedSettingsStorageFactory::ScopedSettingsStorageFactory() {} - -ScopedSettingsStorageFactory::ScopedSettingsStorageFactory( - const scoped_refptr<SettingsStorageFactory>& delegate) - : delegate_(delegate) {} - -ScopedSettingsStorageFactory::~ScopedSettingsStorageFactory() {} - -void ScopedSettingsStorageFactory::Reset( - const scoped_refptr<SettingsStorageFactory>& delegate) { - delegate_ = delegate; -} - -ValueStore* ScopedSettingsStorageFactory::Create( - const base::FilePath& base_path, - const std::string& extension_id) { - DCHECK(delegate_.get()); - return delegate_->Create(base_path, extension_id); -} - -void ScopedSettingsStorageFactory::DeleteDatabaseIfExists( - const base::FilePath& base_path, - const std::string& extension_id) { - delegate_->DeleteDatabaseIfExists(base_path, extension_id); -} - } // namespace settings_test_util } // namespace extensions diff --git a/extensions/browser/api/storage/settings_test_util.h b/extensions/browser/api/storage/settings_test_util.h index 0b23f60..07a1d52 100644 --- a/extensions/browser/api/storage/settings_test_util.h +++ b/extensions/browser/api/storage/settings_test_util.h @@ -14,9 +14,9 @@ #include "base/memory/scoped_ptr.h" #include "chrome/test/base/testing_profile.h" #include "extensions/browser/api/storage/settings_namespace.h" -#include "extensions/browser/api/storage/settings_storage_factory.h" #include "extensions/browser/event_router.h" #include "extensions/browser/mock_extension_system.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "extensions/common/extension.h" class ValueStore; @@ -56,30 +56,6 @@ scoped_refptr<const Extension> AddExtensionWithIdAndPermissions( Manifest::Type type, const std::set<std::string>& permissions); -// SettingsStorageFactory which acts as a wrapper for other factories. -class ScopedSettingsStorageFactory : public SettingsStorageFactory { - public: - ScopedSettingsStorageFactory(); - - explicit ScopedSettingsStorageFactory( - const scoped_refptr<SettingsStorageFactory>& delegate); - - // Sets the delegate factory (equivalent to scoped_ptr::reset). - void Reset(const scoped_refptr<SettingsStorageFactory>& delegate); - - // SettingsStorageFactory implementation. - ValueStore* Create(const base::FilePath& base_path, - const std::string& extension_id) override; - void DeleteDatabaseIfExists(const base::FilePath& base_path, - const std::string& extension_id) override; - - private: - // SettingsStorageFactory is refcounted. - ~ScopedSettingsStorageFactory() override; - - scoped_refptr<SettingsStorageFactory> delegate_; -}; - } // namespace settings_test_util } // namespace extensions diff --git a/extensions/browser/api/storage/storage_api_unittest.cc b/extensions/browser/api/storage/storage_api_unittest.cc index f987db2..4a013db 100644 --- a/extensions/browser/api/storage/storage_api_unittest.cc +++ b/extensions/browser/api/storage/storage_api_unittest.cc @@ -9,7 +9,6 @@ #include "base/strings/stringprintf.h" #include "content/public/test/test_browser_context.h" #include "extensions/browser/api/extensions_api_client.h" -#include "extensions/browser/api/storage/leveldb_settings_storage_factory.h" #include "extensions/browser/api/storage/settings_storage_quota_enforcer.h" #include "extensions/browser/api/storage/settings_test_util.h" #include "extensions/browser/api/storage/storage_api.h" @@ -20,6 +19,7 @@ #include "extensions/browser/test_extensions_browser_client.h" #include "extensions/browser/value_store/leveldb_value_store.h" #include "extensions/browser/value_store/value_store.h" +#include "extensions/browser/value_store/value_store_factory_impl.h" #include "extensions/common/manifest.h" #include "extensions/common/test_util.h" #include "third_party/leveldatabase/src/include/leveldb/db.h" @@ -32,8 +32,9 @@ namespace { // Caller owns the returned object. scoped_ptr<KeyedService> CreateStorageFrontendForTesting( content::BrowserContext* context) { - return StorageFrontend::CreateForTesting(new LeveldbSettingsStorageFactory(), - context); + scoped_refptr<ValueStoreFactory> factory = + new ValueStoreFactoryImpl(context->GetPath()); + return StorageFrontend::CreateForTesting(factory, context); } scoped_ptr<KeyedService> BuildEventRouter(content::BrowserContext* context) { @@ -105,6 +106,8 @@ TEST_F(StorageApiUnittest, RestoreCorruptedStorage) { settings_namespace::LOCAL, StorageFrontend::Get(browser_context())); ASSERT_TRUE(store); + // TODO(cmumford): Modify test as this requires that the factory always + // creates instances of LeveldbValueStore. SettingsStorageQuotaEnforcer* quota_store = static_cast<SettingsStorageQuotaEnforcer*>(store); LeveldbValueStore* leveldb_store = diff --git a/extensions/browser/api/storage/storage_frontend.cc b/extensions/browser/api/storage/storage_frontend.cc index 26d6ec8..911a492 100644 --- a/extensions/browser/api/storage/storage_frontend.cc +++ b/extensions/browser/api/storage/storage_frontend.cc @@ -16,10 +16,11 @@ #include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" #include "extensions/browser/api/extensions_api_client.h" -#include "extensions/browser/api/storage/leveldb_settings_storage_factory.h" #include "extensions/browser/api/storage/local_value_store_cache.h" #include "extensions/browser/event_router.h" #include "extensions/browser/extension_registry.h" +#include "extensions/browser/extension_system.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "extensions/common/api/storage.h" using content::BrowserContext; @@ -69,25 +70,23 @@ StorageFrontend* StorageFrontend::Get(BrowserContext* context) { // static scoped_ptr<StorageFrontend> StorageFrontend::CreateForTesting( - const scoped_refptr<SettingsStorageFactory>& storage_factory, + const scoped_refptr<ValueStoreFactory>& storage_factory, BrowserContext* context) { return make_scoped_ptr(new StorageFrontend(storage_factory, context)); } StorageFrontend::StorageFrontend(BrowserContext* context) - : browser_context_(context) { - Init(new LeveldbSettingsStorageFactory()); + : StorageFrontend(ExtensionSystem::Get(context)->store_factory(), context) { } StorageFrontend::StorageFrontend( - const scoped_refptr<SettingsStorageFactory>& factory, + const scoped_refptr<ValueStoreFactory>& factory, BrowserContext* context) : browser_context_(context) { Init(factory); } -void StorageFrontend::Init( - const scoped_refptr<SettingsStorageFactory>& factory) { +void StorageFrontend::Init(const scoped_refptr<ValueStoreFactory>& factory) { TRACE_EVENT0("browser,startup", "StorageFrontend::Init") SCOPED_UMA_HISTOGRAM_TIMER("Extensions.StorageFrontendInitTime"); @@ -98,8 +97,7 @@ void StorageFrontend::Init( observers_->AddObserver(browser_context_observer_.get()); - caches_[settings_namespace::LOCAL] = - new LocalValueStoreCache(factory, browser_context_->GetPath()); + caches_[settings_namespace::LOCAL] = new LocalValueStoreCache(factory); // Add any additional caches the embedder supports (for example, caches // for chrome.storage.managed and chrome.storage.sync). diff --git a/extensions/browser/api/storage/storage_frontend.h b/extensions/browser/api/storage/storage_frontend.h index 372fd78..6d3427f 100644 --- a/extensions/browser/api/storage/storage_frontend.h +++ b/extensions/browser/api/storage/storage_frontend.h @@ -13,7 +13,6 @@ #include "base/memory/scoped_ptr.h" #include "extensions/browser/api/storage/settings_namespace.h" #include "extensions/browser/api/storage/settings_observer.h" -#include "extensions/browser/api/storage/settings_storage_factory.h" #include "extensions/browser/api/storage/value_store_cache.h" #include "extensions/browser/browser_context_keyed_api_factory.h" @@ -23,6 +22,8 @@ class BrowserContext; namespace extensions { +class ValueStoreFactory; + // The component of the Storage API which runs on the UI thread. class StorageFrontend : public BrowserContextKeyedAPI { public: @@ -31,7 +32,7 @@ class StorageFrontend : public BrowserContextKeyedAPI { // Creates with a specific |storage_factory|. static scoped_ptr<StorageFrontend> CreateForTesting( - const scoped_refptr<SettingsStorageFactory>& storage_factory, + const scoped_refptr<ValueStoreFactory>& storage_factory, content::BrowserContext* context); // Public so tests can create and delete their own instances. @@ -74,10 +75,10 @@ class StorageFrontend : public BrowserContextKeyedAPI { explicit StorageFrontend(content::BrowserContext* context); // Constructor for tests. - StorageFrontend(const scoped_refptr<SettingsStorageFactory>& storage_factory, + StorageFrontend(const scoped_refptr<ValueStoreFactory>& storage_factory, content::BrowserContext* context); - void Init(const scoped_refptr<SettingsStorageFactory>& storage_factory); + void Init(const scoped_refptr<ValueStoreFactory>& storage_factory); // The (non-incognito) browser context this Frontend belongs to. content::BrowserContext* const browser_context_; diff --git a/extensions/browser/api/storage/storage_frontend_unittest.cc b/extensions/browser/api/storage/storage_frontend_unittest.cc index a434a3c..5995ceb 100644 --- a/extensions/browser/api/storage/storage_frontend_unittest.cc +++ b/extensions/browser/api/storage/storage_frontend_unittest.cc @@ -12,12 +12,12 @@ #include "content/public/test/test_browser_context.h" #include "content/public/test/test_browser_thread.h" #include "extensions/browser/api/extensions_api_client.h" -#include "extensions/browser/api/storage/leveldb_settings_storage_factory.h" #include "extensions/browser/api/storage/settings_namespace.h" #include "extensions/browser/api/storage/settings_test_util.h" #include "extensions/browser/api/storage/storage_frontend.h" #include "extensions/browser/extensions_test.h" #include "extensions/browser/value_store/value_store.h" +#include "extensions/browser/value_store/value_store_factory_impl.h" #include "testing/gtest/include/gtest/gtest.h" using content::BrowserThread; @@ -40,13 +40,13 @@ const ValueStore::WriteOptions DEFAULTS = ValueStore::DEFAULTS; class ExtensionSettingsFrontendTest : public ExtensionsTest { public: ExtensionSettingsFrontendTest() - : storage_factory_(new util::ScopedSettingsStorageFactory()), - ui_thread_(BrowserThread::UI, base::MessageLoop::current()), + : ui_thread_(BrowserThread::UI, base::MessageLoop::current()), file_thread_(BrowserThread::FILE, base::MessageLoop::current()) {} void SetUp() override { ExtensionsTest::SetUp(); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + storage_factory_ = new ValueStoreFactoryImpl(temp_dir_.path()); ResetFrontend(); } @@ -59,14 +59,13 @@ class ExtensionSettingsFrontendTest : public ExtensionsTest { protected: void ResetFrontend() { - storage_factory_->Reset(new LeveldbSettingsStorageFactory()); frontend_ = StorageFrontend::CreateForTesting(storage_factory_, browser_context()); } base::ScopedTempDir temp_dir_; scoped_ptr<StorageFrontend> frontend_; - scoped_refptr<util::ScopedSettingsStorageFactory> storage_factory_; + scoped_refptr<ValueStoreFactoryImpl> storage_factory_; private: base::MessageLoop message_loop_; diff --git a/extensions/browser/extension_system.h b/extensions/browser/extension_system.h index bbd8ae5..5382d3f 100644 --- a/extensions/browser/extension_system.h +++ b/extensions/browser/extension_system.h @@ -43,6 +43,7 @@ class RuntimeData; class ServiceWorkerManager; class SharedUserScriptMaster; class StateStore; +class ValueStoreFactory; // ExtensionSystem manages the lifetime of many of the services used by the // extensions and apps system, and it handles startup and shutdown as needed. @@ -85,6 +86,9 @@ class ExtensionSystem : public KeyedService { // The rules store is created at startup. virtual StateStore* rules_store() = 0; + // Returns the |ValueStore| factory created at startup. + virtual scoped_refptr<ValueStoreFactory> store_factory() = 0; + // Returns the IO-thread-accessible extension data. virtual InfoMap* info_map() = 0; diff --git a/extensions/browser/mock_extension_system.cc b/extensions/browser/mock_extension_system.cc index efcc546..6658832 100644 --- a/extensions/browser/mock_extension_system.cc +++ b/extensions/browser/mock_extension_system.cc @@ -4,6 +4,7 @@ #include "extensions/browser/mock_extension_system.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "extensions/common/extension_set.h" namespace extensions { @@ -46,6 +47,10 @@ StateStore* MockExtensionSystem::rules_store() { return nullptr; } +scoped_refptr<ValueStoreFactory> MockExtensionSystem::store_factory() { + return nullptr; +} + InfoMap* MockExtensionSystem::info_map() { return nullptr; } diff --git a/extensions/browser/mock_extension_system.h b/extensions/browser/mock_extension_system.h index 3151edb..b926baa 100644 --- a/extensions/browser/mock_extension_system.h +++ b/extensions/browser/mock_extension_system.h @@ -35,6 +35,7 @@ class MockExtensionSystem : public ExtensionSystem { SharedUserScriptMaster* shared_user_script_master() override; StateStore* state_store() override; StateStore* rules_store() override; + scoped_refptr<ValueStoreFactory> store_factory() override; InfoMap* info_map() override; QuotaService* quota_service() override; AppSorting* app_sorting() override; diff --git a/extensions/browser/state_store.cc b/extensions/browser/state_store.cc index ff4ca02..2c8c9f9 100644 --- a/extensions/browser/state_store.cc +++ b/extensions/browser/state_store.cc @@ -14,6 +14,7 @@ #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" #include "extensions/browser/extension_registry.h" +#include "extensions/browser/value_store/value_store_factory.h" #include "extensions/common/extension.h" namespace { @@ -69,11 +70,10 @@ void StateStore::DelayedTaskQueue::SetReady() { } StateStore::StateStore(content::BrowserContext* context, - const std::string& uma_client_name, - const base::FilePath& db_path, + const scoped_refptr<ValueStoreFactory>& store_factory, + ValueStoreFrontend::BackendType backend_type, bool deferred_load) - : db_path_(db_path), - uma_client_name_(uma_client_name), + : store_(new ValueStoreFrontend(store_factory, backend_type)), task_queue_(new DelayedTaskQueue()), extension_registry_observer_(this) { extension_registry_observer_.Add(ExtensionRegistry::Get(context)); @@ -90,17 +90,6 @@ StateStore::StateStore(content::BrowserContext* context, } } -StateStore::StateStore(content::BrowserContext* context, - scoped_ptr<ValueStore> value_store) - : store_(std::move(value_store)), - task_queue_(new DelayedTaskQueue()), - extension_registry_observer_(this) { - extension_registry_observer_.Add(ExtensionRegistry::Get(context)); - - // This constructor is for testing. No need to delay Init. - Init(); -} - StateStore::~StateStore() { } @@ -115,25 +104,23 @@ void StateStore::RegisterKey(const std::string& key) { void StateStore::GetExtensionValue(const std::string& extension_id, const std::string& key, ReadCallback callback) { - task_queue_->InvokeWhenReady(base::Bind(&ValueStoreFrontend::Get, - base::Unretained(&store_), - GetFullKey(extension_id, key), - callback)); + task_queue_->InvokeWhenReady( + base::Bind(&ValueStoreFrontend::Get, base::Unretained(store_.get()), + GetFullKey(extension_id, key), callback)); } void StateStore::SetExtensionValue(const std::string& extension_id, const std::string& key, scoped_ptr<base::Value> value) { - task_queue_->InvokeWhenReady(base::Bind(&ValueStoreFrontend::Set, - base::Unretained(&store_), - GetFullKey(extension_id, key), - base::Passed(&value))); + task_queue_->InvokeWhenReady( + base::Bind(&ValueStoreFrontend::Set, base::Unretained(store_.get()), + GetFullKey(extension_id, key), base::Passed(&value))); } void StateStore::RemoveExtensionValue(const std::string& extension_id, const std::string& key) { task_queue_->InvokeWhenReady(base::Bind(&ValueStoreFrontend::Remove, - base::Unretained(&store_), + base::Unretained(store_.get()), GetFullKey(extension_id, key))); } @@ -170,8 +157,9 @@ void StateStore::Init() { if (IsInitialized()) return; - if (!db_path_.empty()) - store_.Init(uma_client_name_, db_path_); + // TODO(cmumford): The store now always lazily initializes upon first access. + // A follow-on CL will remove this deferred initialization implementation + // which is now vestigial. task_queue_->SetReady(); } @@ -190,7 +178,7 @@ void StateStore::RemoveKeysForExtension(const std::string& extension_id) { key != registered_keys_.end(); ++key) { task_queue_->InvokeWhenReady(base::Bind(&ValueStoreFrontend::Remove, - base::Unretained(&store_), + base::Unretained(store_.get()), GetFullKey(extension_id, *key))); } } diff --git a/extensions/browser/state_store.h b/extensions/browser/state_store.h index 3809c1e3..5cc0037 100644 --- a/extensions/browser/state_store.h +++ b/extensions/browser/state_store.h @@ -23,6 +23,7 @@ class BrowserContext; namespace extensions { class ExtensionRegistry; +class ValueStoreFactory; // A storage area for per-extension state that needs to be persisted to disk. class StateStore : public base::SupportsWeakPtr<StateStore>, @@ -34,8 +35,8 @@ class StateStore : public base::SupportsWeakPtr<StateStore>, // If |deferred_load| is true, we won't load the database until the first // page has been loaded. StateStore(content::BrowserContext* context, - const std::string& uma_client_name, - const base::FilePath& db_path, + const scoped_refptr<ValueStoreFactory>& store_factory, + ValueStoreFrontend::BackendType backend_type, bool deferred_load); // This variant is useful for testing (using a mock ValueStore). StateStore(content::BrowserContext* context, scoped_ptr<ValueStore> store); @@ -94,14 +95,8 @@ class StateStore : public base::SupportsWeakPtr<StateStore>, bool is_update, const std::string& old_name) override; - // Path to our database, on disk. Empty during testing. - base::FilePath db_path_; - - // Database client name used for UMA logging by database backend. - const std::string uma_client_name_; - // The store that holds our key/values. - ValueStoreFrontend store_; + scoped_ptr<ValueStoreFrontend> store_; // List of all known keys. They will be cleared for each extension when it is // (un)installed. diff --git a/extensions/browser/value_store/legacy_value_store_factory.cc b/extensions/browser/value_store/legacy_value_store_factory.cc new file mode 100644 index 0000000..bafb93e --- /dev/null +++ b/extensions/browser/value_store/legacy_value_store_factory.cc @@ -0,0 +1,264 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "extensions/browser/value_store/legacy_value_store_factory.h" + +#include <string> + +#include "base/files/file_enumerator.h" +#include "base/files/file_util.h" +#include "content/public/browser/browser_thread.h" +#include "extensions/browser/value_store/leveldb_value_store.h" +#include "extensions/common/constants.h" +#include "extensions/common/extension.h" + +using base::AutoLock; +using content::BrowserThread; + +namespace { + +// Statistics are logged to UMA with these strings as part of histogram name. +// They can all be found under Extensions.Database.Open.<client>. Changing this +// needs to synchronize with histograms.xml, AND will also become incompatible +// with older browsers still reporting the previous values. +const char kSettingsDatabaseUMAClientName[] = "Settings"; +const char kRulesDatabaseUMAClientName[] = "Rules"; +const char kStateDatabaseUMAClientName[] = "State"; + +bool ValidDBExists(const base::FilePath& path) { + // TODO(cmumford): Enhance to detect if dir contains valid database. + return base::DirectoryExists(path); +} + +} // namespace + +namespace extensions { + +// +// ModelSettings +// +LegacyValueStoreFactory::ModelSettings::ModelSettings( + const base::FilePath& data_path) + : data_path_(data_path) {} + +base::FilePath LegacyValueStoreFactory::ModelSettings::GetDBPath( + const ExtensionId& extension_id) const { + return data_path_.AppendASCII(extension_id); +} + +bool LegacyValueStoreFactory::ModelSettings::DeleteData( + const ExtensionId& extension_id) { + return base::DeleteFile(GetDBPath(extension_id), true /* recursive */); +} + +bool LegacyValueStoreFactory::ModelSettings::DataExists( + const ExtensionId& extension_id) const { + return ValidDBExists(GetDBPath(extension_id)); +} + +std::set<ExtensionId> +LegacyValueStoreFactory::ModelSettings::GetKnownExtensionIDs() const { + DCHECK_CURRENTLY_ON(BrowserThread::FILE); + std::set<ExtensionId> result; + + // Leveldb databases are directories inside |base_path_|. + base::FileEnumerator extension_dirs(data_path_, false, + base::FileEnumerator::DIRECTORIES); + while (!extension_dirs.Next().empty()) { + base::FilePath extension_dir = extension_dirs.GetInfo().GetName(); + DCHECK(!extension_dir.IsAbsolute()); + // Extension ID's are 'a'..'p', so any directory within this folder will + // either be ASCII, or created by some other application and safe to ignore. + std::string maybe_as_ascii(extension_dir.MaybeAsASCII()); + if (!maybe_as_ascii.empty()) { + result.insert(maybe_as_ascii); + } + } + + return result; +} + +// +// SettingsRoot +// + +LegacyValueStoreFactory::SettingsRoot::SettingsRoot( + const base::FilePath& base_path, + const std::string& extension_dirname, + const std::string& app_dirname) { + if (!extension_dirname.empty()) + extensions_.reset( + new ModelSettings(base_path.AppendASCII(extension_dirname))); + + if (!app_dirname.empty()) + apps_.reset(new ModelSettings(base_path.AppendASCII(app_dirname))); +} + +LegacyValueStoreFactory::SettingsRoot::~SettingsRoot() = default; + +const LegacyValueStoreFactory::ModelSettings* +LegacyValueStoreFactory::SettingsRoot::GetModel(ModelType model_type) const { + switch (model_type) { + case ValueStoreFactory::ModelType::APP: + DCHECK(apps_ != nullptr); + return apps_.get(); + case ValueStoreFactory::ModelType::EXTENSION: + DCHECK(extensions_ != nullptr); + return extensions_.get(); + } + NOTREACHED(); + return nullptr; +} + +LegacyValueStoreFactory::ModelSettings* +LegacyValueStoreFactory::SettingsRoot::GetModel(ModelType model_type) { + switch (model_type) { + case ValueStoreFactory::ModelType::APP: + DCHECK(apps_ != nullptr); + return apps_.get(); + case ValueStoreFactory::ModelType::EXTENSION: + DCHECK(extensions_ != nullptr); + return extensions_.get(); + } + NOTREACHED(); + return nullptr; +} + +std::set<ExtensionId> +LegacyValueStoreFactory::SettingsRoot::GetKnownExtensionIDs( + ModelType model_type) const { + DCHECK_CURRENTLY_ON(BrowserThread::FILE); + switch (model_type) { + case ValueStoreFactory::ModelType::APP: + DCHECK(apps_ != nullptr); + return apps_->GetKnownExtensionIDs(); + case ValueStoreFactory::ModelType::EXTENSION: + DCHECK(extensions_ != nullptr); + return extensions_->GetKnownExtensionIDs(); + } + NOTREACHED(); + return std::set<ExtensionId>(); +} + +// +// LegacyValueStoreFactory +// + +LegacyValueStoreFactory::LegacyValueStoreFactory( + const base::FilePath& profile_path) + : profile_path_(profile_path), + local_settings_(profile_path, + kLocalExtensionSettingsDirectoryName, + kLocalAppSettingsDirectoryName), + sync_settings_(profile_path, + kSyncExtensionSettingsDirectoryName, + kSyncAppSettingsDirectoryName), + // Currently no such thing as a managed app - only an extension. + managed_settings_(profile_path, kManagedSettingsDirectoryName, "") {} + +LegacyValueStoreFactory::~LegacyValueStoreFactory() = default; + +bool LegacyValueStoreFactory::RulesDBExists() const { + return ValidDBExists(GetRulesDBPath()); +} + +bool LegacyValueStoreFactory::StateDBExists() const { + return ValidDBExists(GetStateDBPath()); +} + +scoped_ptr<ValueStore> LegacyValueStoreFactory::CreateRulesStore() { + return make_scoped_ptr( + new LeveldbValueStore(kRulesDatabaseUMAClientName, GetRulesDBPath())); +} + +scoped_ptr<ValueStore> LegacyValueStoreFactory::CreateStateStore() { + return make_scoped_ptr( + new LeveldbValueStore(kStateDatabaseUMAClientName, GetStateDBPath())); +} + +scoped_ptr<ValueStore> LegacyValueStoreFactory::CreateSettingsStore( + settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + const ModelSettings* settings_root = + GetSettingsRoot(settings_namespace).GetModel(model_type); + DCHECK(settings_root != nullptr); + return make_scoped_ptr(new LeveldbValueStore( + kSettingsDatabaseUMAClientName, settings_root->GetDBPath(extension_id))); +} + +void LegacyValueStoreFactory::DeleteSettings( + settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + // TODO(cmumford): Verify that we always need to be called on FILE thread. + DCHECK_CURRENTLY_ON(BrowserThread::FILE); + ModelSettings* model_settings = + GetSettingsRoot(settings_namespace).GetModel(model_type); + if (model_settings == nullptr) { + NOTREACHED(); + return; + } + model_settings->DeleteData(extension_id); +} + +bool LegacyValueStoreFactory::HasSettings( + settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + const ModelSettings* model_settings = + GetSettingsRoot(settings_namespace).GetModel(model_type); + if (model_settings == nullptr) + return false; + return model_settings->DataExists(extension_id); +} + +std::set<ExtensionId> LegacyValueStoreFactory::GetKnownExtensionIDs( + settings_namespace::Namespace settings_type, + ModelType model_type) const { + return GetSettingsRoot(settings_type).GetKnownExtensionIDs(model_type); +} + +const LegacyValueStoreFactory::SettingsRoot& +LegacyValueStoreFactory::GetSettingsRoot( + settings_namespace::Namespace settings_namespace) const { + switch (settings_namespace) { + case settings_namespace::LOCAL: + return local_settings_; + case settings_namespace::SYNC: + return sync_settings_; + case settings_namespace::MANAGED: + return managed_settings_; + case settings_namespace::INVALID: + break; + } + NOTREACHED(); + return local_settings_; +} + +LegacyValueStoreFactory::SettingsRoot& LegacyValueStoreFactory::GetSettingsRoot( + settings_namespace::Namespace settings_namespace) { + switch (settings_namespace) { + case settings_namespace::LOCAL: + return local_settings_; + case settings_namespace::SYNC: + return sync_settings_; + case settings_namespace::MANAGED: + return managed_settings_; + case settings_namespace::INVALID: + break; + } + NOTREACHED(); + return local_settings_; +} + +base::FilePath LegacyValueStoreFactory::GetRulesDBPath() const { + return profile_path_.AppendASCII(kRulesStoreName); +} + +base::FilePath LegacyValueStoreFactory::GetStateDBPath() const { + return profile_path_.AppendASCII(kStateStoreName); +} + +} // namespace extensions diff --git a/extensions/browser/value_store/legacy_value_store_factory.h b/extensions/browser/value_store/legacy_value_store_factory.h new file mode 100644 index 0000000..65db027 --- /dev/null +++ b/extensions/browser/value_store/legacy_value_store_factory.h @@ -0,0 +1,111 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef EXTENSIONS_BROWSER_VALUE_STORE_LEGACY_VALUE_STORE_FACTORY_H_ +#define EXTENSIONS_BROWSER_VALUE_STORE_LEGACY_VALUE_STORE_FACTORY_H_ + +#include <set> +#include <string> + +#include "base/files/file_path.h" +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "extensions/browser/value_store/value_store.h" +#include "extensions/browser/value_store/value_store_factory.h" +#include "extensions/common/extension.h" + +namespace extensions { + +// A factory to create legacy ValueStore instances for storing extension +// state/rules/settings. "legacy" refers to the initial storage implementation +// which created a settings database per extension. +class LegacyValueStoreFactory : public ValueStoreFactory { + public: + explicit LegacyValueStoreFactory(const base::FilePath& profile_path); + + bool RulesDBExists() const; + bool StateDBExists() const; + + // ValueStoreFactory: + scoped_ptr<ValueStore> CreateRulesStore() override; + scoped_ptr<ValueStore> CreateStateStore() override; + scoped_ptr<ValueStore> CreateSettingsStore( + settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + + void DeleteSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + bool HasSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + std::set<ExtensionId> GetKnownExtensionIDs( + settings_namespace::Namespace settings_namespace, + ModelType model_type) const override; + + private: + friend class base::RefCounted<LegacyValueStoreFactory>; + + // Manages a collection of legacy settings databases all within a common + // directory. + class ModelSettings { + public: + explicit ModelSettings(const base::FilePath& data_path); + + base::FilePath GetDBPath(const ExtensionId& extension_id) const; + bool DeleteData(const ExtensionId& extension_id); + bool DataExists(const ExtensionId& extension_id) const; + std::set<ExtensionId> GetKnownExtensionIDs() const; + + private: + // The path containing all settings databases under this root. + const base::FilePath data_path_; + + DISALLOW_COPY_AND_ASSIGN(ModelSettings); + }; + + // Manages two collections of legacy settings databases (apps & extensions) + // within a common base directory. + class SettingsRoot { + public: + // If either |extension_dirname| or |app_dirname| are empty then that + // ModelSetting will *not* be created. + SettingsRoot(const base::FilePath& base_path, + const std::string& extension_dirname, + const std::string& app_dirname); + ~SettingsRoot(); + + std::set<ExtensionId> GetKnownExtensionIDs(ModelType model_type) const; + const ModelSettings* GetModel(ModelType model_type) const; + ModelSettings* GetModel(ModelType model_type); + + private: + scoped_ptr<ModelSettings> extensions_; + scoped_ptr<ModelSettings> apps_; + + DISALLOW_COPY_AND_ASSIGN(SettingsRoot); + }; + + ~LegacyValueStoreFactory() override; + + const SettingsRoot& GetSettingsRoot( + settings_namespace::Namespace settings_namespace) const; + SettingsRoot& GetSettingsRoot( + settings_namespace::Namespace settings_namespace); + + base::FilePath GetRulesDBPath() const; + base::FilePath GetStateDBPath() const; + + const base::FilePath profile_path_; + SettingsRoot local_settings_; + SettingsRoot sync_settings_; + SettingsRoot managed_settings_; + + DISALLOW_COPY_AND_ASSIGN(LegacyValueStoreFactory); +}; + +} // namespace extensions + +#endif // EXTENSIONS_BROWSER_VALUE_STORE_LEGACY_VALUE_STORE_FACTORY_H_ diff --git a/extensions/browser/value_store/test_value_store_factory.cc b/extensions/browser/value_store/test_value_store_factory.cc new file mode 100644 index 0000000..f5135df --- /dev/null +++ b/extensions/browser/value_store/test_value_store_factory.cc @@ -0,0 +1,190 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "extensions/browser/value_store/test_value_store_factory.h" + +#include "extensions/browser/value_store/leveldb_value_store.h" +#include "extensions/browser/value_store/testing_value_store.h" + +namespace { + +const char kUMAClientName[] = "Test"; + +} // namespace + +namespace extensions { + +using SettingsNamespace = settings_namespace::Namespace; + +TestValueStoreFactory::StorageHelper::StorageHelper() = default; + +TestValueStoreFactory::StorageHelper::~StorageHelper() = default; + +std::set<ExtensionId> +TestValueStoreFactory::StorageHelper::GetKnownExtensionIDs( + ModelType model_type) const { + std::set<ExtensionId> ids; + switch (model_type) { + case ValueStoreFactory::ModelType::APP: + for (const auto& key : app_stores_) + ids.insert(key.first); + break; + case ValueStoreFactory::ModelType::EXTENSION: + for (const auto& key : extension_stores_) + ids.insert(key.first); + break; + } + return ids; +} + +void TestValueStoreFactory::StorageHelper::Reset() { + app_stores_.clear(); + extension_stores_.clear(); +} + +ValueStore* TestValueStoreFactory::StorageHelper::AddValueStore( + const ExtensionId& extension_id, + ValueStore* value_store, + ModelType model_type) { + if (model_type == ValueStoreFactory::ModelType::APP) { + DCHECK(app_stores_.find(extension_id) == app_stores_.end()); + app_stores_[extension_id] = value_store; + } else { + DCHECK(extension_stores_.find(extension_id) == extension_stores_.end()); + extension_stores_[extension_id] = value_store; + } + return value_store; +} + +void TestValueStoreFactory::StorageHelper::DeleteSettings( + const ExtensionId& extension_id, + ModelType model_type) { + switch (model_type) { + case ValueStoreFactory::ModelType::APP: + app_stores_.erase(extension_id); + break; + case ValueStoreFactory::ModelType::EXTENSION: + extension_stores_.erase(extension_id); + break; + } +} + +bool TestValueStoreFactory::StorageHelper::HasSettings( + const ExtensionId& extension_id, + ModelType model_type) const { + switch (model_type) { + case ValueStoreFactory::ModelType::APP: + return app_stores_.find(extension_id) != app_stores_.end(); + case ValueStoreFactory::ModelType::EXTENSION: + return extension_stores_.find(extension_id) != extension_stores_.end(); + } + NOTREACHED(); + return false; +} + +ValueStore* TestValueStoreFactory::StorageHelper::GetExisting( + const ExtensionId& extension_id) const { + auto it = app_stores_.find(extension_id); + if (it != app_stores_.end()) + return it->second; + it = extension_stores_.find(extension_id); + if (it != extension_stores_.end()) + return it->second; + return nullptr; +} + +TestValueStoreFactory::TestValueStoreFactory() = default; + +TestValueStoreFactory::TestValueStoreFactory(const base::FilePath& db_path) + : db_path_(db_path) {} + +TestValueStoreFactory::~TestValueStoreFactory() {} + +scoped_ptr<ValueStore> TestValueStoreFactory::CreateRulesStore() { + if (db_path_.empty()) + last_created_store_ = new TestingValueStore(); + else + last_created_store_ = new LeveldbValueStore(kUMAClientName, db_path_); + return make_scoped_ptr(last_created_store_); +} + +scoped_ptr<ValueStore> TestValueStoreFactory::CreateStateStore() { + return CreateRulesStore(); +} + +TestValueStoreFactory::StorageHelper& TestValueStoreFactory::GetStorageHelper( + SettingsNamespace settings_namespace) { + switch (settings_namespace) { + case settings_namespace::LOCAL: + return local_helper_; + case settings_namespace::SYNC: + return sync_helper_; + case settings_namespace::MANAGED: + return managed_helper_; + case settings_namespace::INVALID: + break; + } + NOTREACHED(); + return local_helper_; +} + +scoped_ptr<ValueStore> TestValueStoreFactory::CreateSettingsStore( + SettingsNamespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + scoped_ptr<ValueStore> settings_store(CreateRulesStore()); + // Note: This factory is purposely keeping the raw pointers to each ValueStore + // created. Tests using TestValueStoreFactory must be careful to keep + // those ValueStore's alive for the duration of their test. + GetStorageHelper(settings_namespace) + .AddValueStore(extension_id, settings_store.get(), model_type); + return settings_store; +} + +ValueStore* TestValueStoreFactory::LastCreatedStore() const { + return last_created_store_; +} + +void TestValueStoreFactory::DeleteSettings(SettingsNamespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + GetStorageHelper(settings_namespace).DeleteSettings(extension_id, model_type); +} + +bool TestValueStoreFactory::HasSettings(SettingsNamespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + return GetStorageHelper(settings_namespace) + .HasSettings(extension_id, model_type); +} + +std::set<ExtensionId> TestValueStoreFactory::GetKnownExtensionIDs( + SettingsNamespace settings_namespace, + ModelType model_type) const { + return const_cast<TestValueStoreFactory*>(this) + ->GetStorageHelper(settings_namespace) + .GetKnownExtensionIDs(model_type); +} + +ValueStore* TestValueStoreFactory::GetExisting( + const ExtensionId& extension_id) const { + ValueStore* existing_store = local_helper_.GetExisting(extension_id); + if (existing_store) + return existing_store; + existing_store = sync_helper_.GetExisting(extension_id); + if (existing_store) + return existing_store; + existing_store = managed_helper_.GetExisting(extension_id); + DCHECK(existing_store != nullptr); + return existing_store; +} + +void TestValueStoreFactory::Reset() { + last_created_store_ = nullptr; + local_helper_.Reset(); + sync_helper_.Reset(); + managed_helper_.Reset(); +} + +} // namespace extensions diff --git a/extensions/browser/value_store/test_value_store_factory.h b/extensions/browser/value_store/test_value_store_factory.h new file mode 100644 index 0000000..86fbee77 --- /dev/null +++ b/extensions/browser/value_store/test_value_store_factory.h @@ -0,0 +1,95 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef EXTENSIONS_BROWSER_VALUE_STORE_TEST_VALUE_STORE_FACTORY_H_ +#define EXTENSIONS_BROWSER_VALUE_STORE_TEST_VALUE_STORE_FACTORY_H_ + +#include <map> +#include <set> + +#include "base/memory/scoped_ptr.h" +#include "extensions/browser/value_store/value_store_factory.h" +#include "extensions/common/extension.h" + +class ValueStore; + +namespace extensions { + +// Used for tests when a new test ValueStore is required. Will either open a +// database on disk (if path provided) returning a |LeveldbValueStore|. +// Otherwise a new |TestingValueStore| instance will be returned. +class TestValueStoreFactory : public ValueStoreFactory { + public: + TestValueStoreFactory(); + explicit TestValueStoreFactory(const base::FilePath& db_path); + + // ValueStoreFactory + scoped_ptr<ValueStore> CreateRulesStore() override; + scoped_ptr<ValueStore> CreateStateStore() override; + scoped_ptr<ValueStore> CreateSettingsStore( + settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + void DeleteSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + bool HasSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + std::set<ExtensionId> GetKnownExtensionIDs( + settings_namespace::Namespace settings_namespace, + ModelType model_type) const override; + + // Return the last created |ValueStore|. Use with caution as this may return + // a dangling pointer since the creator now owns the ValueStore which can be + // deleted at any time. + ValueStore* LastCreatedStore() const; + // Return a previously created |ValueStore| for an extension. + ValueStore* GetExisting(const ExtensionId& extension_id) const; + // Reset this class (as if just created). + void Reset(); + + private: + // Manages a collection of |ValueStore|'s created for an app/extension. + // One of these exists for each setting type. + class StorageHelper { + public: + StorageHelper(); + ~StorageHelper(); + std::set<ExtensionId> GetKnownExtensionIDs(ModelType model_type) const; + ValueStore* AddValueStore(const ExtensionId& extension_id, + ValueStore* value_store, + ModelType model_type); + void DeleteSettings(const ExtensionId& extension_id, ModelType model_type); + bool HasSettings(const ExtensionId& extension_id, + ModelType model_type) const; + void Reset(); + ValueStore* GetExisting(const ExtensionId& extension_id) const; + + private: + std::map<ExtensionId, ValueStore*> app_stores_; + std::map<ExtensionId, ValueStore*> extension_stores_; + + DISALLOW_COPY_AND_ASSIGN(StorageHelper); + }; + + StorageHelper& GetStorageHelper( + settings_namespace::Namespace settings_namespace); + + ~TestValueStoreFactory() override; + base::FilePath db_path_; + ValueStore* last_created_store_ = nullptr; + + // None of these value stores are owned by this factory, so care must be + // taken when calling GetExisting. + StorageHelper local_helper_; + StorageHelper sync_helper_; + StorageHelper managed_helper_; + + DISALLOW_COPY_AND_ASSIGN(TestValueStoreFactory); +}; + +} // namespace extensions + +#endif // EXTENSIONS_BROWSER_VALUE_STORE_TEST_VALUE_STORE_FACTORY_H_ diff --git a/extensions/browser/value_store/value_store_factory.h b/extensions/browser/value_store/value_store_factory.h new file mode 100644 index 0000000..ec43547 --- /dev/null +++ b/extensions/browser/value_store/value_store_factory.h @@ -0,0 +1,67 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_FACTORY_H_ +#define EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_FACTORY_H_ + +#include <set> + +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "extensions/browser/api/storage/settings_namespace.h" +#include "extensions/common/extension.h" + +class ValueStore; + +namespace extensions { + +// Create new value stores for rules, state, or settings. For settings will +// also create stores for the specified namespace and model type. +// +// Note: This factory creates the lower level stores that directly read/write to +// disk. Sync/Managed stores are created directly, but delegate their +// calls to a |ValueStore| created by this interface. +class ValueStoreFactory : public base::RefCountedThreadSafe<ValueStoreFactory> { + public: + enum class ModelType { APP, EXTENSION }; + + // Create a |ValueStore| to contain rules data. + virtual scoped_ptr<ValueStore> CreateRulesStore() = 0; + + // Create a |ValueStore| to contain state data. + virtual scoped_ptr<ValueStore> CreateStateStore() = 0; + + // Create a |ValueStore| to contain settings data for a specific extension + // namespace and model type. + virtual scoped_ptr<ValueStore> CreateSettingsStore( + settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) = 0; + + // Delete all settings for specified given extension in the specified + // namespace/model_type. + virtual void DeleteSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) = 0; + + // Are there any settings stored in the specified namespace/model_type for + // the given extension? + virtual bool HasSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) = 0; + + // Return all extension ID's with settings stored in the given + // namespace/model_type. + virtual std::set<ExtensionId> GetKnownExtensionIDs( + settings_namespace::Namespace settings_namespace, + ModelType model_type) const = 0; + + protected: + friend class base::RefCountedThreadSafe<ValueStoreFactory>; + virtual ~ValueStoreFactory() {} +}; + +} // namespace extensions + +#endif // EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_FACTORY_H_ diff --git a/extensions/browser/value_store/value_store_factory_impl.cc b/extensions/browser/value_store/value_store_factory_impl.cc new file mode 100644 index 0000000..27324fd --- /dev/null +++ b/extensions/browser/value_store/value_store_factory_impl.cc @@ -0,0 +1,53 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "extensions/browser/value_store/value_store_factory_impl.h" + +#include "extensions/browser/value_store/legacy_value_store_factory.h" + +namespace extensions { + +using SettingsNamespace = settings_namespace::Namespace; + +ValueStoreFactoryImpl::ValueStoreFactoryImpl(const base::FilePath& profile_path) + : legacy_factory_(new LegacyValueStoreFactory(profile_path)) {} + +ValueStoreFactoryImpl::~ValueStoreFactoryImpl() = default; + +scoped_ptr<ValueStore> ValueStoreFactoryImpl::CreateRulesStore() { + return legacy_factory_->CreateRulesStore(); +} + +scoped_ptr<ValueStore> ValueStoreFactoryImpl::CreateStateStore() { + return legacy_factory_->CreateStateStore(); +} + +scoped_ptr<ValueStore> ValueStoreFactoryImpl::CreateSettingsStore( + SettingsNamespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + return legacy_factory_->CreateSettingsStore(settings_namespace, model_type, + extension_id); +} + +void ValueStoreFactoryImpl::DeleteSettings(SettingsNamespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + legacy_factory_->DeleteSettings(settings_namespace, model_type, extension_id); +} + +bool ValueStoreFactoryImpl::HasSettings(SettingsNamespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) { + return legacy_factory_->HasSettings(settings_namespace, model_type, + extension_id); +} + +std::set<ExtensionId> ValueStoreFactoryImpl::GetKnownExtensionIDs( + SettingsNamespace settings_namespace, + ModelType model_type) const { + return legacy_factory_->GetKnownExtensionIDs(settings_namespace, model_type); +} + +} // namespace extensions diff --git a/extensions/browser/value_store/value_store_factory_impl.h b/extensions/browser/value_store/value_store_factory_impl.h new file mode 100644 index 0000000..d0a0677 --- /dev/null +++ b/extensions/browser/value_store/value_store_factory_impl.h @@ -0,0 +1,56 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_FACTORY_IMPL_H_ +#define EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_FACTORY_IMPL_H_ + +#include <set> +#include <string> + +#include "base/files/file_path.h" +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "extensions/browser/value_store/value_store.h" +#include "extensions/browser/value_store/value_store_factory.h" +#include "extensions/common/extension.h" + +namespace extensions { + +class LegacyValueStoreFactory; + +// Mint new |ValueStore| instances for use by the extensions system. These are +// used for extension rules, state, and settings. +class ValueStoreFactoryImpl : public ValueStoreFactory { + public: + explicit ValueStoreFactoryImpl(const base::FilePath& profile_path); + + // ValueStoreFactory + scoped_ptr<ValueStore> CreateRulesStore() override; + scoped_ptr<ValueStore> CreateStateStore() override; + scoped_ptr<ValueStore> CreateSettingsStore( + settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + void DeleteSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + bool HasSettings(settings_namespace::Namespace settings_namespace, + ModelType model_type, + const ExtensionId& extension_id) override; + std::set<ExtensionId> GetKnownExtensionIDs( + settings_namespace::Namespace settings_namespace, + ModelType model_type) const override; + + private: + // ValueStoreFactory is refcounted. + ~ValueStoreFactoryImpl() override; + + scoped_refptr<LegacyValueStoreFactory> legacy_factory_; + + DISALLOW_COPY_AND_ASSIGN(ValueStoreFactoryImpl); +}; + +} // namespace extensions + +#endif // EXTENSIONS_BROWSER_VALUE_STORE_VALUE_STORE_FACTORY_IMPL_H_ diff --git a/extensions/browser/value_store/value_store_frontend.cc b/extensions/browser/value_store/value_store_frontend.cc index 6f37bbc..b3912d6 100644 --- a/extensions/browser/value_store/value_store_frontend.cc +++ b/extensions/browser/value_store/value_store_frontend.cc @@ -13,31 +13,21 @@ #include "base/trace_event/trace_event.h" #include "content/public/browser/browser_thread.h" #include "extensions/browser/value_store/leveldb_value_store.h" +#include "extensions/browser/value_store/value_store_factory.h" using content::BrowserThread; +using extensions::ValueStoreFactory; class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> { public: - Backend() : storage_(NULL) {} - - void Init(const std::string& uma_client_name, const base::FilePath& db_path) { - DCHECK_CURRENTLY_ON(BrowserThread::FILE); - DCHECK(!storage_); - TRACE_EVENT0("ValueStoreFrontend::Backend", "Init"); - db_path_ = db_path; - storage_ = new LeveldbValueStore(uma_client_name, db_path); - } - - // This variant is useful for testing (using a mock ValueStore). - void InitWithStore(scoped_ptr<ValueStore> storage) { - DCHECK_CURRENTLY_ON(BrowserThread::FILE); - DCHECK(!storage_); - storage_ = storage.release(); - } + Backend(const scoped_refptr<ValueStoreFactory>& store_factory, + BackendType backend_type) + : store_factory_(store_factory), backend_type_(backend_type) {} void Get(const std::string& key, const ValueStoreFrontend::ReadCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::FILE); + LazyInit(); ValueStore::ReadResult result = storage_->Get(key); // Extract the value from the ReadResult and pass ownership of it to the @@ -57,6 +47,7 @@ class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> { void Set(const std::string& key, scoped_ptr<base::Value> value) { DCHECK_CURRENTLY_ON(BrowserThread::FILE); + LazyInit(); // We don't need the old value, so skip generating changes. ValueStore::WriteResult result = storage_->Set( ValueStore::IGNORE_QUOTA | ValueStore::NO_GENERATE_CHANGES, @@ -68,6 +59,7 @@ class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> { void Remove(const std::string& key) { DCHECK_CURRENTLY_ON(BrowserThread::FILE); + LazyInit(); storage_->Remove(key); } @@ -75,10 +67,23 @@ class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> { friend class base::RefCountedThreadSafe<Backend>; virtual ~Backend() { - if (BrowserThread::CurrentlyOn(BrowserThread::FILE)) { - delete storage_; - } else { - BrowserThread::DeleteSoon(BrowserThread::FILE, FROM_HERE, storage_); + if (storage_ && !BrowserThread::CurrentlyOn(BrowserThread::FILE)) + BrowserThread::DeleteSoon(BrowserThread::FILE, FROM_HERE, + storage_.release()); + } + + void LazyInit() { + DCHECK_CURRENTLY_ON(BrowserThread::FILE); + if (storage_) + return; + TRACE_EVENT0("ValueStoreFrontend::Backend", "LazyInit"); + switch (backend_type_) { + case BackendType::RULES: + storage_ = store_factory_->CreateRulesStore(); + break; + case BackendType::STATE: + storage_ = store_factory_->CreateStateStore(); + break; } } @@ -88,43 +93,29 @@ class ValueStoreFrontend::Backend : public base::RefCountedThreadSafe<Backend> { callback.Run(std::move(value)); } + // The factory which will be used to lazily create the ValueStore when needed. + // Used exclusively on the FILE thread. + scoped_refptr<ValueStoreFactory> store_factory_; + BackendType backend_type_; + // The actual ValueStore that handles persisting the data to disk. Used // exclusively on the FILE thread. - ValueStore* storage_; + scoped_ptr<ValueStore> storage_; base::FilePath db_path_; DISALLOW_COPY_AND_ASSIGN(Backend); }; -ValueStoreFrontend::ValueStoreFrontend() - : backend_(new Backend()) { -} - -ValueStoreFrontend::ValueStoreFrontend(const std::string& uma_client_name, - const base::FilePath& db_path) - : backend_(new Backend()) { - Init(uma_client_name, db_path); -} - -ValueStoreFrontend::ValueStoreFrontend(scoped_ptr<ValueStore> value_store) - : backend_(new Backend()) { - BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, - base::Bind(&ValueStoreFrontend::Backend::InitWithStore, - backend_, base::Passed(&value_store))); -} +ValueStoreFrontend::ValueStoreFrontend( + const scoped_refptr<ValueStoreFactory>& store_factory, + BackendType backend_type) + : backend_(new Backend(store_factory, backend_type)) {} ValueStoreFrontend::~ValueStoreFrontend() { DCHECK(CalledOnValidThread()); } -void ValueStoreFrontend::Init(const std::string& uma_client_name, - const base::FilePath& db_path) { - BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, - base::Bind(&ValueStoreFrontend::Backend::Init, - backend_, uma_client_name, db_path)); -} - void ValueStoreFrontend::Get(const std::string& key, const ReadCallback& callback) { DCHECK(CalledOnValidThread()); diff --git a/extensions/browser/value_store/value_store_frontend.h b/extensions/browser/value_store/value_store_frontend.h index 972efad..55253fb 100644 --- a/extensions/browser/value_store/value_store_frontend.h +++ b/extensions/browser/value_store/value_store_frontend.h @@ -20,22 +20,25 @@ namespace base { class FilePath; } +namespace extensions { +class ValueStoreFactory; +} // namespace extensions + // A frontend for a LeveldbValueStore, for use on the UI thread. class ValueStoreFrontend : public base::SupportsWeakPtr<ValueStoreFrontend>, public base::NonThreadSafe { public: + // The kind of extensions data stored in a backend. + enum class BackendType { RULES, STATE }; + typedef base::Callback<void(scoped_ptr<base::Value>)> ReadCallback; - ValueStoreFrontend(); - ValueStoreFrontend(const std::string& uma_client_name, - const base::FilePath& db_path); - // This variant is useful for testing (using a mock ValueStore). - explicit ValueStoreFrontend(scoped_ptr<ValueStore> value_store); + ValueStoreFrontend( + const scoped_refptr<extensions::ValueStoreFactory>& store_factory, + BackendType backend_type); ~ValueStoreFrontend(); - void Init(const std::string& uma_client_name, const base::FilePath& db_path); - // 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); diff --git a/extensions/browser/value_store/value_store_frontend_unittest.cc b/extensions/browser/value_store/value_store_frontend_unittest.cc index 0ee4c5c..21f95b6 100644 --- a/extensions/browser/value_store/value_store_frontend_unittest.cc +++ b/extensions/browser/value_store/value_store_frontend_unittest.cc @@ -12,13 +12,12 @@ #include "base/message_loop/message_loop.h" #include "base/path_service.h" #include "content/public/test/test_browser_thread.h" +#include "extensions/browser/value_store/test_value_store_factory.h" #include "extensions/common/extension_paths.h" #include "testing/gtest/include/gtest/gtest.h" using content::BrowserThread; -const char kDatabaseUMAClientName[] = "Test"; - class ValueStoreFrontendTest : public testing::Test { public: ValueStoreFrontendTest() @@ -35,6 +34,8 @@ class ValueStoreFrontendTest : public testing::Test { db_path_ = temp_dir_.path().AppendASCII("temp_db"); base::CopyDirectory(src_db, db_path_, true); + factory_ = new extensions::TestValueStoreFactory(db_path_); + ResetStorage(); } @@ -45,7 +46,8 @@ class ValueStoreFrontendTest : public testing::Test { // Reset the value store, reloading the DB from disk. void ResetStorage() { - storage_.reset(new ValueStoreFrontend(kDatabaseUMAClientName, db_path_)); + storage_.reset(new ValueStoreFrontend( + factory_, ValueStoreFrontend::BackendType::RULES)); } bool Get(const std::string& key, scoped_ptr<base::Value>* output) { @@ -62,6 +64,7 @@ class ValueStoreFrontendTest : public testing::Test { base::MessageLoop::current()->QuitWhenIdle(); } + scoped_refptr<extensions::TestValueStoreFactory> factory_; scoped_ptr<ValueStoreFrontend> storage_; base::ScopedTempDir temp_dir_; base::FilePath db_path_; diff --git a/extensions/extensions.gypi b/extensions/extensions.gypi index 7af2233..69963fa 100644 --- a/extensions/extensions.gypi +++ b/extensions/extensions.gypi @@ -432,14 +432,11 @@ 'browser/api/sockets_udp/sockets_udp_api.h', 'browser/api/sockets_udp/udp_socket_event_dispatcher.cc', 'browser/api/sockets_udp/udp_socket_event_dispatcher.h', - 'browser/api/storage/leveldb_settings_storage_factory.cc', - 'browser/api/storage/leveldb_settings_storage_factory.h', 'browser/api/storage/local_value_store_cache.cc', 'browser/api/storage/local_value_store_cache.h', 'browser/api/storage/settings_namespace.cc', 'browser/api/storage/settings_namespace.h', 'browser/api/storage/settings_observer.h', - 'browser/api/storage/settings_storage_factory.h', 'browser/api/storage/settings_storage_quota_enforcer.cc', 'browser/api/storage/settings_storage_quota_enforcer.h', 'browser/api/storage/storage_api.cc', @@ -790,14 +787,21 @@ 'browser/user_script_loader.h', 'browser/value_store/lazy_leveldb.cc', 'browser/value_store/lazy_leveldb.h', + 'browser/value_store/legacy_value_store_factory.cc', + 'browser/value_store/legacy_value_store_factory.h', 'browser/value_store/leveldb_value_store.cc', 'browser/value_store/leveldb_value_store.h', + 'browser/value_store/test_value_store_factory.cc', + 'browser/value_store/test_value_store_factory.h', 'browser/value_store/testing_value_store.cc', 'browser/value_store/testing_value_store.h', 'browser/value_store/value_store.cc', 'browser/value_store/value_store.h', 'browser/value_store/value_store_change.cc', 'browser/value_store/value_store_change.h', + 'browser/value_store/value_store_factory.h', + 'browser/value_store/value_store_factory_impl.cc', + 'browser/value_store/value_store_factory_impl.h', 'browser/value_store/value_store_frontend.cc', 'browser/value_store/value_store_frontend.h', 'browser/verified_contents.cc', diff --git a/extensions/extensions_tests.gypi b/extensions/extensions_tests.gypi index 5463453..845924d 100644 --- a/extensions/extensions_tests.gypi +++ b/extensions/extensions_tests.gypi @@ -94,11 +94,11 @@ 'browser/process_map_unittest.cc', 'browser/quota_service_unittest.cc', 'browser/runtime_data_unittest.cc', - 'browser/sandboxed_unpacker_unittest.cc', 'browser/extension_throttle_simulation_unittest.cc', 'browser/extension_throttle_test_support.cc', 'browser/extension_throttle_test_support.h', 'browser/extension_throttle_unittest.cc', + 'browser/sandboxed_unpacker_unittest.cc', 'browser/updater/update_service_unittest.cc', 'browser/value_store/leveldb_value_store_unittest.cc', 'browser/value_store/testing_value_store_unittest.cc', diff --git a/extensions/shell/browser/shell_extension_system.cc b/extensions/shell/browser/shell_extension_system.cc index 2ed9d7b..3fbbf14 100644 --- a/extensions/shell/browser/shell_extension_system.cc +++ b/extensions/shell/browser/shell_extension_system.cc @@ -22,6 +22,7 @@ #include "extensions/browser/quota_service.h" #include "extensions/browser/runtime_data.h" #include "extensions/browser/service_worker_manager.h" +#include "extensions/browser/value_store/value_store_factory_impl.h" #include "extensions/common/constants.h" #include "extensions/common/file_util.h" @@ -31,7 +32,9 @@ using content::BrowserThread; namespace extensions { ShellExtensionSystem::ShellExtensionSystem(BrowserContext* browser_context) - : browser_context_(browser_context), weak_factory_(this) {} + : browser_context_(browser_context), + store_factory_(new ValueStoreFactoryImpl(browser_context->GetPath())), + weak_factory_(this) {} ShellExtensionSystem::~ShellExtensionSystem() { } @@ -134,6 +137,10 @@ StateStore* ShellExtensionSystem::rules_store() { return nullptr; } +scoped_refptr<ValueStoreFactory> ShellExtensionSystem::store_factory() { + return store_factory_; +} + InfoMap* ShellExtensionSystem::info_map() { if (!info_map_.get()) info_map_ = new InfoMap; diff --git a/extensions/shell/browser/shell_extension_system.h b/extensions/shell/browser/shell_extension_system.h index b84c3fd..4448fb9 100644 --- a/extensions/shell/browser/shell_extension_system.h +++ b/extensions/shell/browser/shell_extension_system.h @@ -23,6 +23,8 @@ class BrowserContext; namespace extensions { +class ValueStoreFactory; + // A simplified version of ExtensionSystem for app_shell. Allows // app_shell to skip initialization of services it doesn't need. class ShellExtensionSystem : public ExtensionSystem { @@ -52,6 +54,7 @@ class ShellExtensionSystem : public ExtensionSystem { SharedUserScriptMaster* shared_user_script_master() override; StateStore* state_store() override; StateStore* rules_store() override; + scoped_refptr<ValueStoreFactory> store_factory() override; InfoMap* info_map() override; QuotaService* quota_service() override; AppSorting* app_sorting() override; @@ -81,6 +84,8 @@ class ShellExtensionSystem : public ExtensionSystem { scoped_ptr<QuotaService> quota_service_; scoped_ptr<AppSorting> app_sorting_; + scoped_refptr<ValueStoreFactory> store_factory_; + // Signaled when the extension system has completed its startup tasks. OneShotEvent ready_; diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt index 3afbc46..47bdb1b 100644 --- a/tools/valgrind/memcheck/suppressions.txt +++ b/tools/valgrind/memcheck/suppressions.txt @@ -1730,20 +1730,6 @@ fun:_ZN3gpu5gles216GLES2DecoderImpl26InitializeShaderTranslatorEv } { - bug_163922 - Memcheck:Leak - fun:_Znw* - fun:_ZN10extensions16SettingsFrontendC1ERK13scoped_refptrINS_22SettingsStorageFactoryEEP7Profile - fun:_ZN10extensions16SettingsFrontend6CreateEP7Profile - fun:_ZN16ExtensionServiceC1E* - fun:_ZN10extensions19ExtensionSystemImpl6Shared4InitEb - fun:_ZN10extensions19ExtensionSystemImpl21InitForRegularProfileEb - fun:_ZN14ProfileManager22DoFinalInitForServicesEP7Profileb - fun:_ZN14ProfileManager11DoFinalInitEP7Profileb - fun:_ZN14ProfileManager10AddProfileEP7Profile - fun:_ZN14ProfileManager10GetProfileE* -} -{ bug_163924 Memcheck:Leak fun:_Znw* |