diff options
author | pam@chromium.org <pam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-12 16:48:49 +0000 |
---|---|---|
committer | pam@chromium.org <pam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-12 16:48:49 +0000 |
commit | db198b2722f4592f6b76294f6a7f9c868e05bc15 (patch) | |
tree | ce5c93732e2d202548b6e2d69ab6a80cc06ca7ea | |
parent | ef40c4da832d2082dffe1262b0e2dbf64b5e8031 (diff) | |
download | chromium_src-db198b2722f4592f6b76294f6a7f9c868e05bc15.zip chromium_src-db198b2722f4592f6b76294f6a7f9c868e05bc15.tar.gz chromium_src-db198b2722f4592f6b76294f6a7f9c868e05bc15.tar.bz2 |
Add an ExtensionPrefStore, layered between the user prefs and the managed prefs, to manage preferences set by extensions.
Update various callers of the PrefValueStore constructor accordingly.
The initial user will be the proxy extension API.
BUG=266
TEST=covered by unit tests
Review URL: http://codereview.chromium.org/2823037
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52088 0039d316-1c4b-4281-b951-d872f2087c98
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; } |