diff options
| -rw-r--r-- | chrome/browser/extensions/extension_pref_store.cc | 22 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_pref_store.h | 22 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_pref_store_unittest.cc | 22 | ||||
| -rw-r--r-- | tools/heapcheck/suppressions.txt | 32 | ||||
| -rw-r--r-- | tools/valgrind/memcheck/suppressions.txt | 20 |
5 files changed, 37 insertions, 81 deletions
diff --git a/chrome/browser/extensions/extension_pref_store.cc b/chrome/browser/extensions/extension_pref_store.cc index 437ca77..7635151 100644 --- a/chrome/browser/extensions/extension_pref_store.cc +++ b/chrome/browser/extensions/extension_pref_store.cc @@ -5,7 +5,6 @@ #include "chrome/browser/extensions/extension_pref_store.h" #include "base/logging.h" -#include "base/stl_util-inl.h" #include "base/values.h" #include "chrome/browser/pref_service.h" @@ -49,6 +48,9 @@ void ExtensionPrefStore::UpdateOnePref(const wchar_t* path) { } 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); @@ -70,14 +72,14 @@ void ExtensionPrefStore::InstallExtensionPref(std::string extension_id, PrefValueMap* pref_values; if (i != extension_stack_.end()) { pref_values = (*i)->pref_values; + delete (*pref_values)[pref_path]; (*pref_values)[pref_path] = pref_value; } else { pref_values = new PrefValueMap(); (*pref_values)[pref_path] = pref_value; - ExtensionPrefs* extension_prefs = new ExtensionPrefs(); - extension_prefs->extension_id = extension_id; - extension_prefs->pref_values = pref_values; + ExtensionPrefs* extension_prefs = new ExtensionPrefs(extension_id, + pref_values); extension_stack_.push_front(extension_prefs); } @@ -87,20 +89,16 @@ void ExtensionPrefStore::InstallExtensionPref(std::string extension_id, void ExtensionPrefStore::UninstallExtension(std::string extension_id) { // Remove this extension from the stack. - scoped_ptr<PrefValueMap> pref_values; for (ExtensionStack::iterator i = extension_stack_.begin(); i != extension_stack_.end(); ++i) { if ((*i)->extension_id == extension_id) { - pref_values.reset((*i)->pref_values); - delete *i; + ExtensionPrefs* to_be_deleted = *i; extension_stack_.erase(i); - break; + UpdatePrefs(to_be_deleted->pref_values); + delete to_be_deleted; + return; } } - if (!pref_values.get()) - return; - - UpdatePrefs(pref_values.get()); } void ExtensionPrefStore::GetExtensionIDs(std::vector<std::string>* result) { diff --git a/chrome/browser/extensions/extension_pref_store.h b/chrome/browser/extensions/extension_pref_store.h index fc10b5b..26c6c13 100644 --- a/chrome/browser/extensions/extension_pref_store.h +++ b/chrome/browser/extensions/extension_pref_store.h @@ -12,6 +12,7 @@ #include "base/basictypes.h" #include "base/scoped_ptr.h" +#include "base/stl_util-inl.h" #include "chrome/common/pref_store.h" class DictionaryValue; @@ -35,6 +36,7 @@ class ExtensionPrefStore : public PrefStore { // 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(std::string extension_id, const wchar_t* pref_path, Value* pref_value); @@ -61,26 +63,34 @@ class ExtensionPrefStore : public PrefStore { typedef std::map<const wchar_t*, Value*> PrefValueMap; // Applies the highest-priority extension's setting for the given preference - // path, or clears the setting in this PrefStore if no extensions wish to - // control it. + // path to the |prefs_| store, or clears the setting there if no extensions + // wish to control it. void UpdateOnePref(const wchar_t* path); - // Updates each preference in the |pref_values| list. + // Updates each preference in the key set of the |pref_values| map. void UpdatePrefs(const PrefValueMap* pref_values); // A cache of the highest-priority values for each preference that any // extension is controlling, for quick read access. Owns the stored values. scoped_ptr<DictionaryValue> prefs_; - // Associates an extension ID with the prefs it sets. + // Associates an extension ID with the prefs it sets. Owns the pref values. struct ExtensionPrefs { + ExtensionPrefs(std::string id, PrefValueMap* values) + : extension_id(id), + pref_values(values) {} + + ~ExtensionPrefs() { + STLDeleteValues(pref_values); + delete pref_values; + } + std::string extension_id; PrefValueMap* pref_values; }; // A pseudo-stack of extensions and their preferences. Extensions are always - // added to the head, but may be removed from the middle. This stack owns - // the values in the extensions' PrefValueMaps. + // added to the head, but may be removed from the middle. typedef std::list<ExtensionPrefs*> ExtensionStack; ExtensionStack extension_stack_; diff --git a/chrome/browser/extensions/extension_pref_store_unittest.cc b/chrome/browser/extensions/extension_pref_store_unittest.cc index 166abaa..aa16441 100644 --- a/chrome/browser/extensions/extension_pref_store_unittest.cc +++ b/chrome/browser/extensions/extension_pref_store_unittest.cc @@ -153,7 +153,7 @@ TEST(ExtensionPrefStoreTest, UninstallOnlyExtension) { eps.InstallExtensionPref("id1", kPref1, Value::CreateStringValue("val1")); eps.InstallExtensionPref("id1", kPref2, Value::CreateStringValue("val2")); - // No need to check the state here; the InstallOneExtension already has. + // No need to check the state here; the Install* tests cover that. eps.UninstallExtension("id1"); TestExtensionPrefStore::ExtensionIDs ids; @@ -256,26 +256,26 @@ TEST(ExtensionPrefStoreTest, UninstallExtensionFromMiddle) { } TEST(ExtensionPrefStoreTest, NotifyWhenNeeded) { - TestExtensionPrefStore eps; + TestExtensionPrefStore* eps = new TestExtensionPrefStore; // The PrefValueStore takes ownership of the PrefStores; in this case, that's - // only an ExtensionPrefStore. - PrefValueStore* value_store = new PrefValueStore(NULL, &eps, NULL, NULL, - NULL); - MockPrefService* pref_service = new MockPrefService(value_store); - eps.SetPrefService(pref_service); + // only an ExtensionPrefStore. Likewise, the PrefService takes ownership of + // the PrefValueStore. + PrefValueStore* value_store = new PrefValueStore(NULL, eps, NULL, NULL, NULL); + scoped_ptr<MockPrefService> pref_service(new MockPrefService(value_store)); + eps->SetPrefService(pref_service.get()); pref_service->RegisterStringPref(kPref1, std::string()); - eps.InstallExtensionPref("abc", kPref1, + eps->InstallExtensionPref("abc", kPref1, Value::CreateStringValue("https://www.chromium.org")); EXPECT_TRUE(pref_service->fired_observers_); - eps.InstallExtensionPref("abc", kPref1, + eps->InstallExtensionPref("abc", kPref1, Value::CreateStringValue("https://www.chromium.org")); EXPECT_FALSE(pref_service->fired_observers_); - eps.InstallExtensionPref("abc", kPref1, + eps->InstallExtensionPref("abc", kPref1, Value::CreateStringValue("chrome://newtab")); EXPECT_TRUE(pref_service->fired_observers_); - eps.UninstallExtension("abc"); + eps->UninstallExtension("abc"); EXPECT_TRUE(pref_service->fired_observers_); } diff --git a/tools/heapcheck/suppressions.txt b/tools/heapcheck/suppressions.txt index 8578f0c..ac07700 100644 --- a/tools/heapcheck/suppressions.txt +++ b/tools/heapcheck/suppressions.txt @@ -840,38 +840,6 @@ fun:start_thread } { - 48980_a - Heapcheck:Leak - ... - fun:ExtensionPrefStoreTest_NotifyWhenNeeded_Test::TestBody -} -{ - 48980_b - Heapcheck:Leak - ... - fun:PrefService::RegisterStringPref - fun:ExtensionPrefStoreTest_*_Test::TestBody -} -{ - 48980_c - Heapcheck:Leak - ... - fun:ExtensionPrefStore::InstallExtensionPref -} -{ - 48980_d - Heapcheck:Leak - ... - fun:basic_string - fun:ExtensionPrefStoreTest_*_Test::TestBody -} -{ - 48980_e - Heapcheck:Leak - fun:Value::CreateStringValue - fun:ExtensionPrefStoreTest_*_Test::TestBody -} -{ bug_49300_a Heapcheck:Leak fun:disk_cache::StorageBlock::AllocateData diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt index 34aa1a5..9649db2 100644 --- a/tools/valgrind/memcheck/suppressions.txt +++ b/tools/valgrind/memcheck/suppressions.txt @@ -3469,26 +3469,6 @@ fun:_ZN51AudioRendererHostTest_CreateLowLatencyAndClose_Test8TestBodyEv } { - bug_48980_a - Memcheck:Leak - ... - fun:_ZN18ExtensionPrefStore20InstallExtensionPrefESsPKwP5Value - fun:_ZN*ExtensionPrefStoreTest_*_Test8TestBodyEv -} -{ - bug_48980_b - Memcheck:Leak - fun:_Znw* - fun:_ZN5Value17CreateStringValueERKSs - fun:_ZN*ExtensionPrefStoreTest_*_Test8TestBodyEv -} -{ - bug_48980_c - Memcheck:Leak - fun:_Znw* - fun:_ZN44ExtensionPrefStoreTest_NotifyWhenNeeded_Test8TestBodyEv -} -{ bug_48998 Memcheck:Leak fun:_Znw* |