diff options
Diffstat (limited to 'chrome/browser/prefs')
| -rw-r--r-- | chrome/browser/prefs/pref_member_unittest.cc | 1 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_notifier_impl_unittest.cc | 1 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_service.cc | 9 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_service.h | 3 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_service_mock_builder.cc | 8 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_service_mock_builder.h | 1 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_value_map.cc | 48 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_value_map.h | 18 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_value_map_unittest.cc | 76 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_value_store.cc | 150 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_value_store.h | 52 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_value_store_unittest.cc | 224 |
12 files changed, 156 insertions, 435 deletions
diff --git a/chrome/browser/prefs/pref_member_unittest.cc b/chrome/browser/prefs/pref_member_unittest.cc index bf9204e..3f786afb 100644 --- a/chrome/browser/prefs/pref_member_unittest.cc +++ b/chrome/browser/prefs/pref_member_unittest.cc @@ -6,6 +6,7 @@ #include "chrome/browser/prefs/pref_value_store.h" #include "chrome/common/notification_details.h" #include "chrome/common/notification_source.h" +#include "chrome/common/notification_type.h" #include "chrome/test/testing_pref_service.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/chrome/browser/prefs/pref_notifier_impl_unittest.cc b/chrome/browser/prefs/pref_notifier_impl_unittest.cc index 8f3be3c..a410884 100644 --- a/chrome/browser/prefs/pref_notifier_impl_unittest.cc +++ b/chrome/browser/prefs/pref_notifier_impl_unittest.cc @@ -7,6 +7,7 @@ #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/prefs/pref_value_store.h" #include "chrome/common/notification_observer_mock.h" +#include "chrome/common/notification_registrar.h" #include "chrome/common/notification_service.h" #include "chrome/test/testing_pref_service.h" #include "testing/gmock/include/gmock/gmock.h" diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc index 4a3110c..809a8ac 100644 --- a/chrome/browser/prefs/pref_service.cc +++ b/chrome/browser/prefs/pref_service.cc @@ -26,7 +26,6 @@ #include "chrome/browser/prefs/default_pref_store.h" #include "chrome/browser/prefs/pref_notifier_impl.h" #include "chrome/browser/prefs/pref_value_store.h" -#include "chrome/browser/profiles/profile.h" #include "chrome/common/json_pref_store.h" #include "chrome/common/notification_service.h" #include "grit/chromium_strings.h" @@ -117,7 +116,7 @@ PrefService* PrefService::CreatePrefService(const FilePath& pref_filename, ConfigurationPolicyPrefStore::CreateRecommendedPolicyPrefStore(); return new PrefService(managed, device_management, extension_prefs, - command_line, user, recommended, profile); + command_line, user, recommended); } PrefService::PrefService(PrefStore* managed_platform_prefs, @@ -125,8 +124,7 @@ PrefService::PrefService(PrefStore* managed_platform_prefs, PrefStore* extension_prefs, PrefStore* command_line_prefs, PersistentPrefStore* user_prefs, - PrefStore* recommended_prefs, - Profile* profile) + PrefStore* recommended_prefs) : user_pref_store_(user_prefs) { pref_notifier_.reset(new PrefNotifierImpl(this)); default_store_ = new DefaultPrefStore(); @@ -138,8 +136,7 @@ PrefService::PrefService(PrefStore* managed_platform_prefs, user_pref_store_, recommended_prefs, default_store_, - pref_notifier_.get(), - profile); + pref_notifier_.get()); InitFromStorage(); } diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h index 2e0623b..5485292 100644 --- a/chrome/browser/prefs/pref_service.h +++ b/chrome/browser/prefs/pref_service.h @@ -230,8 +230,7 @@ class PrefService : public NonThreadSafe { PrefStore* extension_prefs, PrefStore* command_line_prefs, PersistentPrefStore* user_prefs, - PrefStore* recommended_prefs, - Profile* profile); + PrefStore* recommended_prefs); // The PrefNotifier handles registering and notifying preference observers. // It is created and owned by this PrefService. Subclasses may access it for diff --git a/chrome/browser/prefs/pref_service_mock_builder.cc b/chrome/browser/prefs/pref_service_mock_builder.cc index 3060948..005b700 100644 --- a/chrome/browser/prefs/pref_service_mock_builder.cc +++ b/chrome/browser/prefs/pref_service_mock_builder.cc @@ -12,8 +12,7 @@ #include "chrome/common/json_pref_store.h" PrefServiceMockBuilder::PrefServiceMockBuilder() - : user_prefs_(new TestingPrefStore), - profile_(NULL) { + : user_prefs_(new TestingPrefStore) { } PrefServiceMockBuilder& @@ -98,10 +97,7 @@ PrefService* PrefServiceMockBuilder::Create() { extension_prefs_.release(), command_line_prefs_.release(), user_prefs_.release(), - recommended_prefs_.release(), - profile_); + recommended_prefs_.release()); user_prefs_.reset(new TestingPrefStore); - profile_ = NULL; - return pref_service; } diff --git a/chrome/browser/prefs/pref_service_mock_builder.h b/chrome/browser/prefs/pref_service_mock_builder.h index c17040e..b8e32755 100644 --- a/chrome/browser/prefs/pref_service_mock_builder.h +++ b/chrome/browser/prefs/pref_service_mock_builder.h @@ -61,7 +61,6 @@ class PrefServiceMockBuilder { scoped_ptr<PrefStore> command_line_prefs_; scoped_ptr<PersistentPrefStore> user_prefs_; scoped_ptr<PrefStore> recommended_prefs_; - Profile* profile_; DISALLOW_COPY_AND_ASSIGN(PrefServiceMockBuilder); }; diff --git a/chrome/browser/prefs/pref_value_map.cc b/chrome/browser/prefs/pref_value_map.cc index ff27049..6e7bf79 100644 --- a/chrome/browser/prefs/pref_value_map.cc +++ b/chrome/browser/prefs/pref_value_map.cc @@ -57,3 +57,51 @@ void PrefValueMap::Clear() { STLDeleteValues(&prefs_); prefs_.clear(); } + +bool PrefValueMap::GetBoolean(const std::string& key, + bool* value) const { + Value* stored_value = NULL; + return GetValue(key, &stored_value) && stored_value->GetAsBoolean(value); +} + +bool PrefValueMap::GetString(const std::string& key, + std::string* value) const { + Value* stored_value = NULL; + return GetValue(key, &stored_value) && stored_value->GetAsString(value); +} + +void PrefValueMap::SetString(const std::string& key, + const std::string& value) { + SetValue(key, Value::CreateStringValue(value)); +} + +void PrefValueMap::GetDifferingKeys( + const PrefValueMap* other, + std::vector<std::string>* differing_keys) const { + differing_keys->clear(); + + // Walk over the maps in lockstep, adding everything that is different. + Map::const_iterator this_pref(prefs_.begin()); + Map::const_iterator other_pref(other->prefs_.begin()); + while (this_pref != prefs_.end() && other_pref != other->prefs_.end()) { + const int diff = this_pref->first.compare(other_pref->first); + if (diff == 0) { + if (!this_pref->second->Equals(other_pref->second)) + differing_keys->push_back(this_pref->first); + ++this_pref; + ++other_pref; + } else if (diff < 0) { + differing_keys->push_back(this_pref->first); + ++this_pref; + } else if (diff > 0) { + differing_keys->push_back(other_pref->first); + ++other_pref; + } + } + + // Add the remaining entries. + for ( ; this_pref != prefs_.end(); ++this_pref) + differing_keys->push_back(this_pref->first); + for ( ; other_pref != other->prefs_.end(); ++other_pref) + differing_keys->push_back(other_pref->first); +} diff --git a/chrome/browser/prefs/pref_value_map.h b/chrome/browser/prefs/pref_value_map.h index ddcddf2..0e99920 100644 --- a/chrome/browser/prefs/pref_value_map.h +++ b/chrome/browser/prefs/pref_value_map.h @@ -8,6 +8,7 @@ #include <map> #include <string> +#include <vector> #include "base/basictypes.h" @@ -35,6 +36,23 @@ class PrefValueMap { // Clears the map. void Clear(); + // Gets a boolean value for |key| and stores it in |value|. Returns true if + // the value was found and of the proper type. + bool GetBoolean(const std::string& key, bool* value) const; + + // Gets a string value for |key| and stores it in |value|. Returns true if + // the value was found and of the proper type. + bool GetString(const std::string& key, std::string* value) const; + + // Sets the value for |key| to the string |value|. + void SetString(const std::string& key, const std::string& value); + + // Compares this value map against |other| and stores all key names that have + // different values in |differing_keys|. This includes keys that are present + // only in one of the maps. + void GetDifferingKeys(const PrefValueMap* other, + std::vector<std::string>* differing_keys) const; + private: typedef std::map<std::string, Value*> Map; diff --git a/chrome/browser/prefs/pref_value_map_unittest.cc b/chrome/browser/prefs/pref_value_map_unittest.cc new file mode 100644 index 0000000..5e248b0 --- /dev/null +++ b/chrome/browser/prefs/pref_value_map_unittest.cc @@ -0,0 +1,76 @@ +// 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 "base/values.h" +#include "chrome/browser/prefs/pref_value_map.h" +#include "testing/gtest/include/gtest/gtest.h" + +class PrefValueMapTest : public testing::Test { +}; + +TEST_F(PrefValueMapTest, SetValue) { + PrefValueMap map; + Value* result = NULL; + EXPECT_FALSE(map.GetValue("key", &result)); + EXPECT_FALSE(result); + + EXPECT_TRUE(map.SetValue("key", Value::CreateStringValue("test"))); + EXPECT_FALSE(map.SetValue("key", Value::CreateStringValue("test"))); + EXPECT_TRUE(map.SetValue("key", Value::CreateStringValue("hi mom!"))); + + EXPECT_TRUE(map.GetValue("key", &result)); + EXPECT_TRUE(StringValue("hi mom!").Equals(result)); +} + +TEST_F(PrefValueMapTest, RemoveValue) { + PrefValueMap map; + EXPECT_FALSE(map.RemoveValue("key")); + + EXPECT_TRUE(map.SetValue("key", Value::CreateStringValue("test"))); + EXPECT_TRUE(map.GetValue("key", NULL)); + + EXPECT_TRUE(map.RemoveValue("key")); + EXPECT_FALSE(map.GetValue("key", NULL)); + + EXPECT_FALSE(map.RemoveValue("key")); +} + +TEST_F(PrefValueMapTest, Clear) { + PrefValueMap map; + EXPECT_TRUE(map.SetValue("key", Value::CreateStringValue("test"))); + EXPECT_TRUE(map.GetValue("key", NULL)); + + map.Clear(); + + EXPECT_FALSE(map.GetValue("key", NULL)); +} + +TEST_F(PrefValueMapTest, GetDifferingKeys) { + PrefValueMap reference; + EXPECT_TRUE(reference.SetValue("b", Value::CreateStringValue("test"))); + EXPECT_TRUE(reference.SetValue("c", Value::CreateStringValue("test"))); + EXPECT_TRUE(reference.SetValue("e", Value::CreateStringValue("test"))); + + PrefValueMap check; + std::vector<std::string> differing_paths; + std::vector<std::string> expected_differing_paths; + + reference.GetDifferingKeys(&check, &differing_paths); + expected_differing_paths.push_back("b"); + expected_differing_paths.push_back("c"); + expected_differing_paths.push_back("e"); + EXPECT_EQ(expected_differing_paths, differing_paths); + + EXPECT_TRUE(check.SetValue("a", Value::CreateStringValue("test"))); + EXPECT_TRUE(check.SetValue("c", Value::CreateStringValue("test"))); + EXPECT_TRUE(check.SetValue("d", Value::CreateStringValue("test"))); + + reference.GetDifferingKeys(&check, &differing_paths); + expected_differing_paths.clear(); + expected_differing_paths.push_back("a"); + expected_differing_paths.push_back("b"); + expected_differing_paths.push_back("d"); + expected_differing_paths.push_back("e"); + EXPECT_EQ(expected_differing_paths, differing_paths); +} diff --git a/chrome/browser/prefs/pref_value_store.cc b/chrome/browser/prefs/pref_value_store.cc index 3ad6c44..0fd3449 100644 --- a/chrome/browser/prefs/pref_value_store.cc +++ b/chrome/browser/prefs/pref_value_store.cc @@ -4,10 +4,7 @@ #include "chrome/browser/prefs/pref_value_store.h" -#include "chrome/browser/browser_thread.h" -#include "chrome/browser/policy/configuration_policy_pref_store.h" #include "chrome/browser/prefs/pref_notifier.h" -#include "chrome/common/notification_service.h" PrefValueStore::PrefStoreKeeper::PrefStoreKeeper() : pref_value_store_(NULL), @@ -48,10 +45,8 @@ PrefValueStore::PrefValueStore(PrefStore* managed_platform_prefs, PrefStore* user_prefs, PrefStore* recommended_prefs, PrefStore* default_prefs, - PrefNotifier* pref_notifier, - Profile* profile) - : pref_notifier_(pref_notifier), - profile_(profile) { + PrefNotifier* pref_notifier) + : pref_notifier_(pref_notifier) { InitPrefStore(MANAGED_PLATFORM_STORE, managed_platform_prefs); InitPrefStore(DEVICE_MANAGEMENT_STORE, device_management_prefs); InitPrefStore(EXTENSION_STORE, extension_prefs); @@ -60,11 +55,6 @@ PrefValueStore::PrefValueStore(PrefStore* managed_platform_prefs, InitPrefStore(RECOMMENDED_STORE, recommended_prefs); InitPrefStore(DEFAULT_STORE, default_prefs); - // TODO(mnissler): Remove after policy refresh cleanup. - registrar_.Add(this, - NotificationType(NotificationType::POLICY_CHANGED), - NotificationService::AllSources()); - CheckInitializationCompleted(); } @@ -250,142 +240,6 @@ bool PrefValueStore::GetValueFromStore(const char* name, return false; } -void PrefValueStore::RefreshPolicyPrefsOnFileThread( - BrowserThread::ID calling_thread_id, - policy::ConfigurationPolicyPrefStore* new_managed_platform_pref_store, - policy::ConfigurationPolicyPrefStore* new_device_management_pref_store, - policy::ConfigurationPolicyPrefStore* new_recommended_pref_store) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - scoped_ptr<policy::ConfigurationPolicyPrefStore> managed_platform_pref_store( - new_managed_platform_pref_store); - scoped_ptr<policy::ConfigurationPolicyPrefStore> device_management_pref_store( - new_device_management_pref_store); - scoped_ptr<policy::ConfigurationPolicyPrefStore> recommended_pref_store( - new_recommended_pref_store); - - BrowserThread::PostTask( - calling_thread_id, FROM_HERE, - NewRunnableMethod(this, - &PrefValueStore::RefreshPolicyPrefsCompletion, - managed_platform_pref_store.release(), - device_management_pref_store.release(), - recommended_pref_store.release())); -} - -void PrefValueStore::RefreshPolicyPrefs() { - using policy::ConfigurationPolicyPrefStore; - // Because loading of policy information must happen on the FILE - // thread, it's not possible to just replace the contents of the - // managed and recommended stores in place due to possible - // concurrent access from the UI thread. Instead, new stores are - // created and the refreshed policy read into them. The new stores - // are swapped with the old from a Task on the UI thread after the - // load is complete. - ConfigurationPolicyPrefStore* new_managed_platform_pref_store( - ConfigurationPolicyPrefStore::CreateManagedPlatformPolicyPrefStore()); - ConfigurationPolicyPrefStore* new_device_management_pref_store( - ConfigurationPolicyPrefStore::CreateDeviceManagementPolicyPrefStore( - profile_)); - ConfigurationPolicyPrefStore* new_recommended_pref_store( - ConfigurationPolicyPrefStore::CreateRecommendedPolicyPrefStore()); - BrowserThread::ID current_thread_id; - CHECK(BrowserThread::GetCurrentThreadIdentifier(¤t_thread_id)); - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, - NewRunnableMethod(this, - &PrefValueStore::RefreshPolicyPrefsOnFileThread, - current_thread_id, - new_managed_platform_pref_store, - new_device_management_pref_store, - new_recommended_pref_store)); -} - -void PrefValueStore::RefreshPolicyPrefsCompletion( - policy::ConfigurationPolicyPrefStore* new_managed_platform_pref_store, - policy::ConfigurationPolicyPrefStore* new_device_management_pref_store, - policy::ConfigurationPolicyPrefStore* new_recommended_pref_store) { - // Determine the paths of all the changed preferences values in the three - // policy-related stores (managed platform, device management and - // recommended). - DictionaryValue* managed_platform_prefs_before( - static_cast<policy::ConfigurationPolicyPrefStore*>( - GetPrefStore(MANAGED_PLATFORM_STORE))->prefs()); - DictionaryValue* managed_platform_prefs_after( - new_managed_platform_pref_store->prefs()); - DictionaryValue* device_management_prefs_before( - static_cast<policy::ConfigurationPolicyPrefStore*>( - GetPrefStore(DEVICE_MANAGEMENT_STORE))->prefs()); - DictionaryValue* device_management_prefs_after( - new_device_management_pref_store->prefs()); - DictionaryValue* recommended_prefs_before( - static_cast<policy::ConfigurationPolicyPrefStore*>( - GetPrefStore(RECOMMENDED_STORE))->prefs()); - DictionaryValue* recommended_prefs_after(new_recommended_pref_store->prefs()); - - std::vector<std::string> changed_managed_platform_paths; - managed_platform_prefs_before->GetDifferingPaths( - managed_platform_prefs_after, - &changed_managed_platform_paths); - - std::vector<std::string> changed_device_management_paths; - device_management_prefs_before->GetDifferingPaths( - device_management_prefs_after, - &changed_device_management_paths); - - std::vector<std::string> changed_recommended_paths; - recommended_prefs_before->GetDifferingPaths( - recommended_prefs_after, - &changed_recommended_paths); - - // Merge all three vectors of changed value paths together, filtering - // duplicates in a post-processing step. - std::vector<std::string> all_changed_managed_platform_paths( - changed_managed_platform_paths.size() + - changed_device_management_paths.size()); - - std::vector<std::string>::iterator last_insert = - std::merge(changed_managed_platform_paths.begin(), - changed_managed_platform_paths.end(), - changed_device_management_paths.begin(), - changed_device_management_paths.end(), - all_changed_managed_platform_paths.begin()); - all_changed_managed_platform_paths.resize( - last_insert - all_changed_managed_platform_paths.begin()); - - std::vector<std::string> changed_paths( - all_changed_managed_platform_paths.size() + - changed_recommended_paths.size()); - last_insert = std::merge(all_changed_managed_platform_paths.begin(), - all_changed_managed_platform_paths.end(), - changed_recommended_paths.begin(), - changed_recommended_paths.end(), - changed_paths.begin()); - changed_paths.resize(last_insert - changed_paths.begin()); - - last_insert = unique(changed_paths.begin(), changed_paths.end()); - changed_paths.resize(last_insert - changed_paths.begin()); - - // Replace the old stores with the new and send notification of the changed - // preferences. - InitPrefStore(MANAGED_PLATFORM_STORE, new_managed_platform_pref_store); - InitPrefStore(DEVICE_MANAGEMENT_STORE, new_device_management_pref_store); - InitPrefStore(RECOMMENDED_STORE, new_recommended_pref_store); - - std::vector<std::string>::const_iterator current; - for (current = changed_paths.begin(); - current != changed_paths.end(); - ++current) { - pref_notifier_->OnPreferenceChanged(current->c_str()); - } -} - -void PrefValueStore::Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details) { - if (type == NotificationType::POLICY_CHANGED) - RefreshPolicyPrefs(); -} - void PrefValueStore::OnPrefValueChanged(PrefValueStore::PrefStoreType type, const std::string& key) { NotifyPrefChanged(key.c_str(), type); diff --git a/chrome/browser/prefs/pref_value_store.h b/chrome/browser/prefs/pref_value_store.h index fec16b4..87bf4f7 100644 --- a/chrome/browser/prefs/pref_value_store.h +++ b/chrome/browser/prefs/pref_value_store.h @@ -11,14 +11,11 @@ #include <vector> #include "base/basictypes.h" -#include "base/callback.h" #include "base/gtest_prod_util.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "base/values.h" #include "chrome/browser/browser_thread.h" -#include "chrome/common/notification_observer.h" -#include "chrome/common/notification_registrar.h" #include "chrome/common/pref_store.h" class FilePath; @@ -26,11 +23,6 @@ class PrefNotifier; class PrefStore; class Profile; -// TODO(danno, mnissler): Remove after policy refresh cleanup. -namespace policy { -class ConfigurationPolicyPrefStore; -} - // The PrefValueStore manages various sources of values for Preferences // (e.g., configuration policies, extensions, and user settings). It returns // the value of a Preference from the source with the highest priority, and @@ -38,8 +30,7 @@ class ConfigurationPolicyPrefStore; // // Unless otherwise explicitly noted, all of the methods of this class must // be called on the UI thread. -class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore>, - public NotificationObserver { +class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore> { public: // In decreasing order of precedence: // |managed_platform_prefs| contains all managed platform (non-cloud policy) @@ -62,10 +53,6 @@ class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore>, // the policy pref stores for new ones, so the |profile| pointer needs to be // kept around for then. It is safe to pass a NULL pointer for local state // preferences. - // - // TODO(mnissler, danno): Refactor the pref store interface and refresh logic - // so refreshes can be handled by the pref store itself without swapping - // stores. This way we can get rid of the profile pointer here. PrefValueStore(PrefStore* managed_platform_prefs, PrefStore* device_management_prefs, PrefStore* extension_prefs, @@ -73,8 +60,7 @@ class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore>, PrefStore* user_prefs, PrefStore* recommended_prefs, PrefStore* default_prefs, - PrefNotifier* pref_notifier, - Profile* profile); + PrefNotifier* pref_notifier); virtual ~PrefValueStore(); // Gets the value for the given preference name that has a valid value type; @@ -223,32 +209,6 @@ class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore>, // controlling the pref has changed. void NotifyPrefChanged(const char* path, PrefStoreType new_store); - // Called as a result of a notification of policy change. Triggers a reload of - // managed platform, device management and recommended preferences from policy - // from a Task on the FILE thread. - void RefreshPolicyPrefs(); - - // Called during policy refresh after ReadPrefs completes on the thread - // that initiated the policy refresh. RefreshPolicyPrefsCompletion takes - // ownership of the |callback| object. - void RefreshPolicyPrefsCompletion( - policy::ConfigurationPolicyPrefStore* new_managed_platform_pref_store, - policy::ConfigurationPolicyPrefStore* new_device_management_pref_store, - policy::ConfigurationPolicyPrefStore* new_recommended_pref_store); - - // Called during policy refresh to do the ReadPrefs on the FILE thread. - // RefreshPolicyPrefsOnFileThread takes ownership of the |callback| object. - void RefreshPolicyPrefsOnFileThread( - BrowserThread::ID calling_thread_id, - policy::ConfigurationPolicyPrefStore* new_managed_platform_pref_store, - policy::ConfigurationPolicyPrefStore* new_device_management_pref_store, - policy::ConfigurationPolicyPrefStore* new_recommended_pref_store); - - // NotificationObserver methods: - virtual void Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details); - // Called from the PrefStoreKeeper implementation when a pref value for |key| // changed in the pref store for |type|. void OnPrefValueChanged(PrefStoreType type, const std::string& key); @@ -284,14 +244,6 @@ class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore>, // A mapping of preference names to their registered types. PrefTypeMap pref_types_; - // The associated profile, in case this value store is associated with a - // profile pref service. Used for recreating the device management pref store - // upon policy refresh. - Profile* profile_; - - // TODO(mnissler): Remove this after cleaning up policy refresh handling. - NotificationRegistrar registrar_; - DISALLOW_COPY_AND_ASSIGN(PrefValueStore); }; diff --git a/chrome/browser/prefs/pref_value_store_unittest.cc b/chrome/browser/prefs/pref_value_store_unittest.cc index b78e0fc..5c0fa53 100644 --- a/chrome/browser/prefs/pref_value_store_unittest.cc +++ b/chrome/browser/prefs/pref_value_store_unittest.cc @@ -99,7 +99,7 @@ class PrefValueStoreTest : public testing::Test { CreateRecommendedPrefs(); CreateDefaultPrefs(); - // Create a new pref-value-store. + // Create a fresh PrefValueStore. pref_value_store_ = new PrefValueStore( managed_platform_pref_store_, device_management_pref_store_, @@ -108,8 +108,7 @@ class PrefValueStoreTest : public testing::Test { user_pref_store_, recommended_pref_store_, default_pref_store_, - &pref_notifier_, - NULL); + &pref_notifier_); // Register prefs with the PrefValueStore. pref_value_store_->RegisterPreferenceType(prefs::kApplicationLocale, @@ -619,222 +618,3 @@ TEST_F(PrefValueStoreTest, PrefValueFromDefaultStore) { EXPECT_FALSE( pref_value_store_->PrefValueFromDefaultStore(prefs::kMissingPref)); } - -// TODO(mnissler, danno): Clean this up when refactoring -// ConfigurationPolicyPrefStore. -class PrefValueStorePolicyRefreshTest : public testing::Test { - protected: - // Records preference changes. - class PrefChangeRecorder { - public: - void Record(const std::string& pref_name) { - changed_prefs_.insert(pref_name); - } - - void Clear() { - changed_prefs_.clear(); - } - - const std::set<std::string>& changed_prefs() { return changed_prefs_; } - - private: - std::set<std::string> changed_prefs_; - }; - - virtual void SetUp() { - using policy::ConfigurationPolicyPrefStore; - - ui_thread_.reset(new BrowserThread(BrowserThread::UI, &loop_)), - file_thread_.reset(new BrowserThread(BrowserThread::FILE, &loop_)), - policy_provider_.reset(new policy::DummyConfigurationPolicyProvider( - policy::ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList())); - - ConfigurationPolicyPrefStore* managed_store = - NewConfigurationPolicyPrefStore(); - managed_store->prefs()->SetString(prefs::kHomePage, - managed_platform_pref::kHomepageValue); - expected_differing_paths_.insert(prefs::kHomePage); - - ConfigurationPolicyPrefStore* device_management_store = - NewConfigurationPolicyPrefStore(); - device_management_store->prefs()->SetString( - prefs::kDefaultSearchProviderName, - device_management_pref::kSearchProviderNameValue); - expected_differing_paths_.insert("default_search_provider"); - expected_differing_paths_.insert(prefs::kDefaultSearchProviderName); - - ConfigurationPolicyPrefStore* recommended_store = - NewConfigurationPolicyPrefStore(); - recommended_store->prefs()->SetInteger(prefs::kStabilityLaunchCount, - recommended_pref::kStabilityLaunchCountValue); - recommended_store->prefs()->SetBoolean(prefs::kRecommendedPref, - recommended_pref::kRecommendedPrefValue); - - expected_differing_paths_.insert("this"); - expected_differing_paths_.insert("this.pref"); - expected_differing_paths_.insert(prefs::kRecommendedPref); - expected_differing_paths_.insert("user_experience_metrics"); - expected_differing_paths_.insert("user_experience_metrics.stability"); - expected_differing_paths_.insert(prefs::kStabilityLaunchCount); - - pref_value_store_ = new PrefValueStore( - managed_store, - device_management_store, - NULL, - NULL, - new TestingPrefStore(), - recommended_store, - NULL, - &pref_notifier_, - NULL); - } - - virtual void TearDown() { - loop_.RunAllPending(); - pref_value_store_ = NULL; - file_thread_.reset(); - ui_thread_.reset(); - } - - // Creates a new ConfigurationPolicyPrefStore for testing. - policy::ConfigurationPolicyPrefStore* NewConfigurationPolicyPrefStore() { - return new policy::ConfigurationPolicyPrefStore(policy_provider_.get()); - } - - // A vector of the preferences paths in policy PrefStores that are set at the - // beginning of a test. Can be modified by the test to track changes that it - // makes to the preferences stored in the managed and recommended PrefStores. - std::set<std::string> expected_differing_paths_; - - MessageLoop loop_; - MockPrefNotifier pref_notifier_; - scoped_refptr<PrefValueStore> pref_value_store_; - - private: - scoped_ptr<BrowserThread> ui_thread_; - scoped_ptr<BrowserThread> file_thread_; - - scoped_ptr<policy::DummyConfigurationPolicyProvider> policy_provider_; -}; - -TEST_F(PrefValueStorePolicyRefreshTest, TestPolicyRefresh) { - // pref_value_store_ is initialized by PrefValueStoreTest to have values in - // the managed platform, device management and recommended stores. By - // replacing them with dummy stores, all of the paths of the prefs originally - // in the managed platform, device management and recommended stores should - // change. - pref_value_store_->RefreshPolicyPrefs(); - - PrefChangeRecorder recorder; - EXPECT_CALL(pref_notifier_, OnPreferenceChanged(_)) - .WillRepeatedly(Invoke(&recorder, &PrefChangeRecorder::Record)); - loop_.RunAllPending(); - EXPECT_EQ(expected_differing_paths_, recorder.changed_prefs()); -} - -TEST_F(PrefValueStorePolicyRefreshTest, TestRefreshPolicyPrefsCompletion) { - using policy::ConfigurationPolicyPrefStore; - PrefChangeRecorder recorder; - EXPECT_CALL(pref_notifier_, OnPreferenceChanged(_)) - .WillRepeatedly(Invoke(&recorder, &PrefChangeRecorder::Record)); - - // Test changed preferences in the managed platform store and removed - // preferences in the recommended store. In addition to "homepage", the other - // prefs that are set by default in the test class are removed by the - // DummyStore. - scoped_ptr<ConfigurationPolicyPrefStore> new_managed_platform_store( - NewConfigurationPolicyPrefStore()); - new_managed_platform_store->prefs()->SetString("homepage", - "some other changed homepage"); - - recorder.Clear(); - pref_value_store_->RefreshPolicyPrefsCompletion( - new_managed_platform_store.release(), - NewConfigurationPolicyPrefStore(), - NewConfigurationPolicyPrefStore()); - EXPECT_EQ(expected_differing_paths_, recorder.changed_prefs()); - - // Test properties that have been removed from the managed platform store. - // Homepage is still set in managed prefs. - expected_differing_paths_.clear(); - expected_differing_paths_.insert(prefs::kHomePage); - - recorder.Clear(); - pref_value_store_->RefreshPolicyPrefsCompletion( - NewConfigurationPolicyPrefStore(), - NewConfigurationPolicyPrefStore(), - NewConfigurationPolicyPrefStore()); - EXPECT_EQ(expected_differing_paths_, recorder.changed_prefs()); - - // Test properties that are added to the device management store. - expected_differing_paths_.clear(); - expected_differing_paths_.insert(prefs::kHomePage); - scoped_ptr<ConfigurationPolicyPrefStore> new_device_management_store( - NewConfigurationPolicyPrefStore()); - new_device_management_store->prefs()->SetString( - "homepage", "some other changed homepage"); - - recorder.Clear(); - pref_value_store_->RefreshPolicyPrefsCompletion( - NewConfigurationPolicyPrefStore(), - new_device_management_store.release(), - NewConfigurationPolicyPrefStore()); - EXPECT_EQ(expected_differing_paths_, recorder.changed_prefs()); - - // Test properties that are added to the recommended store. - scoped_ptr<ConfigurationPolicyPrefStore> new_recommended_store( - NewConfigurationPolicyPrefStore()); - new_recommended_store->prefs()->SetString("homepage", - "some other changed homepage 2"); - expected_differing_paths_.clear(); - expected_differing_paths_.insert(prefs::kHomePage); - - recorder.Clear(); - pref_value_store_->RefreshPolicyPrefsCompletion( - NewConfigurationPolicyPrefStore(), - NewConfigurationPolicyPrefStore(), - new_recommended_store.release()); - EXPECT_EQ(expected_differing_paths_, recorder.changed_prefs()); - - // Test adding a multi-key path. - new_managed_platform_store.reset(NewConfigurationPolicyPrefStore()); - new_managed_platform_store->prefs()->SetString("segment1.segment2", "value"); - expected_differing_paths_.clear(); - expected_differing_paths_.insert(prefs::kHomePage); - expected_differing_paths_.insert("segment1"); - expected_differing_paths_.insert("segment1.segment2"); - - recorder.Clear(); - pref_value_store_->RefreshPolicyPrefsCompletion( - new_managed_platform_store.release(), - NewConfigurationPolicyPrefStore(), - NewConfigurationPolicyPrefStore()); - EXPECT_EQ(expected_differing_paths_, recorder.changed_prefs()); -} - -TEST_F(PrefValueStorePolicyRefreshTest, TestConcurrentPolicyRefresh) { - PrefChangeRecorder recorder; - EXPECT_CALL(pref_notifier_, OnPreferenceChanged(_)) - .WillRepeatedly(Invoke(&recorder, &PrefChangeRecorder::Record)); - - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - NewRunnableMethod( - pref_value_store_.get(), - &PrefValueStore::RefreshPolicyPrefs)); - - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - NewRunnableMethod( - pref_value_store_.get(), - &PrefValueStore::RefreshPolicyPrefs)); - - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - NewRunnableMethod( - pref_value_store_.get(), - &PrefValueStore::RefreshPolicyPrefs)); - - loop_.RunAllPending(); - EXPECT_EQ(expected_differing_paths_, recorder.changed_prefs()); -} |