Expose the Website Settings page in the Content Setting page if it is enabled via flag.

Expose the Website Settings page in the Content Setting page if it is enabled via flag.

Committed: https://crrev.com/e92897b3992aaf64b9d4d12d22de18577e15ab47
Cr-Commit-Position: refs/heads/master@{#294923}

[Daniel Nishi, 2014-09-12 20:12:56]

Patchset #1 (id:1) has been deleted

[Daniel Nishi, 2014-09-12 20:15:26]

dhnishi@chromium.org changed reviewers:
+ bauerb@chromium.org

[Daniel Nishi, 2014-09-12 20:15:26]

bauerb: PTAL.

[Bernhard Bauer, 2014-09-15 09:13:49]

LGTM with some nits and a question:

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/host_content_settings_map.cc:207: NOTREACHED();
Why is this not the case anymore?

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/content_settings.html (right):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/content_settings.html:64: <button
class="website-settings-permission-button" contentType="cookies"
Break this line to make it fit into 80 columns (same below).

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/options.html (right):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/options.html:174: <div id
="overlay-container-3" class="overlay transparent" hidden>
Nit: no space before =

[Daniel Nishi, 2014-09-15 16:57:28]

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/host_content_settings_map.cc:207: NOTREACHED();
On 2014/09/15 09:13:49, Bernhard Bauer wrote:
> Why is this not the case anymore?

If we ask for a default content setting for a content setting whose actual
default is CONTENT_SETTING_DEFAULT from the DefaultProvider, it will fall
through and trigger the NOTREACHED().

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/content_settings.html (right):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/content_settings.html:64: <button
class="website-settings-permission-button" contentType="cookies"
On 2014/09/15 09:13:49, Bernhard Bauer wrote:
> Break this line to make it fit into 80 columns (same below).

Done.

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/options.html (right):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/options.html:174: <div id
="overlay-container-3" class="overlay transparent" hidden>
On 2014/09/15 09:13:49, Bernhard Bauer wrote:
> Nit: no space before =

Done.

[Daniel Nishi, 2014-09-15 16:57:31]

The CQ bit was checked by dhnishi@chromium.org

[Bernhard Bauer, 2014-09-15 17:05:38]

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/host_content_settings_map.cc:207: NOTREACHED();
On 2014/09/15 16:57:27, Daniel Nishi wrote:
> On 2014/09/15 09:13:49, Bernhard Bauer wrote:
> > Why is this not the case anymore?
> 
> If we ask for a default content setting for a content setting whose actual
> default is CONTENT_SETTING_DEFAULT from the DefaultProvider, it will fall
> through and trigger the NOTREACHED().

Yeah, but the point of the DefaultProvider is that it will always return a
(non-DEFAULT) setting... Why doesn't that happen?

https://codereview.chromium.org/566863005/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/content_settings.html (right):

https://codereview.chromium.org/566863005/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/content_settings.html:100: <button
class="website-settings-permission-button" contentType="images"
Here too please (and below).

[commit-bot: I haz the power, 2014-09-15 17:18:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/566863005/40001

[Daniel Nishi, 2014-09-15 17:20:59]

The CQ bit was unchecked by dhnishi@chromium.org

[Daniel Nishi, 2014-09-15 17:43:53]

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

https://codereview.chromium.org/566863005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/host_content_settings_map.cc:207: NOTREACHED();
On 2014/09/15 17:05:38, Bernhard Bauer wrote:
> On 2014/09/15 16:57:27, Daniel Nishi wrote:
> > On 2014/09/15 09:13:49, Bernhard Bauer wrote:
> > > Why is this not the case anymore?
> > 
> > If we ask for a default content setting for a content setting whose actual
> > default is CONTENT_SETTING_DEFAULT from the DefaultProvider, it will fall
> > through and trigger the NOTREACHED().
> 
> Yeah, but the point of the DefaultProvider is that it will always return a
> (non-DEFAULT) setting... Why doesn't that happen?

In some cases, the idea of a default setting doesn't make sense, like for
protocol handlers. In this case, the DefaultSettingsProvider returns the value
CONTENT_SETTING_DEFAULT.

https://codereview.chromium.org/566863005/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/content_settings.html (right):

https://codereview.chromium.org/566863005/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/content_settings.html:100: <button
class="website-settings-permission-button" contentType="images"
On 2014/09/15 17:05:38, Bernhard Bauer wrote:
> Here too please (and below).

Done.

[Daniel Nishi, 2014-09-15 21:39:37]

The CQ bit was checked by dhnishi@chromium.org

[commit-bot: I haz the power, 2014-09-15 22:03:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/566863005/80001

[commit-bot: I haz the power, 2014-09-15 23:39:08]

Committed patchset #4 (id:80001) as 6464150c686dbb13fb5c08d5363ba404e059f883

[commit-bot: I haz the power, 2014-09-15 23:48:27]

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