diff options
23 files changed, 693 insertions, 185 deletions
diff --git a/chrome/browser/dom_ui/new_tab_ui_uitest.cc b/chrome/browser/dom_ui/new_tab_ui_uitest.cc index f2e96df..39f41e2 100644 --- a/chrome/browser/dom_ui/new_tab_ui_uitest.cc +++ b/chrome/browser/dom_ui/new_tab_ui_uitest.cc @@ -75,30 +75,26 @@ TEST_F(NewTabUITest, FLAKY_ChromeInternalLoadsNTP) { TEST_F(NewTabUITest, UpdateUserPrefsVersion) { // PrefService with JSON user-pref file only, no enforced or advised prefs. - PrefService prefs(new PrefValueStore( - NULL, /* no enforced prefs */ - new JsonPrefStore( - FilePath(), - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - /* user prefs */ - NULL /* no advised prefs */)); + FilePath user_prefs = FilePath(); + scoped_ptr<PrefService> prefs( + PrefService::CreateUserPrefService(user_prefs)); // Does the migration - NewTabUI::RegisterUserPrefs(&prefs); + NewTabUI::RegisterUserPrefs(prefs.get()); ASSERT_EQ(NewTabUI::current_pref_version(), - prefs.GetInteger(prefs::kNTPPrefVersion)); + prefs->GetInteger(prefs::kNTPPrefVersion)); // Reset the version - prefs.ClearPref(prefs::kNTPPrefVersion); - ASSERT_EQ(0, prefs.GetInteger(prefs::kNTPPrefVersion)); + prefs->ClearPref(prefs::kNTPPrefVersion); + ASSERT_EQ(0, prefs->GetInteger(prefs::kNTPPrefVersion)); - bool migrated = NewTabUI::UpdateUserPrefsVersion(&prefs); + bool migrated = NewTabUI::UpdateUserPrefsVersion(prefs.get()); ASSERT_TRUE(migrated); ASSERT_EQ(NewTabUI::current_pref_version(), - prefs.GetInteger(prefs::kNTPPrefVersion)); + prefs->GetInteger(prefs::kNTPPrefVersion)); - migrated = NewTabUI::UpdateUserPrefsVersion(&prefs); + migrated = NewTabUI::UpdateUserPrefsVersion(prefs.get()); ASSERT_FALSE(migrated); } diff --git a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc index 031a2b6..42fa47a 100644 --- a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc +++ b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc @@ -19,20 +19,16 @@ class ShownSectionsHandlerTest : public testing::Test { TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs) { // Create a preference value that has only user defined // preference values. - PrefService pref(new PrefValueStore( - NULL, /* no managed preference values */ - new JsonPrefStore( /* user defined preference values */ - FilePath(), - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* no suggested preference values */)); + FilePath user_prefs = FilePath(); + scoped_ptr<PrefService> pref(PrefService::CreateUserPrefService(user_prefs)); // Set an *old* value - pref.RegisterIntegerPref(prefs::kNTPShownSections, 0); - pref.SetInteger(prefs::kNTPShownSections, THUMB); + pref->RegisterIntegerPref(prefs::kNTPShownSections, 0); + pref->SetInteger(prefs::kNTPShownSections, THUMB); - ShownSectionsHandler::MigrateUserPrefs(&pref, 0, 1); + ShownSectionsHandler::MigrateUserPrefs(pref.get(), 0, 1); - int shown_sections = pref.GetInteger(prefs::kNTPShownSections); + int shown_sections = pref->GetInteger(prefs::kNTPShownSections); EXPECT_TRUE(shown_sections & THUMB); EXPECT_FALSE(shown_sections & LIST); @@ -42,20 +38,16 @@ TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs) { } TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs1To2) { - PrefService pref(new PrefValueStore( - NULL, /* no managed preferences */ - new JsonPrefStore( - FilePath(), - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* no suggested preferences */)); + FilePath user_prefs = FilePath(); + scoped_ptr<PrefService> pref(PrefService::CreateUserPrefService(user_prefs)); // Set an *old* value - pref.RegisterIntegerPref(prefs::kNTPShownSections, 0); - pref.SetInteger(prefs::kNTPShownSections, LIST); + pref->RegisterIntegerPref(prefs::kNTPShownSections, 0); + pref->SetInteger(prefs::kNTPShownSections, LIST); - ShownSectionsHandler::MigrateUserPrefs(&pref, 1, 2); + ShownSectionsHandler::MigrateUserPrefs(pref.get(), 1, 2); - int shown_sections = pref.GetInteger(prefs::kNTPShownSections); + int shown_sections = pref->GetInteger(prefs::kNTPShownSections); EXPECT_TRUE(shown_sections & THUMB); EXPECT_FALSE(shown_sections & LIST); diff --git a/chrome/browser/extensions/extension_pref_store.cc b/chrome/browser/extensions/extension_pref_store.cc new file mode 100644 index 0000000..fd0a063 --- /dev/null +++ b/chrome/browser/extensions/extension_pref_store.cc @@ -0,0 +1,105 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/extensions/extension_pref_store.h" + +#include "base/logging.h" +#include "base/values.h" +#include "chrome/browser/pref_service.h" + +ExtensionPrefStore::ExtensionPrefStore(PrefService* pref_service) + : pref_service_(pref_service), + prefs_(new DictionaryValue()) { +} + +// This could be sped up by keeping track of which extension currently controls +// a given preference, among other optimizations. But we estimate that fewer +// than 10 installed extensions will be trying to control any preferences, so +// stick with this simpler algorithm until it causes a problem. +void ExtensionPrefStore::UpdateOnePref(const wchar_t* path) { + // Query the PrefService to find the current value for this pref. + // pref_service_ might be null in unit tests. + scoped_ptr<Value> old_value; + if (pref_service_) { + old_value.reset( + pref_service_->FindPreference(path)->GetValue()->DeepCopy()); + } + + // DictionaryValue::Set complains if a key is overwritten with the same + // value, so delete it first. + prefs_->Remove(path, NULL); + + // Find the first extension that wants to set this pref and use its value. + for (ExtensionStack::iterator ext_iter = extension_stack_.begin(); + ext_iter != extension_stack_.end(); ++ext_iter) { + PrefValueMap::iterator value_iter = (*ext_iter)->pref_values->find(path); + if (value_iter != (*ext_iter)->pref_values->end()) { + prefs_->Set(path, (*value_iter).second->DeepCopy()); + break; + } + } + if (pref_service_) + pref_service_->FireObserversIfChanged(path, old_value.get()); +} + +void ExtensionPrefStore::UpdatePrefs(const PrefValueMap* pref_values) { + for (PrefValueMap::const_iterator i = pref_values->begin(); + i != pref_values->end(); ++i) { + UpdateOnePref(i->first); + } +} + +void ExtensionPrefStore::InstallExtensionPref(std::string extension_id, + const wchar_t* pref_path, + Value* pref_value) { + ExtensionStack::iterator i; + for (i = extension_stack_.begin(); i != extension_stack_.end(); ++i) { + if ((*i)->extension_id == extension_id) + break; + } + + // If this extension is already in the stack, update or add the value of this + // preference, but don't change the extension's position in the stack. + // Otherwise, push the new extension onto the stack. + PrefValueMap* pref_values; + if (i != extension_stack_.end()) { + pref_values = (*i)->pref_values; + (*pref_values)[pref_path] = pref_value; + } else { + pref_values = new PrefValueMap(); + (*pref_values)[pref_path] = pref_value; + + ExtensionPrefs* extension_prefs = new ExtensionPrefs(); + extension_prefs->extension_id = extension_id; + extension_prefs->pref_values = pref_values; + extension_stack_.push_front(extension_prefs); + } + + // Look for an old value with the same type as the one we're modifying. + UpdateOnePref(pref_path); +} + +void ExtensionPrefStore::UninstallExtension(std::string extension_id) { + // Remove this extension from the stack. + scoped_ptr<PrefValueMap> pref_values; + for (ExtensionStack::iterator i = extension_stack_.begin(); + i != extension_stack_.end(); ++i) { + if ((*i)->extension_id == extension_id) { + pref_values.reset((*i)->pref_values); + extension_stack_.erase(i); + break; + } + } + if (!pref_values.get()) + return; + + UpdatePrefs(pref_values.get()); +} + +void ExtensionPrefStore::GetExtensionIDs(std::vector<std::string>* result) { + for (ExtensionStack::iterator i = extension_stack_.begin(); + i != extension_stack_.end(); ++i) { + (*result).push_back((*i)->extension_id); + } +} diff --git a/chrome/browser/extensions/extension_pref_store.h b/chrome/browser/extensions/extension_pref_store.h new file mode 100644 index 0000000..a0c420f --- /dev/null +++ b/chrome/browser/extensions/extension_pref_store.h @@ -0,0 +1,87 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_PREF_STORE_H_ +#define CHROME_BROWSER_EXTENSIONS_EXTENSION_PREF_STORE_H_ + +#include <list> +#include <map> +#include <string> +#include <vector> + +#include "base/scoped_ptr.h" +#include "chrome/common/pref_store.h" + +class DictionaryValue; +class PrefService; +class Value; + +// This PrefStore keeps track of preferences set by extensions: for example, +// proxy settings. A stack of relevant extension IDs is stored in order of +// their addition to this PrefStore. For each preference, the last-added +// enabled extension that tries to set it overrules any others. +class ExtensionPrefStore : public PrefStore { + public: + explicit ExtensionPrefStore(PrefService* pref_service); + virtual ~ExtensionPrefStore() {} + + // The PrefService creates the ExtensionPrefStore, so we need to be able to + // defer setting the PrefService here until after construction. + void SetPrefService(PrefService* pref_service) { + pref_service_ = pref_service; + } + + // Begins tracking the preference and value an extension wishes to set. This + // must be called each time an extension API tries to set a preference. + virtual void InstallExtensionPref(std::string extension_id, + const wchar_t* pref_path, + Value* pref_value); + + // Removes an extension and all its preference settings from this PrefStore. + // This must be called when an extension is uninstalled or disabled. + virtual void UninstallExtension(std::string extension_id); + + // PrefStore methods: + virtual DictionaryValue* prefs() { return prefs_.get(); } + + virtual PrefReadError ReadPrefs() { return PREF_READ_ERROR_NONE; } + + protected: + // Returns a vector of the extension IDs in the extension_stack_. + // This should only be accessed by subclasses for unit-testing. + void GetExtensionIDs(std::vector<std::string>* result); + + private: + // The pref service referring to this pref store. Weak reference. + PrefService* pref_service_; + + // Maps preference paths to their values. + typedef std::map<const wchar_t*, Value*> PrefValueMap; + + // Applies the highest-priority extension's setting for the given preference + // path, or clears the setting in this PrefStore if no extensions wish to + // control it. + void UpdateOnePref(const wchar_t* path); + + // Updates each preference in the |pref_values| list. + void UpdatePrefs(const PrefValueMap* pref_values); + + // A cache of the highest-priority values for each preference that any + // extension is controlling, for quick read access. Owns the stored values. + scoped_ptr<DictionaryValue> prefs_; + + // Associates an extension ID with the prefs it sets. + struct ExtensionPrefs { + std::string extension_id; + PrefValueMap* pref_values; + }; + + // A pseudo-stack of extensions and their preferences. Extensions are always + // added to the head, but may be removed from the middle. This stack owns + // the values in the extensions' PrefValueMaps. + typedef std::list<ExtensionPrefs*> ExtensionStack; + ExtensionStack extension_stack_; +}; + +#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_PREF_STORE_H_ diff --git a/chrome/browser/extensions/extension_pref_store_unittest.cc b/chrome/browser/extensions/extension_pref_store_unittest.cc new file mode 100644 index 0000000..69187cb --- /dev/null +++ b/chrome/browser/extensions/extension_pref_store_unittest.cc @@ -0,0 +1,279 @@ +// Copyright (c) 2010 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 <gtest/gtest.h> + +#include <string> +#include <vector> + +#include "base/values.h" +#include "chrome/browser/extensions/extension_pref_store.h" +#include "chrome/browser/pref_service.h" +#include "chrome/browser/pref_value_store.h" + +namespace { + +class TestExtensionPrefStore : public ExtensionPrefStore { + public: + TestExtensionPrefStore() : ExtensionPrefStore(NULL) {} + + typedef std::vector<std::string> ExtensionIDs; + void GetExtensionIDList(ExtensionIDs* result) { + GetExtensionIDs(result); + } +}; + +class MockPrefService : public PrefService { + public: + explicit MockPrefService(PrefValueStore* value_store) + : PrefService(value_store) {} + + // Tracks whether the observers would have been notified. + virtual void FireObserversIfChanged(const wchar_t* pref_name, + const Value* old_value) { + fired_observers_ = PrefIsChanged(pref_name, old_value); + } + + bool fired_observers_; +}; + +// Use static constants to avoid confusing std::map with hard-coded strings. +static const wchar_t* kPref1 = L"path1.subpath"; +static const wchar_t* kPref2 = L"path2"; +static const wchar_t* kPref3 = L"path3"; +static const wchar_t* kPref4 = L"path4"; + +} // namespace + +TEST(ExtensionPrefStoreTest, InstallOneExtension) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(1u, ids.size()); + EXPECT_EQ("id1", ids[0]); + + DictionaryValue* prefs = eps.prefs(); + ASSERT_EQ(1u, prefs->size()); + std::string actual; + ASSERT_TRUE(prefs->GetString(kPref1, &actual)); + EXPECT_EQ("val1", actual); +} + +// Make sure the last-installed extension wins. +TEST(ExtensionPrefStoreTest, InstallMultipleExtensions) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + eps.InstallExtensionPref("id2", kPref1, Value::CreateStringValue("val2")); + eps.InstallExtensionPref("id3", kPref1, Value::CreateStringValue("val3")); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(3u, ids.size()); + EXPECT_EQ("id3", ids[0]); + EXPECT_EQ("id2", ids[1]); + EXPECT_EQ("id1", ids[2]); + + DictionaryValue* prefs = eps.prefs(); + ASSERT_EQ(1u, prefs->size()); + std::string actual; + ASSERT_TRUE(prefs->GetString(kPref1, &actual)); + EXPECT_EQ("val3", actual); +} + +// Make sure the last-installed extension wins for each preference. +TEST(ExtensionPrefStoreTest, InstallOverwrittenExtensions) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + eps.InstallExtensionPref("id2", kPref1, Value::CreateStringValue("val2")); + eps.InstallExtensionPref("id3", kPref1, Value::CreateStringValue("val3")); + + eps.InstallExtensionPref("id1", kPref2, Value::CreateStringValue("val4")); + eps.InstallExtensionPref("id2", kPref2, Value::CreateStringValue("val5")); + + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val6")); + eps.InstallExtensionPref("id1", kPref2, Value::CreateStringValue("val7")); + eps.InstallExtensionPref("id1", kPref3, Value::CreateStringValue("val8")); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(3u, ids.size()); + EXPECT_EQ("id3", ids[0]); + EXPECT_EQ("id2", ids[1]); + EXPECT_EQ("id1", ids[2]); + + DictionaryValue* prefs = eps.prefs(); + ASSERT_EQ(3u, prefs->size()); + std::string actual; + EXPECT_TRUE(prefs->GetString(kPref1, &actual)); + EXPECT_EQ("val3", actual); + EXPECT_TRUE(prefs->GetString(kPref2, &actual)); + EXPECT_EQ("val5", actual); + EXPECT_TRUE(prefs->GetString(kPref3, &actual)); + EXPECT_EQ("val8", actual); +} + +// Make sure the last-installed extension wins even if other extensions set +// the same or different preferences later. +TEST(ExtensionPrefStoreTest, InstallInterleavedExtensions) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + eps.InstallExtensionPref("id2", kPref2, Value::CreateStringValue("val2")); + eps.InstallExtensionPref("id3", kPref3, Value::CreateStringValue("val3")); + + eps.InstallExtensionPref("id3", kPref3, Value::CreateStringValue("val4")); + eps.InstallExtensionPref("id2", kPref3, Value::CreateStringValue("val5")); + eps.InstallExtensionPref("id1", kPref3, Value::CreateStringValue("val6")); + + eps.InstallExtensionPref("id3", kPref1, Value::CreateStringValue("val7")); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(3u, ids.size()); + EXPECT_EQ("id3", ids[0]); + EXPECT_EQ("id2", ids[1]); + EXPECT_EQ("id1", ids[2]); + + DictionaryValue* prefs = eps.prefs(); + ASSERT_EQ(3u, prefs->size()); + std::string actual; + EXPECT_TRUE(prefs->GetString(kPref1, &actual)); + EXPECT_EQ("val7", actual); + EXPECT_TRUE(prefs->GetString(kPref2, &actual)); + EXPECT_EQ("val2", actual); + EXPECT_TRUE(prefs->GetString(kPref3, &actual)); + EXPECT_EQ("val4", actual); +} + +TEST(ExtensionPrefStoreTest, UninstallOnlyExtension) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + eps.InstallExtensionPref("id1", kPref2, Value::CreateStringValue("val2")); + + // No need to check the state here; the InstallOneExtension already has. + eps.UninstallExtension("id1"); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(0u, ids.size()); + + DictionaryValue* prefs = eps.prefs(); + std::string actual; + // "path1.name" has been removed, but an empty "path1" dictionary is still + // present. + ASSERT_EQ(1u, prefs->size()); + EXPECT_FALSE(prefs->GetString(kPref1, &actual)); + EXPECT_FALSE(prefs->GetString(kPref2, &actual)); +} + +// Tests uninstalling an extension that wasn't winning for any preferences. +TEST(ExtensionPrefStoreTest, UninstallIrrelevantExtension) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + eps.InstallExtensionPref("id2", kPref1, Value::CreateStringValue("val2")); + + eps.InstallExtensionPref("id1", kPref2, Value::CreateStringValue("val3")); + eps.InstallExtensionPref("id2", kPref2, Value::CreateStringValue("val4")); + + eps.UninstallExtension("id1"); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(1u, ids.size()); + EXPECT_EQ("id2", ids[0]); + + DictionaryValue* prefs = eps.prefs(); + ASSERT_EQ(2u, prefs->size()); + std::string actual; + EXPECT_TRUE(prefs->GetString(kPref1, &actual)); + EXPECT_EQ("val2", actual); + EXPECT_TRUE(prefs->GetString(kPref2, &actual)); + EXPECT_EQ("val4", actual); +} + +// Tests uninstalling an extension that was winning for all preferences. +TEST(ExtensionPrefStoreTest, UninstallExtensionFromTop) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + eps.InstallExtensionPref("id2", kPref1, Value::CreateStringValue("val2")); + eps.InstallExtensionPref("id3", kPref1, Value::CreateStringValue("val3")); + + eps.InstallExtensionPref("id1", kPref2, Value::CreateStringValue("val4")); + eps.InstallExtensionPref("id3", kPref2, Value::CreateStringValue("val5")); + + eps.UninstallExtension("id3"); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(2u, ids.size()); + EXPECT_EQ("id2", ids[0]); + EXPECT_EQ("id1", ids[1]); + + DictionaryValue* prefs = eps.prefs(); + ASSERT_EQ(2u, prefs->size()); + std::string actual; + EXPECT_TRUE(prefs->GetString(kPref1, &actual)); + EXPECT_EQ("val2", actual); + EXPECT_TRUE(prefs->GetString(kPref2, &actual)); + EXPECT_EQ("val4", actual); +} + +// Tests uninstalling an extension that was winning for only some preferences. +TEST(ExtensionPrefStoreTest, UninstallExtensionFromMiddle) { + TestExtensionPrefStore eps; + eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); + eps.InstallExtensionPref("id2", kPref1, Value::CreateStringValue("val2")); + eps.InstallExtensionPref("id3", kPref1, Value::CreateStringValue("val3")); + + eps.InstallExtensionPref("id1", kPref2, Value::CreateStringValue("val4")); + eps.InstallExtensionPref("id2", kPref2, Value::CreateStringValue("val5")); + + eps.InstallExtensionPref("id1", kPref3, Value::CreateStringValue("val6")); + + eps.InstallExtensionPref("id2", kPref4, Value::CreateStringValue("val7")); + + eps.UninstallExtension("id2"); + + TestExtensionPrefStore::ExtensionIDs ids; + eps.GetExtensionIDList(&ids); + EXPECT_EQ(2u, ids.size()); + EXPECT_EQ("id3", ids[0]); + EXPECT_EQ("id1", ids[1]); + + DictionaryValue* prefs = eps.prefs(); + ASSERT_EQ(3u, prefs->size()); + std::string actual; + EXPECT_TRUE(prefs->GetString(kPref1, &actual)); + EXPECT_EQ("val3", actual); + EXPECT_TRUE(prefs->GetString(kPref2, &actual)); + EXPECT_EQ("val4", actual); + EXPECT_TRUE(prefs->GetString(kPref3, &actual)); + EXPECT_EQ("val6", actual); + EXPECT_FALSE(prefs->GetString(kPref4, &actual)); +} + +TEST(ExtensionPrefStoreTest, NotifyWhenNeeded) { + TestExtensionPrefStore eps; + + // The PrefValueStore takes ownership of the PrefStores; in this case, that's + // only an ExtensionPrefStore. + PrefValueStore* value_store = new PrefValueStore(NULL, &eps, NULL, NULL); + MockPrefService* pref_service = new MockPrefService(value_store); + eps.SetPrefService(pref_service); + pref_service->RegisterStringPref(kPref1, std::string()); + + eps.InstallExtensionPref("abc", kPref1, + Value::CreateStringValue("https://www.chromium.org")); + EXPECT_TRUE(pref_service->fired_observers_); + eps.InstallExtensionPref("abc", kPref1, + Value::CreateStringValue("https://www.chromium.org")); + EXPECT_FALSE(pref_service->fired_observers_); + eps.InstallExtensionPref("abc", kPref1, + Value::CreateStringValue("chrome://newtab")); + EXPECT_TRUE(pref_service->fired_observers_); + + eps.UninstallExtension("abc"); + EXPECT_TRUE(pref_service->fired_observers_); +} diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc index caeb00b..1d1406f 100644 --- a/chrome/browser/extensions/extensions_service_unittest.cc +++ b/chrome/browser/extensions/extensions_service_unittest.cc @@ -252,12 +252,7 @@ void ExtensionsServiceTestBase::InitializeExtensionsService( ExtensionTestingProfile* profile = new ExtensionTestingProfile(); // Create a preference service that only contains user defined // preference values. - prefs_.reset(new PrefService(new PrefValueStore( - NULL, /* No managed preference values */ - new JsonPrefStore( /* user defined preference values */ - pref_file, - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* No suggested preference values */))); + prefs_.reset(PrefService::CreateUserPrefService(pref_file)); Profile::RegisterUserPrefs(prefs_.get()); browser::RegisterUserPrefs(prefs_.get()); diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc index 97c47e8..8561a4b 100644 --- a/chrome/browser/extensions/test_extension_prefs.cc +++ b/chrome/browser/extensions/test_extension_prefs.cc @@ -42,12 +42,7 @@ void TestExtensionPrefs::RecreateExtensionPrefs() { } // Create a |PrefService| instance that contains only user defined values. - pref_service_.reset(new PrefService(new PrefValueStore( - NULL, /* no managed preference values */ - new JsonPrefStore( /* user defined preferemnce values*/ - preferences_file_, - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* no suggested preference values*/))); + pref_service_.reset(PrefService::CreateUserPrefService(preferences_file_)); ExtensionPrefs::RegisterUserPrefs(pref_service_.get()); prefs_.reset(new ExtensionPrefs(pref_service_.get(), temp_dir_.path())); } diff --git a/chrome/browser/managed_prefs_banner_base_unittest.cc b/chrome/browser/managed_prefs_banner_base_unittest.cc index c3cf4ac..08c80a8 100644 --- a/chrome/browser/managed_prefs_banner_base_unittest.cc +++ b/chrome/browser/managed_prefs_banner_base_unittest.cc @@ -15,16 +15,20 @@ class ManagedPrefsBannerBaseTest : public testing::Test { public: virtual void SetUp() { managed_prefs_ = new DummyPrefStore; + extension_prefs_ = new DummyPrefStore; user_prefs_ = new DummyPrefStore; default_prefs_ = new DummyPrefStore; - pref_service_.reset(new PrefService( - new PrefValueStore(managed_prefs_, user_prefs_, default_prefs_))); + pref_service_.reset(new PrefService(new PrefValueStore(managed_prefs_, + extension_prefs_, + user_prefs_, + default_prefs_))); pref_service_->RegisterStringPref(prefs::kHomePage, "http://google.com"); pref_service_->RegisterBooleanPref(prefs::kHomePageIsNewTabPage, false); } scoped_ptr<PrefService> pref_service_; DummyPrefStore* managed_prefs_; + DummyPrefStore* extension_prefs_; DummyPrefStore* user_prefs_; DummyPrefStore* default_prefs_; }; diff --git a/chrome/browser/metrics/metrics_service_uitest.cc b/chrome/browser/metrics/metrics_service_uitest.cc index 9fb59b2..d705508 100644 --- a/chrome/browser/metrics/metrics_service_uitest.cc +++ b/chrome/browser/metrics/metrics_service_uitest.cc @@ -54,12 +54,7 @@ class MetricsServiceTest : public UITest { FilePath local_state_path = user_data_dir() .Append(chrome::kLocalStateFilename); - return new PrefService(new PrefValueStore( - NULL, /* no managed preferences */ - new JsonPrefStore( /* local user preferences */ - local_state_path, - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* no recommended preferences */)); + return PrefService::CreateUserPrefService(local_state_path); } }; diff --git a/chrome/browser/net/chrome_url_request_context_unittest.cc b/chrome/browser/net/chrome_url_request_context_unittest.cc index 829be5a..f8907b5 100644 --- a/chrome/browser/net/chrome_url_request_context_unittest.cc +++ b/chrome/browser/net/chrome_url_request_context_unittest.cc @@ -159,7 +159,8 @@ TEST(ChromeURLRequestContextTest, CreateProxyConfigTest) { tests[i].description.c_str())); CommandLine command_line(tests[i].command_line); PrefService prefs(new PrefValueStore( - new ConfigurationPolicyPrefStore(&command_line, NULL), NULL, NULL)); + new ConfigurationPolicyPrefStore(&command_line, NULL), + NULL, NULL, NULL)); // No extension, user, or recommended prefs. ChromeURLRequestContextGetter::RegisterUserPrefs(&prefs); scoped_ptr<net::ProxyConfig> config(CreateProxyConfig(&prefs)); diff --git a/chrome/browser/pref_member_unittest.cc b/chrome/browser/pref_member_unittest.cc index dd3fb9cf..2a46c63 100644 --- a/chrome/browser/pref_member_unittest.cc +++ b/chrome/browser/pref_member_unittest.cc @@ -53,7 +53,8 @@ class PrefMemberTestClass : public NotificationObserver { } // anonymous namespace TEST(PrefMemberTest, BasicGetAndSet) { - PrefService prefs(new PrefValueStore(NULL, new DummyPrefStore(), NULL)); + PrefService prefs(new PrefValueStore(NULL, NULL, new DummyPrefStore(), + NULL)); RegisterTestPrefs(&prefs); // Test bool @@ -143,7 +144,8 @@ TEST(PrefMemberTest, BasicGetAndSet) { TEST(PrefMemberTest, TwoPrefs) { // Make sure two RealPrefMembers stay in sync. - PrefService prefs(new PrefValueStore(NULL, new DummyPrefStore(), NULL)); + PrefService prefs(new PrefValueStore(NULL, NULL, new DummyPrefStore(), + NULL)); RegisterTestPrefs(&prefs); RealPrefMember pref1; @@ -163,7 +165,8 @@ TEST(PrefMemberTest, TwoPrefs) { } TEST(PrefMemberTest, Observer) { - PrefService prefs(new PrefValueStore(NULL, new DummyPrefStore(), NULL)); + PrefService prefs(new PrefValueStore(NULL, NULL, new DummyPrefStore(), + NULL)); RegisterTestPrefs(&prefs); PrefMemberTestClass test_obj(&prefs); diff --git a/chrome/browser/pref_service.cc b/chrome/browser/pref_service.cc index c33f373..1450363 100644 --- a/chrome/browser/pref_service.cc +++ b/chrome/browser/pref_service.cc @@ -30,6 +30,7 @@ #include "chrome/browser/dummy_configuration_policy_provider.h" #include "chrome/browser/configuration_policy_pref_store.h" +#include "chrome/browser/extensions/extension_pref_store.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/json_pref_store.h" #include "chrome/common/notification_service.h" @@ -89,8 +90,10 @@ void NotifyReadError(PrefService* pref, int message_id) { } // namespace +// static PrefService* PrefService::CreatePrefService(const FilePath& pref_filename) { PrefStore* managed_prefs = NULL; + ExtensionPrefStore* extension_prefs = new ExtensionPrefStore(NULL); PrefStore* local_prefs = new JsonPrefStore( pref_filename, ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)); @@ -127,11 +130,30 @@ PrefService* PrefService::CreatePrefService(const FilePath& pref_filename) { CommandLine::ForCurrentProcess(), recommended_prefs_provider); - // The PrefValueStore takes to ownership of the parameters. + // The PrefValueStore takes ownership of the PrefStores. PrefValueStore* value_store = new PrefValueStore( managed_prefs, + extension_prefs, local_prefs, recommended_prefs); + + PrefService* pref_service = new PrefService(value_store); + extension_prefs->SetPrefService(pref_service); + + return pref_service; +} + +// static +PrefService* PrefService::CreateUserPrefService( + const FilePath& pref_filename) { + PrefValueStore* value_store = new PrefValueStore( + NULL, /* no enforced prefs */ + NULL, /* no extension prefs */ + new JsonPrefStore( + pref_filename, + ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), + /* user prefs */ + NULL /* no advised prefs */); return new PrefService(value_store); } @@ -376,6 +398,20 @@ const PrefService::Preference* PrefService::FindPreference( return it == prefs_.end() ? NULL : *it; } +void PrefService::FireObserversIfChanged(const wchar_t* path, + const Value* old_value) { + if (PrefIsChanged(path, old_value)) + FireObservers(path); +} + +bool PrefService::PrefIsChanged(const wchar_t* path, + const Value* old_value) { + Value* new_value = NULL; + pref_value_store_->GetValue(path, &new_value); + // Some unit tests have no values for certain prefs. + return (!new_value || !old_value->Equals(new_value)); +} + const DictionaryValue* PrefService::GetDictionary(const wchar_t* path) const { DCHECK(CalledOnValidThread()); @@ -738,14 +774,6 @@ Value* PrefService::GetPrefCopy(const wchar_t* path) { return pref->GetValue()->DeepCopy(); } -void PrefService::FireObserversIfChanged(const wchar_t* path, - const Value* old_value) { - Value* new_value = NULL; - pref_value_store_->GetValue(path, &new_value); - if (!old_value->Equals(new_value)) - FireObservers(path); -} - void PrefService::FireObservers(const wchar_t* path) { DCHECK(CalledOnValidThread()); diff --git a/chrome/browser/pref_service.h b/chrome/browser/pref_service.h index 2cb2fa2f..483624b 100644 --- a/chrome/browser/pref_service.h +++ b/chrome/browser/pref_service.h @@ -69,11 +69,21 @@ class PrefService : public NonThreadSafe { DISALLOW_COPY_AND_ASSIGN(Preference); }; - // Factory method that creates a new instance of a |PrefService|. + // Factory method that creates a new instance of a |PrefService| with + // all platform-applicable PrefStores (managed, extension, user, etc.). + // This is the usual way to create a new PrefService. static PrefService* CreatePrefService(const FilePath& pref_filename); - // The |PrefValueStore| provides preference values. + // Convenience factory method for use in unit tests. Creates a new + // PrefService that uses a PrefValueStore with user preferences at the given + // |pref_filename|, and no other PrefStores (i.e., no other types of + // preferences). + static PrefService* CreateUserPrefService(const FilePath& pref_filename); + + // This constructor is primarily used by tests. The |PrefValueStore| provides + // preference values. explicit PrefService(PrefValueStore* pref_value_store); + ~PrefService(); // Reloads the data from file. This should only be called when the importer @@ -181,8 +191,18 @@ class PrefService : public NonThreadSafe { // preference is not registered. const Preference* FindPreference(const wchar_t* pref_name) const; + // For the given pref_name, fire any observer of the pref only if |old_value| + // is different from the current value. Virtual so it can be mocked for a + // unit test. + virtual void FireObserversIfChanged(const wchar_t* pref_name, + const Value* old_value); + bool read_only() const { return pref_value_store_->ReadOnly(); } + protected: + // This should only be accessed by subclasses for unit-testing. + bool PrefIsChanged(const wchar_t* path, const Value* old_value); + private: // Add a preference to the PreferenceMap. If the pref already exists, return // false. This method takes ownership of |pref|. @@ -195,11 +215,6 @@ class PrefService : public NonThreadSafe { // For the given pref_name, fire any observer of the pref. void FireObservers(const wchar_t* pref_name); - // For the given pref_name, fire any observer of the pref only if |old_value| - // is different from the current value. - void FireObserversIfChanged(const wchar_t* pref_name, - const Value* old_value); - // Load from disk. Returns a non-zero error code on failure. PrefStore::PrefReadError LoadPersistentPrefs(); diff --git a/chrome/browser/pref_service_unittest.cc b/chrome/browser/pref_service_unittest.cc index 72a7ce8..591e00e 100644 --- a/chrome/browser/pref_service_unittest.cc +++ b/chrome/browser/pref_service_unittest.cc @@ -64,7 +64,8 @@ class TestPrefObserver : public NotificationObserver { // TODO(port): port this test to POSIX. #if defined(OS_WIN) TEST(PrefServiceTest, LocalizedPrefs) { - PrefService prefs(new PrefValueStore(NULL, new DummyPrefStore(), NULL)); + PrefService prefs(new PrefValueStore(NULL, NULL, new DummyPrefStore(), + NULL)); const wchar_t kBoolean[] = L"boolean"; const wchar_t kInteger[] = L"integer"; const wchar_t kString[] = L"string"; @@ -87,7 +88,8 @@ TEST(PrefServiceTest, LocalizedPrefs) { #endif TEST(PrefServiceTest, NoObserverFire) { - PrefService prefs(new PrefValueStore(NULL, new DummyPrefStore(), NULL)); + PrefService prefs(new PrefValueStore(NULL, NULL, new DummyPrefStore(), + NULL)); const wchar_t pref_name[] = L"homepage"; prefs.RegisterStringPref(pref_name, ""); @@ -122,7 +124,8 @@ TEST(PrefServiceTest, NoObserverFire) { } TEST(PrefServiceTest, HasPrefPath) { - PrefService prefs(new PrefValueStore(NULL, new DummyPrefStore(), NULL)); + PrefService prefs(new PrefValueStore(NULL, NULL, new DummyPrefStore(), + NULL)); const wchar_t path[] = L"fake.path"; @@ -146,7 +149,7 @@ TEST(PrefServiceTest, Observers) { dict->SetString(pref_name, std::string("http://www.cnn.com")); DummyPrefStore* pref_store = new DummyPrefStore(); pref_store->set_prefs(dict); - PrefService prefs(new PrefValueStore(NULL, pref_store, NULL)); + PrefService prefs(new PrefValueStore(NULL, NULL, pref_store, NULL)); prefs.RegisterStringPref(pref_name, ""); const std::string new_pref_value("http://www.google.com/"); @@ -187,7 +190,7 @@ class PrefServiceSetValueTest : public testing::Test { static const char value_[]; PrefServiceSetValueTest() - : prefs_(new PrefValueStore(NULL, new DummyPrefStore(), NULL)), + : prefs_(new PrefValueStore(NULL, NULL, new DummyPrefStore(), NULL)), name_string_(name_), null_value_(Value::CreateNullValue()) {} diff --git a/chrome/browser/pref_value_store.cc b/chrome/browser/pref_value_store.cc index d408272..6a87abe 100644 --- a/chrome/browser/pref_value_store.cc +++ b/chrome/browser/pref_value_store.cc @@ -5,28 +5,26 @@ #include "chrome/browser/pref_value_store.h" PrefValueStore::PrefValueStore(PrefStore* managed_prefs, + PrefStore* extension_prefs, PrefStore* user_prefs, - PrefStore* recommended_prefs) - : managed_prefs_(managed_prefs), - user_prefs_(user_prefs), - recommended_prefs_(recommended_prefs) { + PrefStore* recommended_prefs) { + pref_stores_[MANAGED].reset(managed_prefs); + pref_stores_[EXTENSION].reset(extension_prefs); + pref_stores_[USER].reset(user_prefs); + pref_stores_[RECOMMENDED].reset(recommended_prefs); } PrefValueStore::~PrefValueStore() { } -bool PrefValueStore::GetValue( - const std::wstring& name, Value** out_value) const { +bool PrefValueStore::GetValue(const std::wstring& name, + Value** out_value) const { // Check the |PrefStore|s in order of their priority from highest to lowest // to find the value of the preference described by the given preference name. - if (managed_prefs_.get() && - managed_prefs_->prefs()->Get(name.c_str(), out_value) ) { - return true; - } else if (user_prefs_.get() && - user_prefs_->prefs()->Get(name.c_str(), out_value) ) { - return true; - } else if (recommended_prefs_.get() && - recommended_prefs_->prefs()->Get(name.c_str(), out_value)) { - return true; + for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) { + if (pref_stores_[i].get() && + pref_stores_[i]->prefs()->Get(name.c_str(), out_value)) { + return true; + } } // No value found for the given preference name, set the return false. *out_value = NULL; @@ -34,49 +32,33 @@ bool PrefValueStore::GetValue( } bool PrefValueStore::WritePrefs() { - // Managed and recommended preferences are not set by the user. - // Hence they will not be written out. - return user_prefs_->WritePrefs(); + bool success = true; + for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) { + if (pref_stores_[i].get()) + success = pref_stores_[i]->WritePrefs() && success; + } + return success; } void PrefValueStore::ScheduleWritePrefs() { - // Managed and recommended preferences are not set by the user. - // Hence they will not be written out. - user_prefs_->ScheduleWritePrefs(); + for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) { + if (pref_stores_[i].get()) + pref_stores_[i]->ScheduleWritePrefs(); + } } PrefStore::PrefReadError PrefValueStore::ReadPrefs() { - PrefStore::PrefReadError managed_pref_error = PrefStore::PREF_READ_ERROR_NONE; - PrefStore::PrefReadError user_pref_error = PrefStore::PREF_READ_ERROR_NONE; - PrefStore::PrefReadError recommended_pref_error = - PrefStore::PREF_READ_ERROR_NONE; - - // Read managed preferences. - if (managed_prefs_.get()) { - managed_pref_error = managed_prefs_->ReadPrefs(); - } - - // Read preferences set by the user. - if (user_prefs_.get()) { - user_pref_error = user_prefs_->ReadPrefs(); - } - - // Read recommended preferences. - if (recommended_prefs_.get()) { - recommended_pref_error = recommended_prefs_->ReadPrefs(); - } - - // TODO(markusheintz): Return a better error status maybe a struct with - // the error status of all PrefStores. - - // Return the first pref store error that occured. - if (managed_pref_error != PrefStore::PREF_READ_ERROR_NONE) { - return managed_pref_error; - } - if (user_pref_error != PrefStore::PREF_READ_ERROR_NONE) { - return user_pref_error; + PrefStore::PrefReadError result = PrefStore::PREF_READ_ERROR_NONE; + for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) { + if (pref_stores_[i].get()) { + PrefStore::PrefReadError this_error = pref_stores_[i]->ReadPrefs(); + if (result == PrefStore::PREF_READ_ERROR_NONE) + result = this_error; + } } - return recommended_pref_error; + // TODO(markusheintz): Return a better error status: maybe a struct with + // the error status of all PrefStores. + return result; } bool PrefValueStore::HasPrefPath(const wchar_t* path) const { @@ -89,28 +71,28 @@ bool PrefValueStore::HasPrefPath(const wchar_t* path) const { // The value of a Preference is managed if the PrefStore for managed // preferences contains a value for the given preference |name|. bool PrefValueStore::PrefValueIsManaged(const wchar_t* name) { - if (managed_prefs_.get() == NULL) { + if (pref_stores_[MANAGED].get() == NULL) { // No managed PreferenceStore set, hence there are no managed // preferences. return false; } Value* tmp_value; - return managed_prefs_->prefs()->Get(name, &tmp_value); + return pref_stores_[MANAGED]->prefs()->Get(name, &tmp_value); } // Note the |DictionaryValue| referenced by the |PrefStore| user_prefs_ -// (returned by the method Prefs()) takes the ownership of the Value referenced +// (returned by the method prefs()) takes the ownership of the Value referenced // by in_value. void PrefValueStore::SetUserPrefValue(const wchar_t* name, Value* in_value) { - user_prefs_->prefs()->Set(name, in_value); + pref_stores_[USER]->prefs()->Set(name, in_value); } bool PrefValueStore::ReadOnly() { - return user_prefs_->ReadOnly(); + return pref_stores_[USER]->ReadOnly(); } void PrefValueStore::RemoveUserPrefValue(const wchar_t* name) { - if (user_prefs_.get()) { - user_prefs_->prefs()->Remove(name, NULL); + if (pref_stores_[USER].get()) { + pref_stores_[USER]->prefs()->Remove(name, NULL); } } diff --git a/chrome/browser/pref_value_store.h b/chrome/browser/pref_value_store.h index a3d92d5..0c4bf56 100644 --- a/chrome/browser/pref_value_store.h +++ b/chrome/browser/pref_value_store.h @@ -28,12 +28,13 @@ class PrefStore; // (like an admin). class PrefValueStore { public: - // |managed_prefs| contains all managed preference values. They have the - // highest priority and precede user-defined preference values. |user_prefs| - // contains all user-defined preference values. User-defined values precede - // recommended values. |recommended_prefs| contains all recommended - // preference values. + // In decreasing order of precedence: + // |managed_prefs| contains all managed (policy) preference values. + // |extension_prefs| contains preference values set by extensions. + // |user_prefs| contains all user-set preference values. + // |recommended_prefs| contains all recommended (policy) preference values. PrefValueStore(PrefStore* managed_prefs, + PrefStore* extension_prefs, PrefStore* user_prefs, PrefStore* recommended_prefs); @@ -44,14 +45,17 @@ class PrefValueStore { bool GetValue(const std::wstring& name, Value** out_value) const; // Read preference values into the three PrefStores so that they are available - // through the GetValue method. + // through the GetValue method. Return the first error that occurs (but + // continue reading the remaining PrefStores). PrefStore::PrefReadError ReadPrefs(); - // Write user settable preference values. Return true if writing values was - // successfull. + // Persists prefs (to disk or elsewhere). Returns true if writing values was + // successful. In practice, only the user prefs are expected to be written + // out. bool WritePrefs(); - // Calls the method ScheduleWritePrefs on the PrefStores. + // Calls the method ScheduleWritePrefs on the PrefStores. In practice, only + // the user prefs are expected to be written out. void ScheduleWritePrefs(); // Returns true if the PrefValueStore contains the given preference. @@ -68,7 +72,8 @@ class PrefValueStore { // set. But GetValue calls will not return this value as long as the // preference is managed. Instead GetValue will return the managed value // of the preference. Note that the PrefValueStore takes the ownership of - // the value referenced by |in_value|. + // the value referenced by |in_value|. It is an error to call this when no + // user PrefStore has been set. void SetUserPrefValue(const wchar_t* name, Value* in_value); // Removes a value from the PrefValueStore. If a preference is managed @@ -81,9 +86,16 @@ class PrefValueStore { bool PrefValueIsManaged(const wchar_t* name); private: - scoped_ptr<PrefStore> managed_prefs_; - scoped_ptr<PrefStore> user_prefs_; - scoped_ptr<PrefStore> recommended_prefs_; + // PrefStores must be listed here in order from highest to lowest priority. + enum PrefStoreType { + MANAGED = 0, + EXTENSION, + USER, + RECOMMENDED, + PREF_STORE_TYPE_MAX = RECOMMENDED + }; + + scoped_ptr<PrefStore> pref_stores_[PREF_STORE_TYPE_MAX + 1]; DISALLOW_COPY_AND_ASSIGN(PrefValueStore); }; diff --git a/chrome/browser/pref_value_store_unittest.cc b/chrome/browser/pref_value_store_unittest.cc index 5fbab74..3d22236 100644 --- a/chrome/browser/pref_value_store_unittest.cc +++ b/chrome/browser/pref_value_store_unittest.cc @@ -16,19 +16,22 @@ using testing::Mock; // Names of the preferences used in this test program. namespace prefs { + const wchar_t kCurrentThemeID[] = L"extensions.theme.id"; const wchar_t kDeleteCache[] = L"browser.clear_data.cache"; - const wchar_t kMaxTabs[] = L"tabs.max_tabs"; + const wchar_t kExtensionPref[] = L"extension.pref"; const wchar_t kHomepage[] = L"homepage"; + const wchar_t kMaxTabs[] = L"tabs.max_tabs"; const wchar_t kMissingPref[] = L"this.pref.does_not_exist"; const wchar_t kRecommendedPref[] = L"this.pref.recommended_value_only"; const wchar_t kSampleDict[] = L"sample.dict"; const wchar_t kSampleList[] = L"sample.list"; } -// Expected values of all preferences used in this test programm. -namespace expected { +// Potentailly expected values of all preferences used in this test program. +namespace user { const int kMaxTabsValue = 31; const bool kDeleteCacheValue = true; + const std::wstring kCurrentThemeIDValue = L"abcdefg"; const std::wstring kHomepageValue = L"http://www.google.com"; } @@ -36,6 +39,11 @@ namespace enforced { const std::wstring kHomepageValue = L"http://www.topeka.com"; } +namespace extension { + const std::wstring kCurrentThemeIDValue = L"set by extension"; + const std::wstring kHomepageValue = L"http://www.chromium.org"; +} + namespace recommended { const int kMaxTabsValue = 10; const bool kRecommendedPrefValue = true; @@ -47,23 +55,28 @@ class PrefValueStoreTest : public testing::Test { // |PrefStore|s are owned by the |PrefValueStore|. DummyPrefStore* enforced_pref_store_; + DummyPrefStore* extension_pref_store_; DummyPrefStore* recommended_pref_store_; DummyPrefStore* user_pref_store_; // Preferences are owned by the individual |DummyPrefStores|. DictionaryValue* enforced_prefs_; + DictionaryValue* extension_prefs_; DictionaryValue* user_prefs_; DictionaryValue* recommended_prefs_; virtual void SetUp() { // Create dummy user preferences. enforced_prefs_= CreateEnforcedPrefs(); + extension_prefs_ = CreateExtensionPrefs(); user_prefs_ = CreateUserPrefs(); recommended_prefs_ = CreateRecommendedPrefs(); // Create |DummyPrefStore|s. enforced_pref_store_ = new DummyPrefStore(); enforced_pref_store_->set_prefs(enforced_prefs_); + extension_pref_store_ = new DummyPrefStore(); + extension_pref_store_->set_prefs(extension_prefs_); user_pref_store_ = new DummyPrefStore(); user_pref_store_->set_read_only(false); user_pref_store_->set_prefs(user_prefs_); @@ -72,6 +85,7 @@ class PrefValueStoreTest : public testing::Test { // Create a new pref-value-store. pref_value_store_.reset(new PrefValueStore(enforced_pref_store_, + extension_pref_store_, user_pref_store_, recommended_pref_store_)); } @@ -80,9 +94,10 @@ class PrefValueStoreTest : public testing::Test { // in it. DictionaryValue* CreateUserPrefs() { DictionaryValue* user_prefs = new DictionaryValue(); - user_prefs->SetBoolean(prefs::kDeleteCache, expected::kDeleteCacheValue); - user_prefs->SetInteger(prefs::kMaxTabs, expected::kMaxTabsValue); - user_prefs->SetString(prefs::kHomepage, expected::kHomepageValue); + user_prefs->SetBoolean(prefs::kDeleteCache, user::kDeleteCacheValue); + user_prefs->SetInteger(prefs::kMaxTabs, user::kMaxTabsValue); + user_prefs->SetString(prefs::kCurrentThemeID, user::kCurrentThemeIDValue); + user_prefs->SetString(prefs::kHomepage, user::kHomepageValue); return user_prefs; } @@ -92,6 +107,14 @@ class PrefValueStoreTest : public testing::Test { return enforced_prefs; } + DictionaryValue* CreateExtensionPrefs() { + DictionaryValue* extension_prefs = new DictionaryValue(); + extension_prefs->SetString(prefs::kCurrentThemeID, + extension::kCurrentThemeIDValue); + extension_prefs->SetString(prefs::kHomepage, extension::kHomepageValue); + return extension_prefs; + } + DictionaryValue* CreateRecommendedPrefs() { DictionaryValue* recommended_prefs = new DictionaryValue(); recommended_prefs->SetInteger(prefs::kMaxTabs, recommended::kMaxTabsValue); @@ -123,6 +146,7 @@ class PrefValueStoreTest : public testing::Test { TEST_F(PrefValueStoreTest, IsReadOnly) { enforced_pref_store_->set_read_only(true); + extension_pref_store_->set_read_only(true); user_pref_store_->set_read_only(true); recommended_pref_store_->set_read_only(true); EXPECT_TRUE(pref_value_store_->ReadOnly()); @@ -134,26 +158,33 @@ TEST_F(PrefValueStoreTest, IsReadOnly) { TEST_F(PrefValueStoreTest, GetValue) { Value* value; - // Test getting an enforced value overwriting a user defined value. + // Test getting an enforced value overwriting a user-defined and + // extension-defined value. value = NULL; ASSERT_TRUE(pref_value_store_->GetValue(prefs::kHomepage, &value)); std::wstring actual_str_value; EXPECT_TRUE(value->GetAsString(&actual_str_value)); EXPECT_EQ(enforced::kHomepageValue, actual_str_value); - // Test getting a user set value. + // Test getting an extension value overwriting a user-defined value. + value = NULL; + ASSERT_TRUE(pref_value_store_->GetValue(prefs::kCurrentThemeID, &value)); + EXPECT_TRUE(value->GetAsString(&actual_str_value)); + EXPECT_EQ(extension::kCurrentThemeIDValue, actual_str_value); + + // Test getting a user-set value. value = NULL; ASSERT_TRUE(pref_value_store_->GetValue(prefs::kDeleteCache, &value)); bool actual_bool_value = false; EXPECT_TRUE(value->GetAsBoolean(&actual_bool_value)); - EXPECT_EQ(expected::kDeleteCacheValue, actual_bool_value); + EXPECT_EQ(user::kDeleteCacheValue, actual_bool_value); // Test getting a user set value overwriting a recommended value. value = NULL; ASSERT_TRUE(pref_value_store_->GetValue(prefs::kMaxTabs, &value)); int actual_int_value = -1; EXPECT_TRUE(value->GetAsInteger(&actual_int_value)); - EXPECT_EQ(expected::kMaxTabsValue, actual_int_value); + EXPECT_EQ(user::kMaxTabsValue, actual_int_value); // Test getting a recommended value. value = NULL; @@ -217,7 +248,7 @@ TEST_F(PrefValueStoreTest, SetUserPrefValue) { pref_value_store_->GetValue(prefs::kMaxTabs, &actual_value); int int_value; EXPECT_TRUE(actual_value->GetAsInteger(&int_value)); - EXPECT_EQ(expected::kMaxTabsValue, int_value); + EXPECT_EQ(user::kMaxTabsValue, int_value); new_value = Value::CreateIntegerValue(1); pref_value_store_->SetUserPrefValue(prefs::kMaxTabs, new_value); diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc index 328cbc2..ba0d171 100644 --- a/chrome/browser/tab_contents/web_contents_unittest.cc +++ b/chrome/browser/tab_contents/web_contents_unittest.cc @@ -54,12 +54,7 @@ class TabContentsTestingProfile : public TestingProfile { // Create a preference service that only contains user defined // preference values. - prefs_.reset(new PrefService(new PrefValueStore( - NULL, /* No managed preference values */ - new JsonPrefStore( /* user defined preference values */ - source_path, - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* No suggested preference values */))); + prefs_.reset(PrefService::CreateUserPrefService(source_path)); Profile::RegisterUserPrefs(prefs_.get()); browser::RegisterAllPrefs(prefs_.get(), prefs_.get()); } diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index e721300..648efde 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1207,6 +1207,8 @@ 'browser/extensions/extension_popup_api.h', 'browser/extensions/extension_prefs.cc', 'browser/extensions/extension_prefs.h', + 'browser/extensions/extension_pref_store.cc', + 'browser/extensions/extension_pref_store.h', 'browser/extensions/extension_process_manager.cc', 'browser/extensions/extension_process_manager.h', 'browser/extensions/extension_processes_api.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 83b7b27..0dce13c 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -782,6 +782,7 @@ 'browser/extensions/extension_menu_manager_unittest.cc', 'browser/extensions/extension_messages_unittest.cc', 'browser/extensions/extension_prefs_unittest.cc', + 'browser/extensions/extension_pref_store_unittest.cc', 'browser/extensions/extension_process_manager_unittest.cc', 'browser/extensions/extension_ui_unittest.cc', 'browser/extensions/extension_updater_unittest.cc', diff --git a/chrome/test/reliability/page_load_test.cc b/chrome/test/reliability/page_load_test.cc index eca9df2..e39d342 100644 --- a/chrome/test/reliability/page_load_test.cc +++ b/chrome/test/reliability/page_load_test.cc @@ -530,12 +530,8 @@ class PageLoadTest : public UITest { FilePath local_state_path = user_data_dir() .Append(chrome::kLocalStateFilename); - PrefService* local_state(new PrefService(new PrefValueStore( - NULL, /* no managed preference values */ - new JsonPrefStore( /* user defined preference values */ - local_state_path, - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* no advised preference values */))); + PrefService* local_state = PrefService::CreateUserPrefService( + local_state_path); return local_state; } diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h index c810a53..67f85a2 100644 --- a/chrome/test/testing_profile.h +++ b/chrome/test/testing_profile.h @@ -154,12 +154,7 @@ class TestingProfile : public Profile { FilePath prefs_filename = path_.Append(FILE_PATH_LITERAL("TestPreferences")); - prefs_.reset(new PrefService(new PrefValueStore( - NULL, /* no managed preference values */ - new JsonPrefStore( /* user defined preference values */ - prefs_filename, - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* no suggested preference values */))); + prefs_.reset(PrefService::CreateUserPrefService(prefs_filename)); Profile::RegisterUserPrefs(prefs_.get()); browser::RegisterAllPrefs(prefs_.get(), prefs_.get()); } diff --git a/chrome_frame/test/reliability/page_load_test.cc b/chrome_frame/test/reliability/page_load_test.cc index badbcd3..d0f8767 100644 --- a/chrome_frame/test/reliability/page_load_test.cc +++ b/chrome_frame/test/reliability/page_load_test.cc @@ -470,12 +470,8 @@ class PageLoadTest : public testing::Test { FilePath local_state_path; chrome::GetChromeFrameUserDataDirectory(&local_state_path); - PrefService* local_state = new PrefService(new PrefValueStore( - NULL, /* no managed preference values */ - new JsonPrefStore(/* user defined preference values */ - local_state_path, - ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)), - NULL /* no sugessted preference values */)); + PrefService* local_state = PrefService::CreateUserPrefService( + local_state_path); return local_state; } |