Explicitly ShutdownOnUIThread the HostContentSettingsMap when destroying the Profile.

Explicitly ShutdownOnUIThread the HostContentSettingsMap when destroying the Profile.

Also, get rid of Profile dependencies while we're at it.

BUG=88037, 88557
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=92128

[Bernhard Bauer, 2011-07-06 13:39:56]

Please review.

Based on http://codereview.chromium.org/7275018/.

[markusheintz_, 2011-07-06 15:56:24]

LGTM with nits

http://codereview.chromium.org/7218073/diff/18001/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_pref_provider.cc (right):

http://codereview.chromium.org/7218073/diff/18001/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_pref_provider.cc:1069:
AutoReset<bool> tmp(&updating_preferences_, true);
This is up to you:
Could we use a more descriptive name than tmp?
Maybe auto_reset or something like this?

http://codereview.chromium.org/7218073/diff/18001/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_pref_provider.h (right):

http://codereview.chromium.org/7218073/diff/18001/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_pref_provider.h:36: explicit
PrefDefaultProvider(HostContentSettingsMap* map,
explicit not needed

http://codereview.chromium.org/7218073/diff/18001/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_pref_provider.h:108: explicit
PrefProvider(HostContentSettingsMap* map,
pls remove the explicit

[markusheintz_, 2011-07-07 15:23:17]

Just three nit comments

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_policy_provider.h (right):

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_policy_provider.h:138:
HostContentSettingsMap* map_;
Maybe you should use a better name here.
e.g. host_content_settings_map. 

This way it is easier in the code to see which 'map' is used.

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_pref_provider.h (right):

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_pref_provider.h:81:
HostContentSettingsMap* map_;
Same as in content_settings_policy_provider.

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map.cc:71:
HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
Since we do not store the profile anywhere. Should we rather pass in the
specific parameters?

HostContentSettingsMap::HostContentSettingsMap(PrefService* prefs, bool otr)

[Bernhard Bauer, 2011-07-08 17:05:27]

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_policy_provider.h (right):

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_policy_provider.h:138:
HostContentSettingsMap* map_;
On 2011/07/07 15:23:18, markusheintz_ wrote:
> Maybe you should use a better name here.
> e.g. host_content_settings_map. 
> 
> This way it is easier in the code to see which 'map' is used.

Done.

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_pref_provider.h (right):

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_pref_provider.h:81:
HostContentSettingsMap* map_;
On 2011/07/07 15:23:18, markusheintz_ wrote:
> Same as in content_settings_policy_provider.

Done.

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/7218073/diff/19040/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map.cc:71:
HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
On 2011/07/07 15:23:18, markusheintz_ wrote:
> Since we do not store the profile anywhere. Should we rather pass in the
> specific parameters?
> 
> HostContentSettingsMap::HostContentSettingsMap(PrefService* prefs, bool otr)

Done.

[commit-bot: I haz the power, 2011-07-08 17:05:44]

Can't apply patch for file tools/valgrind/memcheck/suppressions.txt.
While running patch -p1 --forward --force;
patching file tools/valgrind/memcheck/suppressions.txt
Hunk #2 FAILED at 4786.
1 out of 2 hunks FAILED -- saving rejects to file
tools/valgrind/memcheck/suppressions.txt.rej

[Bernhard Bauer, 2011-07-08 18:32:00]

Miranda, can you look at the Profile changes?
Evan, can you look at the Value changes?

Thanks!

[Miranda Callahan, 2011-07-09 03:21:10]

On 2011/07/08 18:32:00, Bernhard Bauer wrote:
> Miranda, can you look at the Profile changes?
> Evan, can you look at the Value changes?
> 
> Thanks!

Profile changes LGTM.

[markusheintz_, 2011-07-11 08:01:08]

LGTM

[Evan Martin, 2011-07-11 21:04:00]

base LGTM