diff options
26 files changed, 750 insertions, 736 deletions
diff --git a/base/values.cc b/base/values.cc index cd0f6a8..c6a377f 100644 --- a/base/values.cc +++ b/base/values.cc @@ -133,6 +133,13 @@ bool Value::Equals(const Value* other) const { return other->IsType(TYPE_NULL); } +// static +bool Value::Equals(const Value* a, const Value* b) { + if ((a == NULL) && (b == NULL)) return true; + if ((a == NULL) ^ (b == NULL)) return false; + return a->Equals(b); +} + Value::Value(ValueType type) : type_(type) { } diff --git a/base/values.h b/base/values.h index 07d4985..ac226fa 100644 --- a/base/values.h +++ b/base/values.h @@ -100,6 +100,10 @@ class Value { // Compares if two Value objects have equal contents. virtual bool Equals(const Value* other) const; + // Compares if two Value objects have equal contents. Can handle NULLs. + // NULLs are considered equal but different from Value::CreateNullValue(). + static bool Equals(const Value* a, const Value* b); + protected: // This isn't safe for end-users (they should use the Create*Value() // static methods above), but it's useful for subclasses. diff --git a/base/values_unittest.cc b/base/values_unittest.cc index 9f34c62..13f0f19 100644 --- a/base/values_unittest.cc +++ b/base/values_unittest.cc @@ -508,6 +508,29 @@ TEST_F(ValuesTest, Equals) { EXPECT_FALSE(dv.Equals(copy.get())); } +TEST_F(ValuesTest, StaticEquals) { + scoped_ptr<Value> null1(Value::CreateNullValue()); + scoped_ptr<Value> null2(Value::CreateNullValue()); + EXPECT_TRUE(Value::Equals(null1.get(), null2.get())); + EXPECT_TRUE(Value::Equals(NULL, NULL)); + + scoped_ptr<Value> i42(Value::CreateIntegerValue(42)); + scoped_ptr<Value> j42(Value::CreateIntegerValue(42)); + scoped_ptr<Value> i17(Value::CreateIntegerValue(17)); + EXPECT_TRUE(Value::Equals(i42.get(), i42.get())); + EXPECT_TRUE(Value::Equals(j42.get(), i42.get())); + EXPECT_TRUE(Value::Equals(i42.get(), j42.get())); + EXPECT_FALSE(Value::Equals(i42.get(), i17.get())); + EXPECT_FALSE(Value::Equals(i42.get(), NULL)); + EXPECT_FALSE(Value::Equals(NULL, i42.get())); + + // NULL and Value::CreateNullValue() are intentionally different: We need + // support for NULL as a return value for "undefined" without caring for + // ownership of the pointer. + EXPECT_FALSE(Value::Equals(null1.get(), NULL)); + EXPECT_FALSE(Value::Equals(NULL, null1.get())); +} + TEST_F(ValuesTest, RemoveEmptyChildren) { scoped_ptr<DictionaryValue> root(new DictionaryValue); // Remove empty lists and dictionaries. diff --git a/chrome/browser/extensions/extension_pref_store.cc b/chrome/browser/extensions/extension_pref_store.cc deleted file mode 100644 index c39d700..0000000 --- a/chrome/browser/extensions/extension_pref_store.cc +++ /dev/null @@ -1,199 +0,0 @@ -// 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/browser_process.h" -#include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/common/extensions/extension.h" -#include "chrome/common/notification_service.h" - -ExtensionPrefStore::ExtensionPrefStore(Profile* profile, - PrefNotifier::PrefStoreType type) - : prefs_(new DictionaryValue()), - profile_(profile), - type_(type) { - RegisterObservers(); -} - -ExtensionPrefStore::~ExtensionPrefStore() { - STLDeleteElements(&extension_stack_); - notification_registrar_.RemoveAll(); -} - -void ExtensionPrefStore::InstallExtensionPref(const Extension* extension, - const char* new_pref_path, - Value* new_pref_value) { - ExtensionStack::iterator i; - for (i = extension_stack_.begin(); i != extension_stack_.end(); ++i) { - if ((*i)->extension == extension) - break; - } - - // If this extension is not already in the stack, add it. Otherwise, update - // or add the value of this preference, but don't change the extension's - // position in the stack. We store the extension even if this preference - // isn't registered with our PrefService, so that the ordering of extensions - // is consistent among all local-state and user ExtensionPrefStores. - PrefService* pref_service = GetPrefService(); - // The pref_service may be NULL in unit testing. - bool is_registered_pref = (pref_service == NULL || - pref_service->FindPreference(new_pref_path) != NULL); - PrefValueMap* pref_values; - if (i == extension_stack_.end()) { - pref_values = new PrefValueMap(); - if (is_registered_pref) - (*pref_values)[new_pref_path] = new_pref_value; - - ExtensionPrefs* extension_prefs = new ExtensionPrefs(extension, - pref_values); - extension_stack_.push_front(extension_prefs); - } else if (is_registered_pref) { - pref_values = (*i)->pref_values; - delete (*pref_values)[new_pref_path]; - (*pref_values)[new_pref_path] = new_pref_value; - } - - // Apply the preference to our local |prefs_| store. - UpdateOnePref(new_pref_path); -} - -void ExtensionPrefStore::UninstallExtension(const Extension* extension) { - // Remove this extension from the stack. - for (ExtensionStack::iterator i = extension_stack_.begin(); - i != extension_stack_.end(); ++i) { - if ((*i)->extension == extension) { - scoped_ptr<ExtensionPrefs> to_be_deleted(*i); - extension_stack_.erase(i); - UpdatePrefs(to_be_deleted->pref_values); - return; - } - } -} - -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()); - } -} - -// This could be sped up by keeping track of which extension currently controls -// a given preference, among other optimizations. But probably 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 char* path) { - PrefService* pref_service = GetPrefService(); - - // There are at least two PrefServices, one for local state and one for - // user prefs. (See browser_main.cc.) Different preferences are registered - // in each; if this one doesn't have the desired pref registered, we ignore - // it and let the other one handle it. - // The pref_service may be NULL in unit testing. - if (pref_service && !pref_service->FindPreference(path)) - return; - - // Save the old value before removing it from the local cache. - Value* my_old_value_ptr = NULL; - prefs_->Get(path, &my_old_value_ptr); - scoped_ptr<Value> my_old_value; - if (my_old_value_ptr) - my_old_value.reset(my_old_value_ptr->DeepCopy()); - - // DictionaryValue::Set complains if a key is overwritten with the same - // value, so remove it first. - prefs_->Remove(path, NULL); - - // Find the first extension that wants to set this pref and use its value. - Value* my_new_value = NULL; - 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()); - my_new_value = (*value_iter).second; - break; - } - } - - if (pref_service) { - bool value_changed = true; - if (!my_old_value.get() && !my_new_value) { - value_changed = false; - } else if (my_old_value.get() && - my_new_value && - my_old_value->Equals(my_new_value)) { - value_changed = false; - } - - if (value_changed) - pref_service->pref_notifier()->OnPreferenceSet(path, type_); - } -} - -void ExtensionPrefStore::UpdatePrefs(const PrefValueMap* pref_values) { - if (!pref_values) - return; - - for (PrefValueMap::const_iterator i = pref_values->begin(); - i != pref_values->end(); ++i) { - UpdateOnePref(i->first); - } -} - -PrefService* ExtensionPrefStore::GetPrefService() { - if (profile_) - return profile_->GetPrefs(); - return g_browser_process->local_state(); -} - -void ExtensionPrefStore::RegisterObservers() { - notification_registrar_.Add(this, - NotificationType::EXTENSION_PREF_CHANGED, - NotificationService::AllSources()); - - notification_registrar_.Add(this, - NotificationType::EXTENSION_UNLOADED, - NotificationService::AllSources()); -} - -void ExtensionPrefStore::Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details) { - switch (type.value) { - case NotificationType::EXTENSION_PREF_CHANGED: { - Profile* extension_profile = Source<Profile>(source).ptr(); - // The ExtensionPrefStore for the local state watches all profiles. - if (!profile_ || profile_ == extension_profile) { - ExtensionPrefStore::ExtensionPrefDetails* data = - Details<ExtensionPrefStore::ExtensionPrefDetails>(details).ptr(); - InstallExtensionPref(data->first, data->second.first, - data->second.second); - } - break; - } - case NotificationType::EXTENSION_UNLOADED: { - Profile* extension_profile = Source<Profile>(source).ptr(); - const Extension* extension = Details<const Extension>(details).ptr(); - // The ExtensionPrefStore for the local state watches all profiles. - if (profile_ == NULL || profile_ == extension_profile) - UninstallExtension(extension); - break; - } - default: { - NOTREACHED(); - } - } -} - -ExtensionPrefStore::ExtensionPrefs::ExtensionPrefs(const Extension* extension, - PrefValueMap* values) : extension(extension), pref_values(values) {} - -ExtensionPrefStore::ExtensionPrefs::~ExtensionPrefs() { - STLDeleteValues(pref_values); - delete pref_values; -} diff --git a/chrome/browser/extensions/extension_pref_store.h b/chrome/browser/extensions/extension_pref_store.h deleted file mode 100644 index f176758..0000000 --- a/chrome/browser/extensions/extension_pref_store.h +++ /dev/null @@ -1,124 +0,0 @@ -// 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_ -#pragma once - -#include <list> -#include <map> -#include <string> -#include <utility> -#include <vector> - -#include "base/basictypes.h" -#include "base/scoped_ptr.h" -#include "base/stl_util-inl.h" -#include "chrome/browser/prefs/pref_notifier.h" -#include "chrome/common/notification_observer.h" -#include "chrome/common/notification_registrar.h" -#include "chrome/common/pref_store.h" - -class DictionaryValue; -class Extension; -class PrefService; -class Profile; -class Value; - -// This PrefStore keeps track of preferences set by extensions: for example, -// proxy settings. A stack of relevant extensions 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 NotificationObserver { - public: - // Maps preference paths to their values. - typedef std::map<const char*, Value*> PrefValueMap; - - // The type passed as Details for an EXTENSION_PREF_CHANGED notification. - // The nested pairs are <extension, <pref_path, pref_value> >. This is here, - // rather than in (say) notification_type.h, to keep the dependency on - // std::pair out of the many places that include notification_type.h. - typedef std::pair<const Extension*, std::pair<const char*, Value*> > - ExtensionPrefDetails; - - ExtensionPrefStore(Profile* profile, PrefNotifier::PrefStoreType type); - virtual ~ExtensionPrefStore(); - - // 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. - // The ExtensionPrefStore will take ownership of the |pref_value|. - virtual void InstallExtensionPref(const Extension* extension, - const char* 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(const Extension* extension); - - // PrefStore methods: - virtual DictionaryValue* prefs() const { 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); - - // Returns the applicable pref service from the profile (if we have one) or - // the browser's local state. This should only be accessed or overridden by - // subclasses for unit-testing. - virtual PrefService* GetPrefService(); - - private: - // Associates an extension with the prefs it sets. Owns the pref values. - struct ExtensionPrefs { - ExtensionPrefs(const Extension* extension, PrefValueMap* values); - ~ExtensionPrefs(); - - const Extension* extension; - 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. - typedef std::list<ExtensionPrefs*> ExtensionStack; - - // Applies the highest-priority extension's setting for the given preference - // path to the |prefs_| store, or clears the setting there if no extensions - // wish to control it. - void UpdateOnePref(const char* path); - - // Updates each preference in the key set of the |pref_values| map. - void UpdatePrefs(const PrefValueMap* pref_values); - - // Registers this as an observer for relevant notifications. - void RegisterObservers(); - - // Responds to observed notifications. - void Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details); - - // 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_; - - ExtensionStack extension_stack_; - - NotificationRegistrar notification_registrar_; - - // Weak reference to the profile whose extensions we're interested in. May be - // NULL (for the local-state preferences), in which case we watch all - // extensions. - Profile* profile_; - - // My PrefStore type, assigned by the PrefValueStore. - PrefNotifier::PrefStoreType type_; - - DISALLOW_COPY_AND_ASSIGN(ExtensionPrefStore); -}; - -#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 deleted file mode 100644 index 4c64546..0000000 --- a/chrome/browser/extensions/extension_pref_store_unittest.cc +++ /dev/null @@ -1,370 +0,0 @@ -// 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 <string> -#include <vector> - -#include "base/scoped_ptr.h" -#include "base/scoped_temp_dir.h" -#include "base/values.h" -#include "chrome/browser/extensions/extension_pref_store.h" -#include "chrome/browser/prefs/in_memory_pref_store.h" -#include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/prefs/pref_value_store.h" -#include "chrome/common/extensions/extension.h" -#include "chrome/test/testing_pref_service.h" -#include "chrome/test/testing_pref_value_store.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace keys = extension_manifest_keys; - -namespace { - -class TestExtensionPrefStore : public ExtensionPrefStore { - public: - TestExtensionPrefStore() - : ExtensionPrefStore(NULL, PrefNotifier::EXTENSION_STORE), - ext1(NULL), - ext2(NULL), - ext3(NULL), - pref_service_(NULL) { - // Can't use ASSERT_TRUE here because a constructor can't return a value. - if (!temp_dir_.CreateUniqueTempDir()) { - ADD_FAILURE() << "Failed to create temp dir"; - return; - } - DictionaryValue simple_dict; - std::string error; - - simple_dict.SetString(keys::kVersion, "1.0.0.0"); - simple_dict.SetString(keys::kName, "unused"); - - ext1_scoped_ = Extension::Create( - temp_dir_.path().AppendASCII("ext1"), Extension::INVALID, - simple_dict, false, &error); - ext2_scoped_ = Extension::Create( - temp_dir_.path().AppendASCII("ext2"), Extension::INVALID, - simple_dict, false, &error); - ext3_scoped_ = Extension::Create( - temp_dir_.path().AppendASCII("ext3"), Extension::INVALID, - simple_dict, false, &error); - - ext1 = ext1_scoped_.get(); - ext2 = ext2_scoped_.get(); - ext3 = ext3_scoped_.get(); - } - - typedef std::vector<std::string> ExtensionIDs; - void GetExtensionIDList(ExtensionIDs* result) { - GetExtensionIDs(result); - } - - void SetPrefService(PrefService* pref_service) { - pref_service_ = pref_service; - } - - // Overridden from ExtensionPrefStore. - virtual PrefService* GetPrefService() { - return pref_service_; - } - - // Weak references, for convenience. - Extension* ext1; - Extension* ext2; - Extension* ext3; - - private: - ScopedTempDir temp_dir_; - - scoped_refptr<Extension> ext1_scoped_; - scoped_refptr<Extension> ext2_scoped_; - scoped_refptr<Extension> ext3_scoped_; - - // Weak reference. - PrefService* pref_service_; -}; - -// Mock PrefNotifier that allows the notifications to be tracked. -class MockPrefNotifier : public PrefNotifier { - public: - MockPrefNotifier(PrefService* service, PrefValueStore* value_store) - : PrefNotifier(service, value_store) {} - - virtual ~MockPrefNotifier() {} - - MOCK_METHOD1(FireObservers, void(const char* path)); -}; - -// Mock PrefService that allows the PrefNotifier to be injected. -class MockPrefService : public PrefService { - public: - explicit MockPrefService(PrefValueStore* pref_value_store) - : PrefService(pref_value_store) { - } - - void SetPrefNotifier(MockPrefNotifier* notifier) { - pref_notifier_.reset(notifier); - } -}; - -// Use constants to avoid confusing std::map with hard-coded strings. -const char kPref1[] = "path1.subpath"; -const char kPref2[] = "path2"; -const char kPref3[] = "path3"; -const char kPref4[] = "path4"; - -} // namespace - -TEST(ExtensionPrefStoreTest, InstallOneExtension) { - TestExtensionPrefStore eps; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - - TestExtensionPrefStore::ExtensionIDs ids; - eps.GetExtensionIDList(&ids); - EXPECT_EQ(1u, ids.size()); - EXPECT_EQ(eps.ext1->id(), 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; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - eps.InstallExtensionPref(eps.ext2, kPref1, Value::CreateStringValue("val2")); - eps.InstallExtensionPref(eps.ext3, kPref1, Value::CreateStringValue("val3")); - - TestExtensionPrefStore::ExtensionIDs ids; - eps.GetExtensionIDList(&ids); - EXPECT_EQ(3u, ids.size()); - EXPECT_EQ(eps.ext3->id(), ids[0]); - EXPECT_EQ(eps.ext2->id(), ids[1]); - EXPECT_EQ(eps.ext1->id(), 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; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - eps.InstallExtensionPref(eps.ext2, kPref1, Value::CreateStringValue("val2")); - eps.InstallExtensionPref(eps.ext3, kPref1, Value::CreateStringValue("val3")); - - eps.InstallExtensionPref(eps.ext1, kPref2, Value::CreateStringValue("val4")); - eps.InstallExtensionPref(eps.ext2, kPref2, Value::CreateStringValue("val5")); - - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val6")); - eps.InstallExtensionPref(eps.ext1, kPref2, Value::CreateStringValue("val7")); - eps.InstallExtensionPref(eps.ext1, kPref3, Value::CreateStringValue("val8")); - - TestExtensionPrefStore::ExtensionIDs ids; - eps.GetExtensionIDList(&ids); - EXPECT_EQ(3u, ids.size()); - EXPECT_EQ(eps.ext3->id(), ids[0]); - EXPECT_EQ(eps.ext2->id(), ids[1]); - EXPECT_EQ(eps.ext1->id(), 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; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - eps.InstallExtensionPref(eps.ext2, kPref2, Value::CreateStringValue("val2")); - eps.InstallExtensionPref(eps.ext3, kPref3, Value::CreateStringValue("val3")); - - eps.InstallExtensionPref(eps.ext3, kPref3, Value::CreateStringValue("val4")); - eps.InstallExtensionPref(eps.ext2, kPref3, Value::CreateStringValue("val5")); - eps.InstallExtensionPref(eps.ext1, kPref3, Value::CreateStringValue("val6")); - - eps.InstallExtensionPref(eps.ext3, kPref1, Value::CreateStringValue("val7")); - - TestExtensionPrefStore::ExtensionIDs ids; - eps.GetExtensionIDList(&ids); - EXPECT_EQ(3u, ids.size()); - EXPECT_EQ(eps.ext3->id(), ids[0]); - EXPECT_EQ(eps.ext2->id(), ids[1]); - EXPECT_EQ(eps.ext1->id(), 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; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - eps.InstallExtensionPref(eps.ext1, kPref2, Value::CreateStringValue("val2")); - - // No need to check the state here; the Install* tests cover that. - eps.UninstallExtension(eps.ext1); - - 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; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - eps.InstallExtensionPref(eps.ext2, kPref1, Value::CreateStringValue("val2")); - - eps.InstallExtensionPref(eps.ext1, kPref2, Value::CreateStringValue("val3")); - eps.InstallExtensionPref(eps.ext2, kPref2, Value::CreateStringValue("val4")); - - eps.UninstallExtension(eps.ext1); - - TestExtensionPrefStore::ExtensionIDs ids; - eps.GetExtensionIDList(&ids); - EXPECT_EQ(1u, ids.size()); - EXPECT_EQ(eps.ext2->id(), 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; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - eps.InstallExtensionPref(eps.ext2, kPref1, Value::CreateStringValue("val2")); - eps.InstallExtensionPref(eps.ext3, kPref1, Value::CreateStringValue("val3")); - - eps.InstallExtensionPref(eps.ext1, kPref2, Value::CreateStringValue("val4")); - eps.InstallExtensionPref(eps.ext3, kPref2, Value::CreateStringValue("val5")); - - eps.UninstallExtension(eps.ext3); - - TestExtensionPrefStore::ExtensionIDs ids; - eps.GetExtensionIDList(&ids); - EXPECT_EQ(2u, ids.size()); - EXPECT_EQ(eps.ext2->id(), ids[0]); - EXPECT_EQ(eps.ext1->id(), 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; - ASSERT_TRUE(eps.ext1 != NULL); - eps.InstallExtensionPref(eps.ext1, kPref1, Value::CreateStringValue("val1")); - eps.InstallExtensionPref(eps.ext2, kPref1, Value::CreateStringValue("val2")); - eps.InstallExtensionPref(eps.ext3, kPref1, Value::CreateStringValue("val3")); - - eps.InstallExtensionPref(eps.ext1, kPref2, Value::CreateStringValue("val4")); - eps.InstallExtensionPref(eps.ext2, kPref2, Value::CreateStringValue("val5")); - - eps.InstallExtensionPref(eps.ext1, kPref3, Value::CreateStringValue("val6")); - - eps.InstallExtensionPref(eps.ext2, kPref4, Value::CreateStringValue("val7")); - - eps.UninstallExtension(eps.ext2); - - TestExtensionPrefStore::ExtensionIDs ids; - eps.GetExtensionIDList(&ids); - EXPECT_EQ(2u, ids.size()); - EXPECT_EQ(eps.ext3->id(), ids[0]); - EXPECT_EQ(eps.ext1->id(), 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) { - using testing::Mock; - - TestExtensionPrefStore* eps = new TestExtensionPrefStore; - InMemoryPrefStore* dps = new InMemoryPrefStore; - ASSERT_TRUE(eps->ext1 != NULL); - - // The PrefValueStore takes ownership of the PrefStores; in this case, that's - // only an ExtensionPrefStore. Likewise, the PrefService takes ownership of - // the PrefValueStore and PrefNotifier. - PrefValueStore* value_store = - new TestingPrefValueStore(NULL, NULL, eps, NULL, NULL, NULL, dps); - scoped_ptr<MockPrefService> pref_service(new MockPrefService(value_store)); - MockPrefNotifier* pref_notifier = new MockPrefNotifier(pref_service.get(), - value_store); - pref_service->SetPrefNotifier(pref_notifier); - - eps->SetPrefService(pref_service.get()); - pref_service->RegisterStringPref(kPref1, std::string()); - - EXPECT_CALL(*pref_notifier, FireObservers(kPref1)); - eps->InstallExtensionPref(eps->ext1, kPref1, - Value::CreateStringValue("https://www.chromium.org")); - Mock::VerifyAndClearExpectations(pref_notifier); - - EXPECT_CALL(*pref_notifier, FireObservers(kPref1)).Times(0); - eps->InstallExtensionPref(eps->ext1, kPref1, - Value::CreateStringValue("https://www.chromium.org")); - Mock::VerifyAndClearExpectations(pref_notifier); - - EXPECT_CALL(*pref_notifier, FireObservers(kPref1)).Times(2); - eps->InstallExtensionPref(eps->ext1, kPref1, - Value::CreateStringValue("chrome://newtab")); - eps->UninstallExtension(eps->ext1); -} diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index d3b5bb7..4ab239d 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -92,6 +92,12 @@ const char kPrefGrantedPermissionsAPI[] = "granted_permissions.api"; const char kPrefGrantedPermissionsHost[] = "granted_permissions.host"; const char kPrefGrantedPermissionsAll[] = "granted_permissions.full"; +// A preference that indicates when an extension was installed. +const char kPrefInstallTime[] = "install_time"; + +// A preference that contains any extension-controlled preferences. +const char kPrefPreferences[] = "preferences"; + } // namespace //////////////////////////////////////////////////////////////////////////////// @@ -139,9 +145,11 @@ ExtensionPrefs::ExtensionPrefs(PrefService* prefs, const FilePath& root_dir) install_directory_(root_dir) { // TODO(asargent) - Remove this in a couple of months. (See comment above // CleanupBadExtensionKeys). - CleanupBadExtensionKeys(prefs); + CleanupBadExtensionKeys(prefs_); MakePathsRelative(); + + InitPrefStore(); } ExtensionPrefs::~ExtensionPrefs() {} @@ -624,7 +632,7 @@ bool ExtensionPrefs::IsExtensionKilled(const std::string& id) { } std::vector<std::string> ExtensionPrefs::GetToolbarOrder() { - std::vector<std::string> extension_ids; + ExtensionPrefs::ExtensionIdSet extension_ids; const ListValue* toolbar_order = prefs_->GetList(kExtensionToolbar); if (toolbar_order) { for (size_t i = 0; i < toolbar_order->GetSize(); ++i) { @@ -651,12 +659,18 @@ void ExtensionPrefs::OnExtensionInstalled( const Extension* extension, Extension::State initial_state, bool initial_incognito_enabled) { const std::string& id = extension->id(); + const base::Time install_time = GetCurrentTime(); UpdateExtensionPref(id, kPrefState, Value::CreateIntegerValue(initial_state)); UpdateExtensionPref(id, kPrefIncognitoEnabled, Value::CreateBooleanValue(initial_incognito_enabled)); UpdateExtensionPref(id, kPrefLocation, Value::CreateIntegerValue(extension->location())); + UpdateExtensionPref(id, kPrefInstallTime, + Value::CreateStringValue( + base::Int64ToString(install_time.ToInternalValue()))); + UpdateExtensionPref(id, kPrefPreferences, new DictionaryValue()); + FilePath::StringType path = MakePathRelative(install_directory_, extension->path(), NULL); UpdateExtensionPref(id, kPrefPath, Value::CreateStringValue(path)); @@ -674,6 +688,9 @@ void ExtensionPrefs::OnExtensionInstalled( void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id, const Extension::Location& location, bool external_uninstall) { + PrefKeySet pref_keys; + GetExtensionControlledPrefKeys(extension_id, &pref_keys); + // For external extensions, we save a preference reminding ourself not to try // and install the extension anymore (except when |external_uninstall| is // true, which signifies that the registry key was deleted or the pref file @@ -685,10 +702,12 @@ void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id, } else { DeleteExtensionPrefs(extension_id); } + + UpdatePrefStore(pref_keys); } Extension::State ExtensionPrefs::GetExtensionState( - const std::string& extension_id) { + const std::string& extension_id) const { DictionaryValue* extension = GetExtensionPref(extension_id); // If the extension doesn't have a pref, it's a --load-extension. @@ -709,6 +728,11 @@ void ExtensionPrefs::SetExtensionState(const Extension* extension, Extension::State state) { UpdateExtensionPref(extension->id(), kPrefState, Value::CreateIntegerValue(state)); + + PrefKeySet pref_keys; + GetExtensionControlledPrefKeys(extension->id(), &pref_keys); + UpdatePrefStore(pref_keys); + SavePrefsAndNotify(); } @@ -812,6 +836,18 @@ DictionaryValue* ExtensionPrefs::GetExtensionPref( return extension; } +DictionaryValue* ExtensionPrefs::GetExtensionControlledPrefs( + const std::string& extension_id) const { + DictionaryValue* extension = GetExtensionPref(extension_id); + if (!extension) { + NOTREACHED(); + return NULL; + } + DictionaryValue* preferences = NULL; + extension->GetDictionary(kPrefPreferences, &preferences); + return preferences; +} + // Helper function for GetInstalledExtensionsInfo. static ExtensionInfo* GetInstalledExtensionInfoImpl( DictionaryValue* extension_data, @@ -1071,6 +1107,184 @@ std::string ExtensionPrefs::GetUpdateUrlData(const std::string& extension_id) { return data; } +base::Time ExtensionPrefs::GetCurrentTime() const { + return base::Time::Now(); +} + +base::Time ExtensionPrefs::GetInstallTime( + const std::string& extension_id) const { + const DictionaryValue* extension = GetExtensionPref(extension_id); + if (!extension) { + NOTREACHED(); + return base::Time::Time(); + } + std::string install_time_str("0"); + extension->GetString(kPrefInstallTime, &install_time_str); + int64 install_time_i64 = 0; + base::StringToInt64(install_time_str, &install_time_i64); + LOG_IF(ERROR, install_time_i64 == 0) + << "Error parsing installation time of an extension."; + return base::Time::FromInternalValue(install_time_i64); +} + +void ExtensionPrefs::GetEnabledExtensions(ExtensionIdSet* out) const { + CHECK(out); + const DictionaryValue* extensions = + pref_service()->GetDictionary(kExtensionsPref); + + for (DictionaryValue::key_iterator ext_id = extensions->begin_keys(); + ext_id != extensions->end_keys(); ++ext_id) { + if (GetExtensionState(*ext_id) != Extension::ENABLED) + continue; + out->push_back(*ext_id); + } +} + +void ExtensionPrefs::FixMissingPrefs(const ExtensionIdSet& extension_ids) { + // Fix old entries that did not get an installation time entry when they + // were installed or don't have a preferences field. + bool persist_required = false; + for (ExtensionIdSet::const_iterator ext_id = extension_ids.begin(); + ext_id != extension_ids.end(); ++ext_id) { + DictionaryValue* extension = GetExtensionPref(*ext_id); + CHECK(extension); + + if (GetInstallTime(*ext_id) == base::Time::Time()) { + const base::Time install_time = GetCurrentTime(); + extension->Set(kPrefInstallTime, + Value::CreateStringValue( + base::Int64ToString(install_time.ToInternalValue()))); + persist_required = true; + } + } + if (persist_required) + SavePrefsAndNotify(); +} + +void ExtensionPrefs::InitPrefStore() { + // When this is called, the PrefService is initialized and provides access + // to the user preferences stored in a JSON file. + ExtensionIdSet extension_ids; + GetEnabledExtensions(&extension_ids); + FixMissingPrefs(extension_ids); + + // Collect the unique extension controlled preference keys of all extensions. + PrefKeySet ext_controlled_prefs; + for (ExtensionIdSet::iterator ext_id = extension_ids.begin(); + ext_id != extension_ids.end(); ++ext_id) { + GetExtensionControlledPrefKeys(*ext_id, &ext_controlled_prefs); + } + + // Store winning preference for each extension controlled preference. + UpdatePrefStore(ext_controlled_prefs); +} + +const Value* ExtensionPrefs::GetWinningExtensionControlledPrefValue( + const std::string& key) const { + Value *winner = NULL; + base::Time winners_install_time = base::Time::Time(); + + ExtensionIdSet extension_ids; + GetEnabledExtensions(&extension_ids); + for (ExtensionIdSet::iterator ext_id = extension_ids.begin(); + ext_id != extension_ids.end(); ++ext_id) { + base::Time extension_install_time = GetInstallTime(*ext_id); + + // We do not need to consider extensions that were installed before the + // most recent extension found that provides the requested preference. + if (extension_install_time < winners_install_time) + continue; + + DictionaryValue* preferences = GetExtensionControlledPrefs(*ext_id); + Value *value = NULL; + if (preferences && preferences->GetWithoutPathExpansion(key, &value)) { + // This extension is more recent than the last one providing this pref. + winner = value; + winners_install_time = extension_install_time; + } + } + + return winner; +} + +void ExtensionPrefs::UpdatePrefStore( + const ExtensionPrefs::PrefKeySet& pref_keys) { + for (PrefKeySet::const_iterator i = pref_keys.begin(); + i != pref_keys.end(); ++i) { + UpdatePrefStore(*i); + } +} + +void ExtensionPrefs::UpdatePrefStore(const std::string& pref_key) { + PrefStore* extension_pref_store = + pref_service()->GetExtensionPrefStore(); + if (extension_pref_store == NULL) + return; // Profile is being shut down, Pref Service is already gone. + const Value* winning_pref_value = + GetWinningExtensionControlledPrefValue(pref_key); + Value* old_value = NULL; + extension_pref_store->prefs()->Get(pref_key, &old_value); + bool changed = !Value::Equals(winning_pref_value, old_value); + + if (winning_pref_value) { + extension_pref_store->prefs()->Set(pref_key, + winning_pref_value->DeepCopy()); + } else { + extension_pref_store->prefs()->Remove(pref_key, NULL); + } + + if (changed) { + pref_service()->pref_notifier()->OnPreferenceSet( + pref_key.c_str(), PrefNotifier::EXTENSION_STORE); + } +} + +void ExtensionPrefs::SetExtensionControlledPref(const std::string& extension_id, + const std::string& pref_key, + Value* value) { + DCHECK(pref_service()->FindPreference(pref_key.c_str())) + << "Extension controlled preference key " << pref_key + << " not registered."; + DictionaryValue* extension_preferences = + GetExtensionControlledPrefs(extension_id); + + if (extension_preferences == NULL) { // May be pruned when writing to disk. + DictionaryValue* extension = GetExtensionPref(extension_id); + if (extension == NULL) { + LOG(ERROR) << "Extension preference for " << extension_id << " undefined"; + return; + } + extension_preferences = new DictionaryValue; + extension->Set(kPrefPreferences, extension_preferences); + } + + Value* oldValue = NULL; + extension_preferences->GetWithoutPathExpansion(pref_key, &oldValue); + bool modified = !Value::Equals(oldValue, value); + if (!modified) + return; + + if (value == NULL) + extension_preferences->RemoveWithoutPathExpansion(pref_key, NULL); + else + extension_preferences->SetWithoutPathExpansion(pref_key, value); + pref_service()->ScheduleSavePersistentPrefs(); + + UpdatePrefStore(pref_key); +} + +void ExtensionPrefs::GetExtensionControlledPrefKeys( + const std::string& extension_id, PrefKeySet *out) const { + DCHECK(out != NULL); + DictionaryValue* ext_prefs = GetExtensionControlledPrefs(extension_id); + if (ext_prefs) { + for (DictionaryValue::key_iterator i = ext_prefs->begin_keys(); + i != ext_prefs->end_keys(); ++i) { + out->insert(*i); + } + } +} + // static void ExtensionPrefs::RegisterUserPrefs(PrefService* prefs) { prefs->RegisterDictionaryPref(kExtensionsPref); diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index 90f1a7f..b0f03f5 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -17,8 +17,20 @@ #include "googleurl/src/gurl.h" // Class for managing global and per-extension preferences. -// This class is instantiated by ExtensionsService, so it should be accessed -// from there. +// +// This class distinguishes the following kinds of preferences: +// - global preferences: +// internal state for the extension system in general, not associated +// with an individual extension, such as lastUpdateTime. +// - per-extension preferences: +// meta-preferences describing properties of the extension like +// installation time, whether the extension is enabled, etc. +// - extension controlled preferences: +// browser preferences that an extension controls. For example, an +// extension could use the proxy API to specify the browser's proxy +// preference. Extension-controlled preferences are stored in +// PrefValueStore::extension_prefs(), which this class populates and +// maintains as the underlying extensions change. class ExtensionPrefs { public: // Key name for a preference that keeps track of per-extension settings. This @@ -28,6 +40,12 @@ class ExtensionPrefs { typedef std::vector<linked_ptr<ExtensionInfo> > ExtensionsInfo; + // Vector containing identifiers for preferences. + typedef std::set<std::string> PrefKeySet; + + // Vector containing identifiers for extensions. + typedef std::vector<std::string> ExtensionIdSet; + // This enum is used for the launch type the user wants to use for an // application. // Do not remove items or re-order this enum as it is used in preferences @@ -39,7 +57,7 @@ class ExtensionPrefs { LAUNCH_WINDOW }; - explicit ExtensionPrefs(PrefService* prefs, const FilePath& root_dir_); + explicit ExtensionPrefs(PrefService* prefs, const FilePath& root_dir); ~ExtensionPrefs(); // Returns a copy of the Extensions prefs. @@ -75,11 +93,14 @@ class ExtensionPrefs { bool external_uninstall); // Returns the state (enabled/disabled) of the given extension. - Extension::State GetExtensionState(const std::string& extension_id); + Extension::State GetExtensionState(const std::string& extension_id) const; // Called to change the extension's state when it is enabled/disabled. void SetExtensionState(const Extension* extension, Extension::State); + // Returns all installed and enabled extensions + void GetEnabledExtensions(ExtensionIdSet* out) const; + // Getter and setter for browser action visibility. bool GetBrowserActionVisibility(const Extension* extension); void SetBrowserActionVisibility(const Extension* extension, bool visible); @@ -233,11 +254,23 @@ class ExtensionPrefs { const std::string& data); std::string GetUpdateUrlData(const std::string& extension_id); + // Sets a preference value that is controlled by the extension. In other + // words, this is not a pref value *about* the extension but something + // global the extension wants to override. + void SetExtensionControlledPref(const std::string& extension_id, + const std::string& pref_key, + Value* value); + static void RegisterUserPrefs(PrefService* prefs); // The underlying PrefService. PrefService* pref_service() const { return prefs_; } + protected: + // For unit testing. Enables injecting an artificial clock that is used + // to query the current time, when an extension is installed. + virtual base::Time GetCurrentTime() const; + private: // Converts absolute paths in the pref to paths relative to the // install_directory_. @@ -297,6 +330,11 @@ class ExtensionPrefs { // Same as above, but returns NULL if it doesn't exist. DictionaryValue* GetExtensionPref(const std::string& id) const; + // Returns the dictionary of preferences controlled by the specified extension + // or NULL if unknown. All entries in the dictionary contain non-expanded + // paths. + DictionaryValue* GetExtensionControlledPrefs(const std::string& id) const; + // Serializes the data and schedules a persistent save via the |PrefService|. // Additionally fires a PREF_CHANGED notification with the top-level // |kExtensionsPref| path set. @@ -314,6 +352,35 @@ class ExtensionPrefs { base::Time LastPingDayImpl(const DictionaryValue* dictionary) const; void SetLastPingDayImpl(const base::Time& time, DictionaryValue* dictionary); + // Helper method to acquire the installation time of an extension. + base::Time GetInstallTime(const std::string& extension_id) const; + + // Fix missing preference entries in the extensions that are were introduced + // in a later Chrome version. + void FixMissingPrefs(const ExtensionIdSet& extension_ids); + + // Installs the persistent extension preferences into |prefs_|'s extension + // pref store. + void InitPrefStore(); + + // Returns the extension controlled preference value of the extension that was + // installed most recently. + const Value* GetWinningExtensionControlledPrefValue( + const std::string& key) const; + + // Executes UpdatePrefStore for all |pref_keys|. + void UpdatePrefStore(const PrefKeySet& pref_keys); + + // Finds the most recently installed extension that defines a preference + // for |pref_key|, then stores its value in the PrefValueStore's extension + // pref store and sends notifications to observers in case the value changed. + void UpdatePrefStore(const std::string& pref_key); + + // Retrieves a list of preference keys that the specified extension + // intends to manage. Keys are always appended, |out| is not cleared. + void GetExtensionControlledPrefKeys(const std::string& extension_id, + PrefKeySet *out) const; + // The pref service specific to this set of extension prefs. PrefService* prefs_; diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc index 84b2224..a670692 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -11,14 +11,29 @@ #include "chrome/browser/browser_thread.h" #include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/test_extension_prefs.h" +#include "chrome/browser/prefs/pref_change_registrar.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/extension_constants.h" +#include "chrome/common/notification_details.h" +#include "chrome/common/notification_observer_mock.h" +#include "chrome/common/notification_source.h" #include "testing/gtest/include/gtest/gtest.h" using base::Time; using base::TimeDelta; +const char kPref1[] = "path1.subpath"; +const char kPref2[] = "path2"; +const char kPref3[] = "path3"; +const char kPref4[] = "path4"; + +// Default values in case an extension pref value is not overridden. +const char kDefaultPref1[] = "default pref 1"; +const char kDefaultPref2[] = "default pref 2"; +const char kDefaultPref3[] = "default pref 3"; +const char kDefaultPref4[] = "default pref 4"; + static void AddPattern(ExtensionExtent* extent, const std::string& pattern) { int schemes = URLPattern::SCHEME_ALL; extent->AddPattern(URLPattern(schemes, pattern)); @@ -55,7 +70,11 @@ class ExtensionPrefsTest : public testing::Test { // things don't break after any ExtensionPrefs startup work. virtual void Verify() = 0; + // This function is called to Register preference default values. + virtual void RegisterPreferences() {} + virtual void SetUp() { + RegisterPreferences(); Initialize(); } @@ -64,6 +83,7 @@ class ExtensionPrefsTest : public testing::Test { // Reset ExtensionPrefs, and re-verify. prefs_.RecreateExtensionPrefs(); + RegisterPreferences(); Verify(); } @@ -525,3 +545,314 @@ class ExtensionPrefsAppLaunchIndex : public ExtensionPrefsTest { scoped_refptr<Extension> extension_; }; TEST_F(ExtensionPrefsAppLaunchIndex, ExtensionPrefsAppLaunchIndex) {} + +namespace keys = extension_manifest_keys; + +class ExtensionPrefsPreferencesBase : public ExtensionPrefsTest { + public: + ExtensionPrefsPreferencesBase() + : ExtensionPrefsTest(), + ext1_(NULL), + ext2_(NULL), + ext3_(NULL), + installed() { + DictionaryValue simple_dict; + std::string error; + + simple_dict.SetString(keys::kVersion, "1.0.0.0"); + simple_dict.SetString(keys::kName, "unused"); + + ext1_scoped_ = Extension::Create( + prefs_.temp_dir().AppendASCII("ext1_"), Extension::INVALID, + simple_dict, false, &error); + ext2_scoped_ = Extension::Create( + prefs_.temp_dir().AppendASCII("ext2_"), Extension::INVALID, + simple_dict, false, &error); + ext3_scoped_ = Extension::Create( + prefs_.temp_dir().AppendASCII("ext3_"), Extension::INVALID, + simple_dict, false, &error); + + ext1_ = ext1_scoped_.get(); + ext2_ = ext2_scoped_.get(); + ext3_ = ext3_scoped_.get(); + + for (size_t i = 0; i < arraysize(installed); ++i) + installed[i] = false; + } + + void RegisterPreferences() { + prefs()->pref_service()->RegisterStringPref(kPref1, kDefaultPref1); + prefs()->pref_service()->RegisterStringPref(kPref2, kDefaultPref2); + prefs()->pref_service()->RegisterStringPref(kPref3, kDefaultPref3); + prefs()->pref_service()->RegisterStringPref(kPref4, kDefaultPref4); + } + + void InstallExtControlledPref(Extension *ext, + const std::string& key, + Value* val) { + // Install extension the first time a preference is set for it. + Extension* extensions[] = {ext1_, ext2_, ext3_}; + for (int i = 0; i < 3; ++i) { + if (ext == extensions[i] && !installed[i]) { + prefs()->OnExtensionInstalled(ext, Extension::ENABLED, true); + installed[i] = true; + break; + } + } + + prefs()->SetExtensionControlledPref(ext->id(), key, val); + } + + void UninstallExtension(const std::string& extension_id) { + prefs()->OnExtensionUninstalled(extension_id, Extension::INTERNAL, false); + } + + // Weak references, for convenience. + Extension* ext1_; + Extension* ext2_; + Extension* ext3_; + + // Flags indicating whether each of the extensions has been installed, yet. + bool installed[3]; + + private: + scoped_refptr<Extension> ext1_scoped_; + scoped_refptr<Extension> ext2_scoped_; + scoped_refptr<Extension> ext3_scoped_; +}; + +class ExtensionPrefsInstallOneExtension + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + } + virtual void Verify() { + std::string actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val1", actual); + } +}; +TEST_F(ExtensionPrefsInstallOneExtension, ExtensionPrefsInstallOneExtension) {} + +// Make sure the last-installed extension wins for each preference. +class ExtensionPrefsInstallOverwrittenExtensions + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + InstallExtControlledPref(ext2_, kPref1, Value::CreateStringValue("val2")); + InstallExtControlledPref(ext3_, kPref1, Value::CreateStringValue("val3")); + + InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val4")); + InstallExtControlledPref(ext2_, kPref2, Value::CreateStringValue("val5")); + + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val6")); + InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val7")); + InstallExtControlledPref(ext1_, kPref3, Value::CreateStringValue("val8")); + } + virtual void Verify() { + std::string actual; + actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val3", actual); + actual = prefs()->pref_service()->GetString(kPref2); + EXPECT_EQ("val5", actual); + actual = prefs()->pref_service()->GetString(kPref3); + EXPECT_EQ("val8", actual); + } +}; +TEST_F(ExtensionPrefsInstallOverwrittenExtensions, + ExtensionPrefsInstallOverwrittenExtensions) {} + +// Make sure the last-installed extension wins even if other extensions set +// the same or different preferences later. +class ExtensionPrefsInstallInterleavedExtensions + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + InstallExtControlledPref(ext2_, kPref2, Value::CreateStringValue("val2")); + InstallExtControlledPref(ext3_, kPref3, Value::CreateStringValue("val3")); + + InstallExtControlledPref(ext3_, kPref3, Value::CreateStringValue("val4")); + InstallExtControlledPref(ext2_, kPref3, Value::CreateStringValue("val5")); + InstallExtControlledPref(ext1_, kPref3, Value::CreateStringValue("val6")); + + InstallExtControlledPref(ext3_, kPref1, Value::CreateStringValue("val7")); + } + virtual void Verify() { + std::string actual; + actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val7", actual); + actual = prefs()->pref_service()->GetString(kPref2); + EXPECT_EQ("val2", actual); + actual = prefs()->pref_service()->GetString(kPref3); + EXPECT_EQ("val4", actual); + } +}; +TEST_F(ExtensionPrefsInstallInterleavedExtensions, + ExtensionPrefsInstallInterleavedExtensions) {} + +class ExtensionPrefsUninstallOnlyExtension + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val2")); + + UninstallExtension(ext1_->id()); + } + virtual void Verify() { + std::string actual; + actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ(kDefaultPref1, actual); + actual = prefs()->pref_service()->GetString(kPref2); + EXPECT_EQ(kDefaultPref2, actual); + } +}; +TEST_F(ExtensionPrefsUninstallOnlyExtension, + ExtensionPrefsUninstallOnlyExtension) {} + +// Tests uninstalling an extension that wasn't winning for any preferences. +class ExtensionPrefsUninstallIrrelevantExtension + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + InstallExtControlledPref(ext2_, kPref1, Value::CreateStringValue("val2")); + + InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val3")); + InstallExtControlledPref(ext2_, kPref2, Value::CreateStringValue("val4")); + + UninstallExtension(ext1_->id()); + } + virtual void Verify() { + std::string actual; + actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val2", actual); + actual = prefs()->pref_service()->GetString(kPref2); + EXPECT_EQ("val4", actual); + } +}; +TEST_F(ExtensionPrefsUninstallIrrelevantExtension, + ExtensionPrefsUninstallIrrelevantExtension) {} + +// Tests uninstalling an extension that was winning for all preferences. +class ExtensionPrefsUninstallExtensionFromTop + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + InstallExtControlledPref(ext2_, kPref1, Value::CreateStringValue("val2")); + InstallExtControlledPref(ext3_, kPref1, Value::CreateStringValue("val3")); + + InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val4")); + InstallExtControlledPref(ext3_, kPref2, Value::CreateStringValue("val5")); + + UninstallExtension(ext3_->id()); + } + virtual void Verify() { + std::string actual; + actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val2", actual); + actual = prefs()->pref_service()->GetString(kPref2); + EXPECT_EQ("val4", actual); + } +}; +TEST_F(ExtensionPrefsUninstallExtensionFromTop, + ExtensionPrefsUninstallExtensionFromTop) {} + +// Tests uninstalling an extension that was winning for only some preferences. +class ExtensionPrefsUninstallExtensionFromMiddle + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + InstallExtControlledPref(ext2_, kPref1, Value::CreateStringValue("val2")); + InstallExtControlledPref(ext3_, kPref1, Value::CreateStringValue("val3")); + + InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val4")); + InstallExtControlledPref(ext2_, kPref2, Value::CreateStringValue("val5")); + + InstallExtControlledPref(ext1_, kPref3, Value::CreateStringValue("val6")); + + InstallExtControlledPref(ext2_, kPref4, Value::CreateStringValue("val7")); + + UninstallExtension(ext2_->id()); + } + virtual void Verify() { + std::string actual; + actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val3", actual); + actual = prefs()->pref_service()->GetString(kPref2); + EXPECT_EQ("val4", actual); + actual = prefs()->pref_service()->GetString(kPref3); + EXPECT_EQ("val6", actual); + actual = prefs()->pref_service()->GetString(kPref4); + EXPECT_EQ(kDefaultPref4, actual); + } +}; +TEST_F(ExtensionPrefsUninstallExtensionFromMiddle, + ExtensionPrefsUninstallExtensionFromMiddle) {} + +// Tests triggering of notifications to registered observers +class ExtensionPrefsNotifyWhenNeeded + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + using testing::_; + using testing::Mock; + using testing::StrEq; + + scoped_ptr<NotificationObserverMock> observer( + new NotificationObserverMock()); + PrefChangeRegistrar registrar; + registrar.Init(prefs()->pref_service()); + registrar.Add(kPref1, observer.get()); + + EXPECT_CALL(*observer, Observe(_, _, _)); + InstallExtControlledPref(ext1_, kPref1, + Value::CreateStringValue("https://www.chromium.org")); + Mock::VerifyAndClearExpectations(observer.get()); + + EXPECT_CALL(*observer, Observe(_, _, _)).Times(0); + InstallExtControlledPref(ext1_, kPref1, + Value::CreateStringValue("https://www.chromium.org")); + Mock::VerifyAndClearExpectations(observer.get()); + + EXPECT_CALL(*observer, Observe(_, _, _)).Times(2); + InstallExtControlledPref(ext1_, kPref1, + Value::CreateStringValue("chrome://newtab")); + + UninstallExtension(ext1_->id()); + registrar.Remove(kPref1, observer.get()); + } + virtual void Verify() { + std::string actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ(kDefaultPref1, actual); + } +}; +TEST_F(ExtensionPrefsNotifyWhenNeeded, + ExtensionPrefsNotifyWhenNeeded) {} + +// Tests disabling an extension +class ExtensionPrefsDisableExt + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + std::string actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val1", actual); + prefs()->SetExtensionState(ext1_, Extension::DISABLED); + } + virtual void Verify() { + std::string actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ(kDefaultPref1, actual); + } +}; +TEST_F(ExtensionPrefsDisableExt, ExtensionPrefsDisableExt) {} + +// Tests disabling and reenabling an extension +class ExtensionPrefsReenableExt + : public ExtensionPrefsPreferencesBase { + virtual void Initialize() { + InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1")); + prefs()->SetExtensionState(ext1_, Extension::DISABLED); + prefs()->SetExtensionState(ext1_, Extension::ENABLED); + } + virtual void Verify() { + std::string actual = prefs()->pref_service()->GetString(kPref1); + EXPECT_EQ("val1", actual); + } +}; +TEST_F(ExtensionPrefsDisableExt, ExtensionPrefsReenableExt) {} diff --git a/chrome/browser/extensions/extension_proxy_api.cc b/chrome/browser/extensions/extension_proxy_api.cc index 030b711..cb9d363 100644 --- a/chrome/browser/extensions/extension_proxy_api.cc +++ b/chrome/browser/extensions/extension_proxy_api.cc @@ -7,8 +7,8 @@ #include "base/string_util.h" #include "base/stringprintf.h" #include "base/values.h" -#include "chrome/browser/extensions/extension_pref_store.h" -#include "chrome/common/notification_service.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/extensions/extensions_service.h" #include "chrome/common/pref_names.h" namespace { @@ -159,11 +159,6 @@ bool UseCustomProxySettingsFunction::ApplyProxyRules( void UseCustomProxySettingsFunction::SendNotification(const char* pref_path, Value* pref_value) { - ExtensionPrefStore::ExtensionPrefDetails details = - std::make_pair(GetExtension(), std::make_pair(pref_path, pref_value)); - - NotificationService::current()->Notify( - NotificationType::EXTENSION_PREF_CHANGED, - Source<Profile>(profile_), - Details<ExtensionPrefStore::ExtensionPrefDetails>(&details)); + profile()->GetExtensionsService()->extension_prefs() + ->SetExtensionControlledPref(extension_id(), pref_path, pref_value); } diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc index 2157f9c..f6fb71d 100644 --- a/chrome/browser/extensions/extensions_service.cc +++ b/chrome/browser/extensions/extensions_service.cc @@ -554,10 +554,10 @@ bool ExtensionsService::UninstallExtensionHelper( ExtensionsService::ExtensionsService(Profile* profile, const CommandLine* command_line, const FilePath& install_directory, + ExtensionPrefs* extension_prefs, bool autoupdate_enabled) : profile_(profile), - extension_prefs_(new ExtensionPrefs(profile->GetPrefs(), - install_directory)), + extension_prefs_(extension_prefs), install_directory_(install_directory), extensions_enabled_(true), show_extensions_prompts_(true), diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h index 3a232fe..d3e67438 100644 --- a/chrome/browser/extensions/extensions_service.h +++ b/chrome/browser/extensions/extensions_service.h @@ -38,7 +38,6 @@ class ExtensionsServiceBackend; class ExtensionToolbarModel; class ExtensionUpdater; class GURL; -class PrefService; class Profile; class Version; @@ -150,9 +149,12 @@ class ExtensionsService static bool UninstallExtensionHelper(ExtensionsService* extensions_service, const std::string& extension_id); + // Constructor stores pointers to |profile| and |extension_prefs| but + // ownership remains at caller. ExtensionsService(Profile* profile, const CommandLine* command_line, const FilePath& install_directory, + ExtensionPrefs* extension_prefs, bool autoupdate_enabled); // Gets the list of currently installed extensions. @@ -404,7 +406,7 @@ class ExtensionsService // it. void DestroyingProfile(); - ExtensionPrefs* extension_prefs() { return extension_prefs_.get(); } + ExtensionPrefs* extension_prefs() { return extension_prefs_; } // Whether the extension service is ready. // TODO(skerner): Get rid of this method. crbug.com/63756 @@ -507,8 +509,8 @@ class ExtensionsService // The profile this ExtensionsService is part of. Profile* profile_; - // Preferences for the owning profile. - scoped_ptr<ExtensionPrefs> extension_prefs_; + // Preferences for the owning profile (weak reference). + ExtensionPrefs* extension_prefs_; // The current list of installed extensions. ExtensionList extensions_; diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc index 2cbb2d1..d7f83fdb 100644 --- a/chrome/browser/extensions/test_extension_prefs.cc +++ b/chrome/browser/extensions/test_extension_prefs.cc @@ -17,7 +17,27 @@ #include "chrome/common/json_pref_store.h" #include "testing/gtest/include/gtest/gtest.h" -TestExtensionPrefs::TestExtensionPrefs() { +// Mock ExtensionPrefs class with artificial clock to guarantee that no two +// extensions get the same installation time stamp and we can reliably +// assert the installation order in the tests below. +class MockExtensionPrefs : public ExtensionPrefs { + public: + MockExtensionPrefs(PrefService* prefs, const FilePath& root_dir_) + : ExtensionPrefs(prefs, root_dir_), + currentTime(base::Time::Now()) + {} + ~MockExtensionPrefs() {} + + protected: + mutable base::Time currentTime; + + virtual base::Time GetCurrentTime() const { + currentTime += base::TimeDelta::FromSeconds(10); + return currentTime; + } +}; + +TestExtensionPrefs::TestExtensionPrefs() : pref_service_(NULL) { EXPECT_TRUE(temp_dir_.CreateUniqueTempDir()); preferences_file_ = temp_dir_.path().AppendASCII("Preferences"); extensions_dir_ = temp_dir_.path().AppendASCII("Extensions"); @@ -29,6 +49,9 @@ TestExtensionPrefs::TestExtensionPrefs() { TestExtensionPrefs::~TestExtensionPrefs() {} void TestExtensionPrefs::RecreateExtensionPrefs() { + // We persist and reload the PrefService's PrefStores because this process + // deletes all empty dictionaries. The ExtensionPrefs implementation + // needs to be able to handle this situation. if (pref_service_.get()) { // The PrefService writes its persistent file on the file thread, so we // need to wait for any pending I/O to complete before creating a new @@ -42,7 +65,7 @@ void TestExtensionPrefs::RecreateExtensionPrefs() { // Create a |PrefService| instance that contains only user defined values. pref_service_.reset(PrefService::CreateUserPrefService(preferences_file_)); ExtensionPrefs::RegisterUserPrefs(pref_service_.get()); - prefs_.reset(new ExtensionPrefs(pref_service_.get(), temp_dir_.path())); + prefs_.reset(new MockExtensionPrefs(pref_service_.get(), temp_dir_.path())); } scoped_refptr<Extension> TestExtensionPrefs::AddExtension(std::string name) { diff --git a/chrome/browser/extensions/test_extension_prefs.h b/chrome/browser/extensions/test_extension_prefs.h index cb723b2..3220d7a 100644 --- a/chrome/browser/extensions/test_extension_prefs.h +++ b/chrome/browser/extensions/test_extension_prefs.h @@ -16,7 +16,6 @@ class DictionaryValue; class ExtensionPrefs; class PrefService; - // This is a test class intended to make it easier to work with ExtensionPrefs // in tests. class TestExtensionPrefs { diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc index fda53a7..f7a11ffe 100644 --- a/chrome/browser/prefs/pref_service.cc +++ b/chrome/browser/prefs/pref_service.cc @@ -579,6 +579,10 @@ void PrefService::SetUserPrefValue(const char* path, Value* new_value) { pref_notifier_->OnUserPreferenceSet(path); } +PrefStore* PrefService::GetExtensionPrefStore() const { + return pref_value_store()->GetExtensionPrefStore(); +} + /////////////////////////////////////////////////////////////////////////////// // PrefService::Preference diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h index 632bc84..18266bc 100644 --- a/chrome/browser/prefs/pref_service.h +++ b/chrome/browser/prefs/pref_service.h @@ -225,6 +225,8 @@ class PrefService : public NonThreadSafe { PrefValueStore* pref_value_store() const { return pref_value_store_.get(); } + PrefStore* GetExtensionPrefStore() const; + protected: // 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_value_store.cc b/chrome/browser/prefs/pref_value_store.cc index 84b6919..b327e33 100644 --- a/chrome/browser/prefs/pref_value_store.cc +++ b/chrome/browser/prefs/pref_value_store.cc @@ -5,7 +5,6 @@ #include "chrome/browser/prefs/pref_value_store.h" #include "chrome/browser/browser_thread.h" -#include "chrome/browser/extensions/extension_pref_store.h" #include "chrome/browser/policy/configuration_policy_pref_store.h" #include "chrome/browser/prefs/command_line_pref_store.h" #include "chrome/browser/prefs/in_memory_pref_store.h" @@ -41,13 +40,13 @@ PrefValueStore* PrefValueStore::CreatePrefValueStore( using policy::ConfigurationPolicyPrefStore; ConfigurationPolicyPrefStore* managed = NULL; ConfigurationPolicyPrefStore* device_management = NULL; - ExtensionPrefStore* extension = NULL; CommandLinePrefStore* command_line = NULL; ConfigurationPolicyPrefStore* recommended = NULL; JsonPrefStore* user = new JsonPrefStore( pref_filename, BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)); + InMemoryPrefStore* extension = new InMemoryPrefStore(); InMemoryPrefStore* default_store = new InMemoryPrefStore(); if (!user_only) { @@ -56,7 +55,6 @@ PrefValueStore* PrefValueStore::CreatePrefValueStore( device_management = ConfigurationPolicyPrefStore::CreateDeviceManagementPolicyPrefStore( profile); - extension = new ExtensionPrefStore(profile, PrefNotifier::EXTENSION_STORE); command_line = new CommandLinePrefStore(CommandLine::ForCurrentProcess()); recommended = ConfigurationPolicyPrefStore::CreateRecommendedPolicyPrefStore(); @@ -148,13 +146,19 @@ bool PrefValueStore::HasPrefPath(const char* path) const { bool PrefValueStore::PrefHasChanged(const char* path, PrefNotifier::PrefStoreType new_store) { DCHECK(new_store != PrefNotifier::INVALID_STORE); + // If we get a change notification about an unregistered preference, + // discard it silently because there cannot be any listeners. + if (pref_types_.find(path) == pref_types_.end()) + return false; + // Replying that the pref has changed may cause problems, but it's the safer // choice. if (new_store == PrefNotifier::INVALID_STORE) return true; PrefNotifier::PrefStoreType controller = ControllingPrefStoreForPref(path); - DCHECK(controller != PrefNotifier::INVALID_STORE); + DCHECK(controller != PrefNotifier::INVALID_STORE) + << "Invalid controller for path " << path; if (controller == PrefNotifier::INVALID_STORE) return true; @@ -463,6 +467,10 @@ bool PrefValueStore::HasPolicyConflictingUserProxySettings() { return false; } +PrefStore* PrefValueStore::GetExtensionPrefStore() const { + return pref_stores_[PrefNotifier::EXTENSION_STORE].get(); +} + PrefValueStore::PrefValueStore(PrefStore* managed_platform_prefs, PrefStore* device_management_prefs, PrefStore* extension_prefs, diff --git a/chrome/browser/prefs/pref_value_store.h b/chrome/browser/prefs/pref_value_store.h index 37c1167..f4884e2 100644 --- a/chrome/browser/prefs/pref_value_store.h +++ b/chrome/browser/prefs/pref_value_store.h @@ -172,6 +172,10 @@ class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore> { // that conflict with proxy settings specified by proxy policy. bool HasPolicyConflictingUserProxySettings(); + // TODO(mnissler) delete after applying your patch. + // This is only called only by PrefService. + PrefStore* GetExtensionPrefStore() const; + protected: // In decreasing order of precedence: // |managed_platform_prefs| contains all managed platform (non-cloud policy) diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index 71eb505..66b023f 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -276,6 +276,12 @@ ProfileImpl::ProfileImpl(const FilePath& path) pref_change_registrar_.Add(prefs::kEnableAutoSpellCorrect, this); pref_change_registrar_.Add(prefs::kClearSiteDataOnExit, this); + // Ensure that preferences set by extensions are restored in the profile + // as early as possible. The constructor takes care of that. + extension_prefs_.reset(new ExtensionPrefs( + GetPrefs(), + GetPath().AppendASCII(ExtensionsService::kInstallDirectoryName))); + // Convert active labs into switches. Modifies the current command line. about_flags::ConvertFlagsToSwitches(prefs, CommandLine::ForCurrentProcess()); @@ -354,6 +360,7 @@ void ProfileImpl::InitExtensions() { this, CommandLine::ForCurrentProcess(), GetPath().AppendASCII(ExtensionsService::kInstallDirectoryName), + extension_prefs_.get(), true); RegisterComponentExtensions(); diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h index d1a2d1e..d018825 100644 --- a/chrome/browser/profiles/profile_impl.h +++ b/chrome/browser/profiles/profile_impl.h @@ -18,6 +18,7 @@ #include "chrome/common/notification_observer.h" #include "chrome/common/notification_registrar.h" +class ExtensionPrefs; class PrefService; #if defined(OS_CHROMEOS) @@ -169,8 +170,15 @@ class ProfileImpl : public Profile, FilePath path_; FilePath base_cache_path_; + // Keep prefs_ on top for destruction order because extension_prefs_, + // net_pref_observer_, web_resource_service_ and background_contents_service_ + // store pointers to prefs_ and shall be destructed first. + scoped_ptr<PrefService> prefs_; scoped_ptr<VisitedLinkEventListener> visited_link_event_listener_; scoped_ptr<VisitedLinkMaster> visited_link_master_; + // Keep extension_prefs_ on top of extensions_service_ because the latter + // maintains a pointer to the first and shall be destructed first. + scoped_ptr<ExtensionPrefs> extension_prefs_; scoped_refptr<ExtensionsService> extensions_service_; scoped_refptr<UserScriptMaster> user_script_master_; scoped_refptr<ExtensionDevToolsManager> extension_devtools_manager_; @@ -183,7 +191,6 @@ class ProfileImpl : public Profile, scoped_refptr<TransportSecurityPersister> transport_security_persister_; scoped_ptr<policy::ProfilePolicyContext> profile_policy_context_; - scoped_ptr<PrefService> prefs_; scoped_ptr<NetPrefObserver> net_pref_observer_; scoped_ptr<TemplateURLFetcher> template_url_fetcher_; scoped_ptr<TemplateURLModel> template_url_model_; diff --git a/chrome/browser/ui/cocoa/extensions/extension_popup_controller_unittest.mm b/chrome/browser/ui/cocoa/extensions/extension_popup_controller_unittest.mm index 0e74e5e..ac00647 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_popup_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_popup_controller_unittest.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "base/scoped_nsobject.h" +#include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/extension_process_manager.h" #include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/ui/cocoa/browser_test_helper.h" @@ -25,9 +26,12 @@ class ExtensionTestingProfile : public TestingProfile { DCHECK(!GetExtensionsService()); manager_.reset(ExtensionProcessManager::Create(this)); + extension_prefs_.reset(new ExtensionPrefs(GetPrefs(), + GetExtensionsInstallDir())); service_ = new ExtensionsService(this, CommandLine::ForCurrentProcess(), - GetExtensionsInstallDir(), + GetExtensionsInstallDir(), + extension_prefs_.get(), false); service_->set_extensions_enabled(true); service_->set_show_extensions_prompts(false); @@ -38,6 +42,7 @@ class ExtensionTestingProfile : public TestingProfile { void ShutdownExtensionProfile() { manager_.reset(); service_ = NULL; + extension_prefs_.reset(); } virtual ExtensionProcessManager* GetExtensionProcessManager() { @@ -50,6 +55,7 @@ class ExtensionTestingProfile : public TestingProfile { private: scoped_ptr<ExtensionProcessManager> manager_; + scoped_ptr<ExtensionPrefs> extension_prefs_; scoped_refptr<ExtensionsService> service_; DISALLOW_COPY_AND_ASSIGN(ExtensionTestingProfile); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 6d5c3fa..3f3a54c 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1102,8 +1102,6 @@ '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 bcaecf2..defc011 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1134,7 +1134,6 @@ 'browser/extensions/extension_info_map_unittest.cc', 'browser/extensions/extension_menu_manager_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_omnibox_unittest.cc', 'browser/extensions/extension_ui_unittest.cc', diff --git a/chrome/test/testing_pref_service.cc b/chrome/test/testing_pref_service.cc index d8d08e5..3713c8c 100644 --- a/chrome/test/testing_pref_service.cc +++ b/chrome/test/testing_pref_service.cc @@ -14,13 +14,13 @@ // which they want, and expand usage of this class to more unit tests. TestingPrefService::TestingPrefService() : PrefService(new TestingPrefValueStore( - managed_platform_prefs_ = new DummyPrefStore(), - device_management_prefs_ = new DummyPrefStore(), - NULL, - NULL, - user_prefs_ = new DummyPrefStore(), - NULL, - default_prefs_ = new DummyPrefStore())) { + managed_platform_prefs_ = new DummyPrefStore(), + device_management_prefs_ = new DummyPrefStore(), + NULL, + NULL, + user_prefs_ = new DummyPrefStore(), + NULL, + default_prefs_ = new DummyPrefStore())) { } TestingPrefService::TestingPrefService( diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc index c36f66b..52e5332 100644 --- a/chrome/test/testing_profile.cc +++ b/chrome/test/testing_profile.cc @@ -249,7 +249,7 @@ void TestingProfile::DestroyHistoryService() { void TestingProfile::CreateTopSites() { DestroyTopSites(); top_sites_ = new history::TopSites(this); - FilePath file_name = temp_dir_.path().Append(chrome::kTopSitesFilename); + FilePath file_name = GetPath().Append(chrome::kTopSitesFilename); top_sites_->Init(file_name); } @@ -347,9 +347,11 @@ void TestingProfile::UseThemeProvider(BrowserThemeProvider* theme_provider) { scoped_refptr<ExtensionsService> TestingProfile::CreateExtensionsService( const CommandLine* command_line, const FilePath& install_directory) { + extension_prefs_.reset(new ExtensionPrefs(GetPrefs(),install_directory)); extensions_service_ = new ExtensionsService(this, command_line, install_directory, + extension_prefs_.get(), false); return extensions_service_; } diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h index 6e6b6b4..3469c33 100644 --- a/chrome/test/testing_profile.h +++ b/chrome/test/testing_profile.h @@ -25,6 +25,7 @@ class BookmarkModel; class BrowserThemeProvider; class CommandLine; class DesktopNotificationService; +class ExtensionPrefs; class FaviconService; class FindBarState; class GeolocationContentSettingsMap; @@ -328,7 +329,7 @@ class TestingProfile : public Profile { // from the destructor. void DestroyWebDataService(); - // Creates a TestingPrefService and associates it with the TestingProfile + // Creates a TestingPrefService and associates it with the TestingProfile. void CreateTestingPrefService(); // The favicon service. Only created if CreateFaviconService is invoked. @@ -404,6 +405,10 @@ class TestingProfile : public Profile { FilePath last_selected_directory_; scoped_refptr<history::TopSites> top_sites_; // For history and thumbnails. + // The Extension Preferences. Only created if CreateExtensionsService is + // invoked. + scoped_ptr<ExtensionPrefs> extension_prefs_; + // For properly notifying the ExtensionsService when the profile // is disposed. scoped_refptr<ExtensionsService> extensions_service_; |