Cleanup content settings code

- Remove unused includes
- Add missing includes
- Inline function.


BUG=63656
TEST=HostContentSettingsMapTest*, 
     ContentSettingsProviderTest*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113610

[markusheintz_, 2011-12-06 09:30:12]

@marja: Please review my CL. Thanks

@bauerb: I didn't rename CONTENT_SETTING_DEFAULT yet due to the lack of a good
name. CONTENT_SETTING_NONE was my favorit, but using CONTENT_SETTING_NONE to
reset a setting to the hard coded default is wired. So I think we should keep it
for now.

[Bernhard Bauer, 2011-12-06 09:35:16]

On 2011/12/06 09:30:12, markusheintz_ wrote:
> @marja: Please review my CL. Thanks
> 
> @bauerb: I didn't rename CONTENT_SETTING_DEFAULT yet due to the lack of a good
> name. CONTENT_SETTING_NONE was my favorit, but using CONTENT_SETTING_NONE to
> reset a setting to the hard coded default is wired. So I think we should keep
it
> for now.

Is it? Note that CONTENT_SETTING_NONE means no setting is defined *at this
level*, not that no setting is defined at all.

[marja, 2011-12-06 09:38:20]

On 2011/12/06 09:35:16, Bernhard Bauer wrote:
> Is it? Note that CONTENT_SETTING_NONE means no setting is defined *at this
> level*, not that no setting is defined at all.

I also think NONE is clearer than DEFAULT (DEFAULT is a bit overloaded, since
the default setting can also be ALLOW, BLOCK etc.)

[marja, 2011-12-06 09:47:12]

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:22: #include
"chrome/browser/content_settings/content_settings_utils.h"
I don't think you should remove this include, this file uses
content_settings::GetContentSettingValueAndPatterns.

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:446: base::Value*
value = content_settings::GetContentSettingValueAndPatterns(
(Here.)

[markusheintz_, 2011-12-06 09:48:38]

On 2011/12/06 09:38:20, marja wrote:
> On 2011/12/06 09:35:16, Bernhard Bauer wrote:
> > Is it? Note that CONTENT_SETTING_NONE means no setting is defined *at this
> > level*, not that no setting is defined at all.

Right, so if you call SetContentSetting on the host_content_settings_map level,
then is is strange if you set a setting to NONE but get a DEFAULT. Otherwise I'm
more than fine with the renaming. But i think it is strange from the perspective
of a client using the HCSM.

> 
> I also think NONE is clearer than DEFAULT (DEFAULT is a bit overloaded, since
> the default setting can also be ALLOW, BLOCK etc.)

[marja, 2011-12-06 09:51:38]

On 2011/12/06 09:48:38, markusheintz_ wrote:
> Right, so if you call SetContentSetting on the host_content_settings_map
level,
> then is is strange if you set a setting to NONE but get a DEFAULT.

Hmm? I don't understand. If DEFAULT is renamed to NONE, it wouldn't be possible
to set NONE and get DEFAULT?

[markusheintz_, 2011-12-06 15:36:54]

On 2011/12/06 09:51:38, marja wrote:
> On 2011/12/06 09:48:38, markusheintz_ wrote:
> > Right, so if you call SetContentSetting on the host_content_settings_map
> level,
> > then is is strange if you set a setting to NONE but get a DEFAULT.
> 
> Hmm? I don't understand. If DEFAULT is renamed to NONE, it wouldn't be
possible
> to set NONE and get DEFAULT?

No

To reset a content setting to it's default value (= delete the user defined
setting) you must call SetContentSetting(type, CONTENT_SETTING_NONE).

I find it wired if you set a setting to none, but get CONTENT_SETTING_ALLOW
after calling GetContentSetting(type).

I know this is not much different from what we have now except the you replace
CONTENT_SETTING_NONE with CONTENT_SETTING_DEFAULT.

Maybe we can handle this in a separate CL so that I can proceed with this one.

[marja, 2011-12-06 15:56:33]

ok.

[marja, 2011-12-06 16:02:14]

lgtm modulo the #include removal comment.

[markusheintz_, 2011-12-06 16:29:59]

Thanks for the review.

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:22: #include
"chrome/browser/content_settings/content_settings_utils.h"
On 2011/12/06 09:47:12, marja wrote:
> I don't think you should remove this include, this file uses
> content_settings::GetContentSettingValueAndPatterns.

Done.

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8804016/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:446: base::Value*
value = content_settings::GetContentSettingValueAndPatterns(
On 2011/12/06 09:47:12, marja wrote:
> (Here.)

Done.

[commit-bot: I haz the power, 2011-12-08 14:36:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/markusheintz@chromium.org/8804016/9001

[commit-bot: I haz the power, 2011-12-08 16:27:12]

Change committed as 113610