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

Side by Side 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: Fix memory leaks 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 unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright (c) 2011 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/content_settings/content_settings_default_provider.h" 5 #include "chrome/browser/content_settings/content_settings_default_provider.h"
6 6
7 #include <string> 7 #include <string>
8 #include <vector> 8 #include <vector>
9 9
10 #include "base/auto_reset.h" 10 #include "base/auto_reset.h"
(...skipping 116 matching lines...) Expand 10 before | Expand all | Expand 10 after
127 } 127 }
128 128
129 DefaultProvider::~DefaultProvider() { 129 DefaultProvider::~DefaultProvider() {
130 } 130 }
131 131
132 bool DefaultProvider::SetWebsiteSetting( 132 bool DefaultProvider::SetWebsiteSetting(
133 const ContentSettingsPattern& primary_pattern, 133 const ContentSettingsPattern& primary_pattern,
134 const ContentSettingsPattern& secondary_pattern, 134 const ContentSettingsPattern& secondary_pattern,
135 ContentSettingsType content_type, 135 ContentSettingsType content_type,
136 const ResourceIdentifier& resource_identifier, 136 const ResourceIdentifier& resource_identifier,
137 Value* value) { 137 Value* in_value) {
138 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); 138 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
139 DCHECK(prefs_); 139 DCHECK(prefs_);
140 140
141 // Ignore non default settings 141 // Ignore non default settings
142 if (primary_pattern != ContentSettingsPattern::Wildcard() || 142 if (primary_pattern != ContentSettingsPattern::Wildcard() ||
143 secondary_pattern != ContentSettingsPattern::Wildcard()) { 143 secondary_pattern != ContentSettingsPattern::Wildcard()) {
144 return false; 144 return false;
145 } 145 }
146 146
147 // The default settings may not be directly modified for OTR sessions. 147 // The default settings may not be directly modified for OTR sessions.
148 // Instead, they are synced to the main profile's setting. 148 // Instead, they are synced to the main profile's setting.
149 if (is_incognito_) 149 if (is_incognito_)
150 return false; 150 return false;
151 151
152 // Put |in_value| in a scoped pointer to ensure that it gets cleaned up
153 // properly if we don't pass on the ownership.
154 scoped_ptr<base::Value> value(in_value);
152 { 155 {
153 AutoReset<bool> auto_reset(&updating_preferences_, true); 156 AutoReset<bool> auto_reset(&updating_preferences_, true);
154 // Keep the obsolete pref in sync as long as backwards compatibility is 157 // Keep the obsolete pref in sync as long as backwards compatibility is
155 // required. This is required to keep sync working correctly. 158 // required. This is required to keep sync working correctly.
156 if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) { 159 if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) {
157 if (value) { 160 if (value.get()) {
158 prefs_->Set(prefs::kGeolocationDefaultContentSetting, *value); 161 prefs_->Set(prefs::kGeolocationDefaultContentSetting, *value);
159 } else { 162 } else {
160 prefs_->ClearPref(prefs::kGeolocationDefaultContentSetting); 163 prefs_->ClearPref(prefs::kGeolocationDefaultContentSetting);
161 } 164 }
162 } 165 }
163 166
164 // |DefaultProvider| should not send any notifications when holding 167 // |DefaultProvider| should not send any notifications when holding
165 // |lock_|. |DictionaryPrefUpdate| destructor and 168 // |lock_|. |DictionaryPrefUpdate| destructor and
166 // |PrefService::SetInteger()| send out notifications. As a response, the 169 // |PrefService::SetInteger()| send out notifications. As a response, the
167 // upper layers may call |GetAllContentSettingRules| which acquires |lock_| 170 // upper layers may call |GetAllContentSettingRules| which acquires |lock_|
168 // again. 171 // again.
169 DictionaryPrefUpdate update(prefs_, prefs::kDefaultContentSettings); 172 DictionaryPrefUpdate update(prefs_, prefs::kDefaultContentSettings);
170 DictionaryValue* default_settings_dictionary = update.Get(); 173 DictionaryValue* default_settings_dictionary = update.Get();
171 base::AutoLock lock(lock_); 174 base::AutoLock lock(lock_);
172 if (value == NULL || 175 if (value.get() == NULL ||
173 ValueToContentSetting(value) == kDefaultSettings[content_type]) { 176 ValueToContentSetting(value.get()) == kDefaultSettings[content_type]) {
174 // If |value| is NULL we need to reset the default setting the the 177 // If |value| is NULL we need to reset the default setting the the
175 // hardcoded default. 178 // hardcoded default.
176 default_settings_[content_type].reset( 179 default_settings_[content_type].reset(
177 Value::CreateIntegerValue(kDefaultSettings[content_type])); 180 Value::CreateIntegerValue(kDefaultSettings[content_type]));
178 181
179 // Remove the corresponding pref entry since the hardcoded default value 182 // Remove the corresponding pref entry since the hardcoded default value
180 // is used. 183 // is used.
181 default_settings_dictionary->RemoveWithoutPathExpansion( 184 default_settings_dictionary->RemoveWithoutPathExpansion(
182 GetTypeName(content_type), NULL); 185 GetTypeName(content_type), NULL);
183 } else { 186 } else {
184 default_settings_[content_type].reset(value->DeepCopy()); 187 default_settings_[content_type].reset(value->DeepCopy());
185 // Transfer ownership of |value| to the |default_settings_dictionary|. 188 // Transfer ownership of |value| to the |default_settings_dictionary|.
186 default_settings_dictionary->SetWithoutPathExpansion( 189 default_settings_dictionary->SetWithoutPathExpansion(
187 GetTypeName(content_type), value); 190 GetTypeName(content_type), value.release());
188 } 191 }
189 } 192 }
190 193
191 NotifyObservers(ContentSettingsPattern(), 194 NotifyObservers(ContentSettingsPattern(),
192 ContentSettingsPattern(), 195 ContentSettingsPattern(),
193 content_type, 196 content_type,
194 std::string()); 197 std::string());
195 198
196 return true; 199 return true;
197 } 200 }
(...skipping 150 matching lines...) Expand 10 before | Expand all | Expand 10 after
348 SetWebsiteSetting( 351 SetWebsiteSetting(
349 ContentSettingsPattern::Wildcard(), 352 ContentSettingsPattern::Wildcard(),
350 ContentSettingsPattern::Wildcard(), 353 ContentSettingsPattern::Wildcard(),
351 CONTENT_SETTINGS_TYPE_GEOLOCATION, 354 CONTENT_SETTINGS_TYPE_GEOLOCATION,
352 std::string(), 355 std::string(),
353 value->DeepCopy()); 356 value->DeepCopy());
354 } 357 }
355 } 358 }
356 359
357 } // namespace content_settings 360 } // namespace content_settings
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698