Replace HostContentSettingsMap::AddExceptionForURL with SetNarrowestWebsiteSetting

Replace HostContentSettingsMap::AddExceptionForURL with SetNarrowestWebsiteSetting

Both SetNarrowestWebsiteSetting and AddExceptionForURL are used for the same
purpose although they do slightly different things. They get called when a user
changes a content setting via a content settings page action icon or via the
origin info bubble. The aim is to ensure that the user's setting gets changed
even if there is an existing setting which takes precedence over the default
scoping of the setting they are changing. AddExceptionForURL handles this by
deleting the most narrow setting and setting the new setting.
SetNarrowestWebsiteSetting achieves this by changing the most narrow setting.
The behavior should be consistent for these two UI actions anyway and
SetNarrowestWebsiteSetting seemed to be the most sensible behavior because it
won't delete any exceptions the user may have created.

Note that this function will not be needed at all when all settings are scoped
to origin scope by default. There is no pattern that is more narrow than origin
scope so when that is the case we can call SetContentSetting directly.

BUG=551747
Committed: https://crrev.com/fbaaaaac87cc7a4dc64062d61f2f0c4a123a4c96
Cr-Commit-Position: refs/heads/master@{#358755}

[raymes, 2015-11-05 05:48:14]

raymes@chromium.org changed reviewers:
+ bauerb@chromium.org, lshang@chromium.org

[raymes, 2015-11-05 06:06:45]

Actually, please don't review this yet, I realised there is a different approach
I should take.

[raymes, 2015-11-05 06:42:11]

Description was changed from

==========
Remove HostContentSettingsMap::SetNarrowestWebsiteSetting

This CL removes the function
HostContentSettingsMap::SetNarrowestWebsiteSetting. The function only has one
callsite, so it is moved to the file where it is called. There are 2 reasons
for this:
1) To simplify the HostContentSettingsMap API
2) To prepare the code for making the scoping type a property of content
settings

BUG=551747
==========

to

==========
Replace HostContentSettingsMap::AddExceptionForURL with
SetNarrowestWebsiteSetting

Both SetNarrowestWebsiteSetting and AddExceptionForURL are used for the same
purpose although they do slightly different things. They get called when a user
changes a content setting via a content settings page action icon or via the
origin info bubble. The aim is to ensure that the user's setting gets changed
even if there is an existing setting which takes precedence over the default
scoping of the setting they are changing. AddExceptionForURL handles this by
deleting the most narrow setting and setting the new setting. 
SetNarrowestWebsiteSetting achieves this by changing the most narrow setting.
The behavior should be consistent for these two UI actions anyway and 
SetNarrowestWebsiteSetting seemed to be the most sensible behavior because it
won't delete any exceptions the user may have created.

Note that this function will not be needed at all when all settings are scoped
to origin scope by default. There is no pattern that is more narrow than origin
scope so when that is the case we can call SetContentSetting directly.

BUG=551747
==========

[raymes, 2015-11-05 06:42:28]

I've updated the CL and description, PTAL :)

https://codereview.chromium.org/1417023007/diff/20001/components/content_sett...
File components/content_settings/core/browser/host_content_settings_map.h
(right):

https://codereview.chromium.org/1417023007/diff/20001/components/content_sett...
components/content_settings/core/browser/host_content_settings_map.h:160: void
SetNarrowestContentSetting(const GURL& primary_url,
Do you think I should move this function out into a util function that is just
used by the UI (all of the use-cases are UI related)?

[Bernhard Bauer, 2015-11-05 11:28:10]

lgtm

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
File components/content_settings/core/browser/host_content_settings_map.h
(right):

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
components/content_settings/core/browser/host_content_settings_map.h:157: // are
scoped to origin scope. There is no scope more narrow than origin
Just FTR: If that should happen in the future, I predict you will get complaints
because people will have to change settings for every. single. origin. one by
one instead of applying a subdomain wildcard or something. I'll shut up now
though ☺

[lshang, 2015-11-05 23:14:21]

lgtm

[raymes, 2015-11-08 23:44:44]

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
File components/content_settings/core/browser/host_content_settings_map.h
(right):

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
components/content_settings/core/browser/host_content_settings_map.h:157: // are
scoped to origin scope. There is no scope more narrow than origin
On 2015/11/05 11:28:10, Bernhard Bauer wrote:
> Just FTR: If that should happen in the future, I predict you will get
complaints
> because people will have to change settings for every. single. origin. one by
> one instead of applying a subdomain wildcard or something. I'll shut up now
> though ☺

Thanks Bernhard - this is a valid concern :) I wouldn't want to do something
that causes users pain. Let's experiment - if it turns out this is a problem we
can keep the current behavior or figure out something else :)

[raymes, 2015-11-08 23:47:34]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2015-11-08 23:47:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1417023007/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1417023007/40001

[commit-bot: I haz the power, 2015-11-08 23:55:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-08 23:55:17]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[raymes, 2015-11-08 23:57:48]

raymes@chromium.org changed reviewers:
+ msw@chromium.org

[raymes, 2015-11-08 23:57:48]

+msw for chrome/browser/ui/* OWNERS

[msw, 2015-11-09 18:41:56]

https://codereview.chromium.org/1417023007/diff/40001/chrome/browser/ui/conte...
File chrome/browser/ui/content_settings/content_setting_bubble_model.cc (right):

https://codereview.chromium.org/1417023007/diff/40001/chrome/browser/ui/conte...
chrome/browser/ui/content_settings/content_setting_bubble_model.cc:417: void
ContentSettingSingleRadioGroup::AddException(ContentSetting setting) {
nit: rename AddException to match SetNarrowestContentSetting?

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
File components/content_settings/core/browser/host_content_settings_map.h
(right):

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
components/content_settings/core/browser/host_content_settings_map.h:156: //
given permission type. TODO(raymes): Remove this once all content settings
nit: 'given content type'?

[raymes, 2015-11-09 23:29:02]

https://codereview.chromium.org/1417023007/diff/40001/chrome/browser/ui/conte...
File chrome/browser/ui/content_settings/content_setting_bubble_model.cc (right):

https://codereview.chromium.org/1417023007/diff/40001/chrome/browser/ui/conte...
chrome/browser/ui/content_settings/content_setting_bubble_model.cc:417: void
ContentSettingSingleRadioGroup::AddException(ContentSetting setting) {
On 2015/11/09 18:41:56, msw wrote:
> nit: rename AddException to match SetNarrowestContentSetting?

Done.

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
File components/content_settings/core/browser/host_content_settings_map.h
(right):

https://codereview.chromium.org/1417023007/diff/40001/components/content_sett...
components/content_settings/core/browser/host_content_settings_map.h:156: //
given permission type. TODO(raymes): Remove this once all content settings
On 2015/11/09 18:41:56, msw wrote:
> nit: 'given content type'?

Done.

[msw, 2015-11-09 23:39:09]

lgtm

[raymes, 2015-11-10 00:44:16]

The CQ bit was checked by raymes@chromium.org

[raymes, 2015-11-10 00:44:17]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
lshang@chromium.org
Link to the patchset: https://codereview.chromium.org/1417023007/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-11-10 00:49:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1417023007/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1417023007/60001

[commit-bot: I haz the power, 2015-11-10 02:20:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-10 02:21:52]

Patchset 4 (id:??) landed as
https://crrev.com/fbaaaaac87cc7a4dc64062d61f2f0c4a123a4c96
Cr-Commit-Position: refs/heads/master@{#358755}