Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1109)

Unified Diff: chrome/browser/content_settings/content_settings_default_provider.cc

Issue 8528031: Fix memory leak in SetDefaultContentSettings. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Remove unneccessary delete and comment Created 9 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/content_settings/content_settings_default_provider.cc
diff --git a/chrome/browser/content_settings/content_settings_default_provider.cc b/chrome/browser/content_settings/content_settings_default_provider.cc
index 8c44d8cabbb9fae94409e9ffd5d9069a7a4ca01d..15b31de6d5bba2983be216e8b462d2b9861a7ee8 100644
--- a/chrome/browser/content_settings/content_settings_default_provider.cc
+++ b/chrome/browser/content_settings/content_settings_default_provider.cc
@@ -134,7 +134,7 @@ bool DefaultProvider::SetWebsiteSetting(
const ContentSettingsPattern& secondary_pattern,
ContentSettingsType content_type,
const ResourceIdentifier& resource_identifier,
- Value* value) {
+ Value* in_value) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(prefs_);
@@ -149,12 +149,15 @@ bool DefaultProvider::SetWebsiteSetting(
if (is_incognito_)
return false;
+ // Put |in_value| in a scoped pointer to ensure that it gets cleaned up
+ // properly if we don't pass on the ownership.
+ scoped_ptr<base::Value> value(in_value);
{
AutoReset<bool> auto_reset(&updating_preferences_, true);
// Keep the obsolete pref in sync as long as backwards compatibility is
// required. This is required to keep sync working correctly.
if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) {
- if (value) {
+ if (value.get()) {
prefs_->Set(prefs::kGeolocationDefaultContentSetting, *value);
} else {
prefs_->ClearPref(prefs::kGeolocationDefaultContentSetting);
@@ -169,8 +172,8 @@ bool DefaultProvider::SetWebsiteSetting(
DictionaryPrefUpdate update(prefs_, prefs::kDefaultContentSettings);
DictionaryValue* default_settings_dictionary = update.Get();
base::AutoLock lock(lock_);
- if (value == NULL ||
- ValueToContentSetting(value) == kDefaultSettings[content_type]) {
+ if (value.get() == NULL ||
+ ValueToContentSetting(value.get()) == kDefaultSettings[content_type]) {
// If |value| is NULL we need to reset the default setting the the
// hardcoded default.
default_settings_[content_type].reset(
@@ -184,7 +187,7 @@ bool DefaultProvider::SetWebsiteSetting(
default_settings_[content_type].reset(value->DeepCopy());
// Transfer ownership of |value| to the |default_settings_dictionary|.
default_settings_dictionary->SetWithoutPathExpansion(
- GetTypeName(content_type), value);
+ GetTypeName(content_type), value.release());
}
}

Powered by Google App Engine
This is Rietveld 408576698