Fix memory leak in the SetDefaultContentSettings method of the HostContentSettingaMap.

BUG=104775, 102637
TEST=HostContentSettingsMapTest*

Review URL: http://codereview.chromium.org/8528031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@110908 0039d316-1c4b-4281-b951-d872f2087c98
parent f36f3747
...@@ -134,7 +134,7 @@ bool DefaultProvider::SetWebsiteSetting( ...@@ -134,7 +134,7 @@ bool DefaultProvider::SetWebsiteSetting(
const ContentSettingsPattern& secondary_pattern, const ContentSettingsPattern& secondary_pattern,
ContentSettingsType content_type, ContentSettingsType content_type,
const ResourceIdentifier& resource_identifier, const ResourceIdentifier& resource_identifier,
Value* value) { Value* in_value) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(prefs_); DCHECK(prefs_);
...@@ -149,12 +149,15 @@ bool DefaultProvider::SetWebsiteSetting( ...@@ -149,12 +149,15 @@ bool DefaultProvider::SetWebsiteSetting(
if (is_incognito_) if (is_incognito_)
return false; 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); AutoReset<bool> auto_reset(&updating_preferences_, true);
// Keep the obsolete pref in sync as long as backwards compatibility is // Keep the obsolete pref in sync as long as backwards compatibility is
// required. This is required to keep sync working correctly. // required. This is required to keep sync working correctly.
if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) { if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) {
if (value) { if (value.get()) {
prefs_->Set(prefs::kGeolocationDefaultContentSetting, *value); prefs_->Set(prefs::kGeolocationDefaultContentSetting, *value);
} else { } else {
prefs_->ClearPref(prefs::kGeolocationDefaultContentSetting); prefs_->ClearPref(prefs::kGeolocationDefaultContentSetting);
...@@ -169,8 +172,8 @@ bool DefaultProvider::SetWebsiteSetting( ...@@ -169,8 +172,8 @@ bool DefaultProvider::SetWebsiteSetting(
DictionaryPrefUpdate update(prefs_, prefs::kDefaultContentSettings); DictionaryPrefUpdate update(prefs_, prefs::kDefaultContentSettings);
DictionaryValue* default_settings_dictionary = update.Get(); DictionaryValue* default_settings_dictionary = update.Get();
base::AutoLock lock(lock_); base::AutoLock lock(lock_);
if (value == NULL || if (value.get() == NULL ||
ValueToContentSetting(value) == kDefaultSettings[content_type]) { ValueToContentSetting(value.get()) == kDefaultSettings[content_type]) {
// If |value| is NULL we need to reset the default setting the the // If |value| is NULL we need to reset the default setting the the
// hardcoded default. // hardcoded default.
default_settings_[content_type].reset( default_settings_[content_type].reset(
...@@ -184,7 +187,7 @@ bool DefaultProvider::SetWebsiteSetting( ...@@ -184,7 +187,7 @@ bool DefaultProvider::SetWebsiteSetting(
default_settings_[content_type].reset(value->DeepCopy()); default_settings_[content_type].reset(value->DeepCopy());
// Transfer ownership of |value| to the |default_settings_dictionary|. // Transfer ownership of |value| to the |default_settings_dictionary|.
default_settings_dictionary->SetWithoutPathExpansion( default_settings_dictionary->SetWithoutPathExpansion(
GetTypeName(content_type), value); GetTypeName(content_type), value.release());
} }
} }
......
...@@ -210,12 +210,15 @@ void HostContentSettingsMap::SetDefaultContentSetting( ...@@ -210,12 +210,15 @@ void HostContentSettingsMap::SetDefaultContentSetting(
DCHECK_NE(content_type, CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE); DCHECK_NE(content_type, CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE);
DCHECK(IsSettingAllowedForType(setting, content_type)); DCHECK(IsSettingAllowedForType(setting, content_type));
base::Value* value = Value::CreateIntegerValue(setting); base::Value* value = NULL;
if (!content_settings_providers_[DEFAULT_PROVIDER]->SetWebsiteSetting( if (setting != CONTENT_SETTING_DEFAULT)
ContentSettingsPattern::Wildcard(), ContentSettingsPattern::Wildcard(), value = Value::CreateIntegerValue(setting);
content_type, std::string(), value)) { SetWebsiteSetting(
delete value; ContentSettingsPattern::Wildcard(),
} ContentSettingsPattern::Wildcard(),
content_type,
std::string(),
value);
} }
void HostContentSettingsMap::SetWebsiteSetting( void HostContentSettingsMap::SetWebsiteSetting(
......
...@@ -1745,11 +1745,3 @@ ...@@ -1745,11 +1745,3 @@
# fun:PrintPreviewTabControllerUnitTest_MultiplePreviewTabs_Test::TestBody # fun:PrintPreviewTabControllerUnitTest_MultiplePreviewTabs_Test::TestBody
# fun:PrintPreviewTabControllerUnitTest_ClearInitiatorTabDetails_Test::TestBody # fun:PrintPreviewTabControllerUnitTest_ClearInitiatorTabDetails_Test::TestBody
} }
{
bug_104775
Heapcheck:Leak
fun:base::Value::CreateIntegerValue
fun:HostContentSettingsMap::SetDefaultContentSetting
fun:CookieSettings::SetDefaultCookieSetting
fun:ExtensionSpecialStoragePolicyTest_HasSessionOnlyOrigins_Test::TestBody
}
...@@ -5219,15 +5219,6 @@ ...@@ -5219,15 +5219,6 @@
fun:_ZN8chromeos18LowBatteryObserver12PowerChangedERKNS_17PowerSupplyStatusE fun:_ZN8chromeos18LowBatteryObserver12PowerChangedERKNS_17PowerSupplyStatusE
fun:_ZN8chromeos26PowerManagerClientStubImpl6UpdateEv fun:_ZN8chromeos26PowerManagerClientStubImpl6UpdateEv
} }
{
bug_104775
Memcheck:Leak
fun:_Znw*
fun:_ZN4base5Value18CreateIntegerValueEi
fun:_ZN22HostContentSettingsMap24SetDefaultContentSettingE19ContentSettingsType14ContentSetting
fun:_ZN14CookieSettings23SetDefaultCookieSettingE14ContentSetting
fun:_ZN60ExtensionSpecialStoragePolicyTest_HasSessionOnlyOrigins_Test8TestBodyEv
}
{ {
bug_104806_a bug_104806_a
Memcheck:Leak Memcheck:Leak
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment