Re-enable Images permission for all sites.

Re-enable Images permission for all sites.

For a brief period, it was possible to disable images for all websites
in Site Settings. This was an accident, and the switch was soon removed.
However, some M44 Beta users may have disabled images, and would now be
stuck in a state where images are permanently disable. This puts those
users back in a state where images are enabled.

For reference: the Images permission was added in
https://codereview.chromium.org/1068223002 and removed in
https://codereview.chromium.org/1221833002

BUG=505844
Committed: https://crrev.com/9dace830a490413c72a4dc42997f3e3000974626
Cr-Commit-Position: refs/heads/master@{#339576}

[newt (away), 2015-07-17 21:01:11]

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

[newt (away), 2015-07-17 21:01:12]

bauerb: PTAL

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
(right):

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java:141:
if (!isContentSettingManaged(ContentSettingsType.CONTENT_SETTINGS_TYPE_IMAGES))
{
bauerb: Do I need to check isContentSettingManaged(), or is it OK to call
setContentSettingEnabled() unconditionally? (setContentSettingEnabled() just
calls through to HostContentSettingsMap::SetDefaultContentSetting())

[Bernhard Bauer, 2015-07-20 09:58:31]

lgtm

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
(right):

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java:139:
// Settings. Now that this option has since been removed, ensure that users are
stuck with
Nit: "are _not_ stuck"? 😃

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java:141:
if (!isContentSettingManaged(ContentSettingsType.CONTENT_SETTINGS_TYPE_IMAGES))
{
On 2015/07/17 21:01:12, newt wrote:
> bauerb: Do I need to check isContentSettingManaged(), or is it OK to call
> setContentSettingEnabled() unconditionally? (setContentSettingEnabled() just
> calls through to HostContentSettingsMap::SetDefaultContentSetting())

The isContentSettingManaged() check is just to ensure we don't accidentally
override policy? If so, then it is indeed not necessary.

[newt (away), 2015-07-20 20:38:12]

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
(right):

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java:141:
if (!isContentSettingManaged(ContentSettingsType.CONTENT_SETTINGS_TYPE_IMAGES))
{
On 2015/07/20 09:58:31, Bernhard Bauer wrote:
> On 2015/07/17 21:01:12, newt wrote:
> > bauerb: Do I need to check isContentSettingManaged(), or is it OK to call
> > setContentSettingEnabled() unconditionally? (setContentSettingEnabled() just
> > calls through to HostContentSettingsMap::SetDefaultContentSetting())
> 
> The isContentSettingManaged() check is just to ensure we don't accidentally
> override policy? If so, then it is indeed not necessary.

Right. I don't want to override policy, or cause a DCHECK/CHECK when I call
setContentSettingEnabled().

I'm removing the isContentSettingManaged() check per your advice. So, the policy
will happily override whatever value I pass to setContentSettingEnabled()?

[Bernhard Bauer, 2015-07-20 21:09:37]

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
(right):

https://codereview.chromium.org/1240153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java:141:
if (!isContentSettingManaged(ContentSettingsType.CONTENT_SETTINGS_TYPE_IMAGES))
{
On 2015/07/20 20:38:12, newt wrote:
> On 2015/07/20 09:58:31, Bernhard Bauer wrote:
> > On 2015/07/17 21:01:12, newt wrote:
> > > bauerb: Do I need to check isContentSettingManaged(), or is it OK to call
> > > setContentSettingEnabled() unconditionally? (setContentSettingEnabled()
just
> > > calls through to HostContentSettingsMap::SetDefaultContentSetting())
> > 
> > The isContentSettingManaged() check is just to ensure we don't accidentally
> > override policy? If so, then it is indeed not necessary.
> 
> Right. I don't want to override policy, or cause a DCHECK/CHECK when I call
> setContentSettingEnabled().
> 
> I'm removing the isContentSettingManaged() check per your advice. So, the
policy
> will happily override whatever value I pass to setContentSettingEnabled()?

Exactly. The HostContentSettingsMap will apply all the policy providers by
precedence, and the policy one has the highest precedence, so whatever it says
goes.

[newt (away), 2015-07-20 23:53:18]

The CQ bit was checked by newt@chromium.org

[newt (away), 2015-07-20 23:53:18]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1240153002/#ps20001
(title: "bauerb's comments")

[newt (away), 2015-07-20 23:53:50]

On 2015/07/20 21:09:37, Bernhard Bauer wrote:
> Exactly. The HostContentSettingsMap will apply all the policy providers by
> precedence, and the policy one has the highest precedence, so whatever it says
> goes.

Great. Thanks for the review.

[commit-bot: I haz the power, 2015-07-20 23:54:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1240153002/20001

[commit-bot: I haz the power, 2015-07-21 00:34:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-21 00:36:09]

Patchset 2 (id:??) landed as
https://crrev.com/9dace830a490413c72a4dc42997f3e3000974626
Cr-Commit-Position: refs/heads/master@{#339576}