Unregister Images, Plugins and Mouselock content settings on android

Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG=604632
TBR= msramek@chromium.org

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

[lshang, 2016-06-06 06:45:14]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

BUG=
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

BUG=604632
==========

[lshang, 2016-06-06 06:47:50]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

BUG=604632
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

BUG=604632
==========

[lshang, 2016-06-06 06:48:38]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

BUG=604632
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

This is a follow up CL of https://codereview.chromium.org/1991623005/. 

BUG=604632
==========

[lshang, 2016-06-06 07:00:01]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

This is a follow up CL of https://codereview.chromium.org/1991623005/. 

BUG=604632
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

Several places in shared code are if-defed out when they try to get access to 
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images 
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/. 

BUG=604632
==========

[lshang, 2016-06-06 23:48:44]

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

[lshang, 2016-06-06 23:48:45]

PTAL thanks!

[raymes, 2016-06-07 01:40:44]

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
(right):

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_pref_provider_unittest.cc:295:
#if !defined(OS_ANDROID)
Should we use 

#if defined(ENABLE_PLUGINS)

instead (and for all the new ifdefs below too)?

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/host_content_settings_map_unittest.cc
(right):

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map_unittest.cc:196:
#endif
Maybe we can just remove this? The only coverage it gives is that the default
value for images is ALLOW, but that's not too useful.

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map_unittest.cc:228:
#endif
Same with this one.

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map_unittest.cc:482:
#endif
nit: likewise we can probably remove this chunk - we should get coverage from
the CONTENT_SETTINGS_TYPE_JAVASCRIPT type

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/ui/website_s...
File chrome/browser/ui/website_settings/website_settings_unittest.cc (right):

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings_unittest.cc:233: #if
defined(OS_ANDROID)
Should we have #if !defined(ENABLE_PLUGINS) ?

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(left):

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
components/content_settings/core/browser/content_settings_pref_provider.cc:156: 
nit: these changes aren't needed in this CL

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
File
components/content_settings/core/browser/content_settings_registry_unittest.cc
(right):

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
components/content_settings/core/browser/content_settings_registry_unittest.cc:41:
#if defined(OS_IOS) | defined(OS_ANDROID)
|| instead of |

[lshang, 2016-06-07 05:01:24]

Patchset #2 (id:20001) has been deleted

[lshang, 2016-06-07 05:45:50]

Patchset #2 (id:40001) has been deleted

[lshang, 2016-06-07 06:40:00]

Thanks Raymes!

I add wildcard ALLOW settings for images in
ContentSettingsUtils::GetRendererContentSettingRules(), if it is on Android, so
that we don't need to handle this case in chrome/renderer.

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
(right):

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/content_settings_pref_provider_unittest.cc:295:
#if !defined(OS_ANDROID)
On 2016/06/07 01:40:44, raymes wrote:
> Should we use 
> 
> #if defined(ENABLE_PLUGINS)
> 
> instead (and for all the new ifdefs below too)?

Done.

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/host_content_settings_map_unittest.cc
(right):

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map_unittest.cc:196:
#endif
On 2016/06/07 01:40:44, raymes wrote:
> Maybe we can just remove this? The only coverage it gives is that the default
> value for images is ALLOW, but that's not too useful.

Done.

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map_unittest.cc:228:
#endif
On 2016/06/07 01:40:44, raymes wrote:
> Same with this one.

Done.

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/host_content_settings_map_unittest.cc:482:
#endif
On 2016/06/07 01:40:44, raymes wrote:
> nit: likewise we can probably remove this chunk - we should get coverage from
> the CONTENT_SETTINGS_TYPE_JAVASCRIPT type

Done.

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/ui/website_s...
File chrome/browser/ui/website_settings/website_settings_unittest.cc (right):

https://codereview.chromium.org/2039803002/diff/1/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings_unittest.cc:233: #if
defined(OS_ANDROID)
On 2016/06/07 01:40:44, raymes wrote:
> Should we have #if !defined(ENABLE_PLUGINS) ?

Done.

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(left):

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
components/content_settings/core/browser/content_settings_pref_provider.cc:156: 
On 2016/06/07 01:40:44, raymes wrote:
> nit: these changes aren't needed in this CL

Done.

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
File
components/content_settings/core/browser/content_settings_registry_unittest.cc
(right):

https://codereview.chromium.org/2039803002/diff/1/components/content_settings...
components/content_settings/core/browser/content_settings_registry_unittest.cc:41:
#if defined(OS_IOS) | defined(OS_ANDROID)
On 2016/06/07 01:40:44, raymes wrote:
> || instead of |

Done.

[raymes, 2016-06-08 02:55:33]

lgtm

https://codereview.chromium.org/2039803002/diff/80001/components/content_sett...
File components/content_settings/core/browser/content_settings_utils.cc (right):

https://codereview.chromium.org/2039803002/diff/80001/components/content_sett...
components/content_settings/core/browser/content_settings_utils.cc:143: //
wildcard.
nit: is added for wildcard->is added for all origins.

[lshang, 2016-06-09 00:43:19]

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

[lshang, 2016-06-09 00:43:20]

Bernhard, could you give OWNER review for:

chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
chrome/browser/android/preferences/pref_service_bridge.cc
chrome/browser/browsing_data/browsing_data_remover_unittest.cc

Thanks!

https://codereview.chromium.org/2039803002/diff/80001/components/content_sett...
File components/content_settings/core/browser/content_settings_utils.cc (right):

https://codereview.chromium.org/2039803002/diff/80001/components/content_sett...
components/content_settings/core/browser/content_settings_utils.cc:143: //
wildcard.
On 2016/06/08 02:55:33, raymes wrote:
> nit: is added for wildcard->is added for all origins.

Done.

[Bernhard Bauer, 2016-06-09 08:55:45]

LGTM

Out of curiosity: Do we not support mouselock at all on Android? It's very much
a special case, but in principle Android can support pointing input devices 😃

https://codereview.chromium.org/2039803002/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
(left):

https://codereview.chromium.org/2039803002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java:160:
// Steps 2,3 intentionally skipped.
Keep this comment and include step 4?

[lshang, 2016-06-14 00:55:10]

On 2016/06/09 08:55:45, Bernhard Bauer wrote:
> LGTM
> 
> Out of curiosity: Do we not support mouselock at all on Android? It's very
much
> a special case, but in principle Android can support pointing input devices 😃
>

Yeah, mouselock settings are already hidden from ui on Android, and we don't
support mouselock at all on Android despite that Android can support pointing
input devices. 

 
>
https://codereview.chromium.org/2039803002/diff/100001/chrome/android/java/sr...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
> (left):
> 
>
https://codereview.chromium.org/2039803002/diff/100001/chrome/android/java/sr...
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java:160:
> // Steps 2,3 intentionally skipped.
> Keep this comment and include step 4?

Done.

[lshang, 2016-06-14 01:20:05]

The CQ bit was checked by lshang@chromium.org

[lshang, 2016-06-14 01:20:06]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org,
bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2039803002/#ps140001
(title: "address review comments, minor change")

[commit-bot: I haz the power, 2016-06-14 01:20:33]

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

[commit-bot: I haz the power, 2016-06-14 01:25:21]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

Several places in shared code are if-defed out when they try to get access to 
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images 
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/. 

BUG=604632
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

Several places in shared code are if-defed out when they try to get access to 
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images 
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/. 

BUG=604632
==========

[commit-bot: I haz the power, 2016-06-14 01:25:22]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-06-14 01:25:33]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-14 01:27:02]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so 
unregister them in ContentSettingsRegistry and handle places in shared code 
where get access to these types.

Several places in shared code are if-defed out when they try to get access to 
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images 
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/. 

BUG=604632
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG=604632

Committed: https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9
Cr-Commit-Position: refs/heads/master@{#399634}
==========

[commit-bot: I haz the power, 2016-06-14 01:27:04]

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

[lshang, 2016-06-14 01:48:38]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG=604632

Committed: https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9
Cr-Commit-Position: refs/heads/master@{#399634}
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG=604632
TBR=msramek@chromium.org

Committed: https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9
Cr-Commit-Position: refs/heads/master@{#399634}
==========

[lshang, 2016-06-14 01:49:28]

Description was changed from

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG=604632
TBR=msramek@chromium.org

Committed: https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9
Cr-Commit-Position: refs/heads/master@{#399634}
==========

to

==========
Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG=604632
TBR= msramek@chromium.org

Committed: https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9
Cr-Commit-Position: refs/heads/master@{#399634}
==========