[Extensions] Fix platforms for privacy.websites.protectedContentEnabled

[Extensions] Fix platforms for privacy.websites.protectedContentEnabled 

privacy.websites.protectedContentEnabled is enabled per-platform, and
the platform list includes "cros" and "cros touch". But the
determination of the platforms in binding.js includes "chromeos" and
"chromeos touch". As a result, the protectedContentEnabled feature
is never actually available on a CrOS platform (despite being documented
as such).

It appears that, since it's addition in
https://crrev.com/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86, this has
been unavailable on ChromeOS: See
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86/chrome/renderer/resources/extensions/schema_generated_bindings.js#318
and
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86/chrome/common/extensions/api/privacy.json#88
where the patch was added.

Update the platforms listed to be those that are used in the features
system.

Also update the browser test to correctly check the
privacy.websites.protectedContentEnabled feature on all platforms, as
well as expanding it to test setting preferences.

BUG=746064
Review-Url: https://codereview.chromium.org/2966613002
Cr-Commit-Position: refs/heads/master@{#487931}
Committed: https://chromium.googlesource.com/chromium/src/+/392484cfd0bb44ee1606071fbe39c23b5934798f

[Devlin, 2017-06-29 21:35:30]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-29 21:36:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 22:48:23]

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

[commit-bot: I haz the power, 2017-06-29 22:48:24]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-06-30 00:56:35]

Description was changed from

==========
fix stuff
BUG=
==========

to

==========
[Extensions] Exclude privacy.websites.protectedContentEnabled on CrOS

privacy.websites.protectedContentEnabled is enabled per-platform, and
the platform list includes "cros" and "cros touch". But the
determination of the platforms in binding.js includes "chromeos" and
"chromeos touch". As a result, the protectedContentEnabled feature
is never actually available on a CrOS platform (despite being documented
as such).

It appears that, since it's addition in
https://crrev.com/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86, this has
been unavailable on ChromeOS: See
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
and
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
where the patch was added.

Remove the "cros" and "cros touch" entries from the platforms limitation
of privacy.websites.protectedContentEnabled, and update the
documentation. In practice, this should have no effect, but will at
least make the schema and the documentation match reality.

In addition, remove the "chromeos touch" platform from the list of
possibilities, since it's never used as a differentiator. This makes the
platforms used in the schema match those in the feature files.

BUG=TBD
==========

[Devlin, 2017-06-30 00:58:03]

rdevlin.cronin@chromium.org changed reviewers:
+ lazyboy@chromium.org, mkwst@chromium.org

[Devlin, 2017-06-30 00:58:06]

Hey Mike, it looks like you added this way back in
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...,
but as best I can tell, it never actually would have worked on ChromeOS.  Can
you take a look and offer any insight?

lazyboy@, can you take a look as extensions owner?

jbroman@, FYI.

[Mike West, 2017-06-30 08:59:04]

mkwst@chromium.org changed reviewers:
+ msramek@chromium.org

[Mike West, 2017-06-30 08:59:05]

It should have worked cross platform. :/

+msramek@ who will have opinions about privacy implications.

[msramek, 2017-06-30 09:38:30]

I'm not sure what I'm looking at.

It's certainly better to update the documentation to match the current
implementation, but wouldn't it be better to simply add "chromeos" (as in, this
time for real)? Same way as Windows was once added as a new platform for this
API.

I don't see any good reason to artificially restrict this API to a subset of
platform where it is useful.

[Devlin, 2017-07-05 15:49:09]

On 2017/06/30 09:38:30, msramek (slow) wrote:
> I'm not sure what I'm looking at.
> 
> It's certainly better to update the documentation to match the current
> implementation, but wouldn't it be better to simply add "chromeos" (as in,
this
> time for real)? Same way as Windows was once added as a new platform for this
> API.
> 
> I don't see any good reason to artificially restrict this API to a subset of
> platform where it is useful.

I'm happy to change this either way.  My default was to remove the CrOS
platforms because I wasn't sure it was desired on CrOS, and it is clearly not
being used. :)  But if we would prefer enabling it on CrOS, that's fine with me
too.

[Devlin, 2017-07-14 01:15:22]

On 2017/07/05 15:49:09, Devlin wrote:
> On 2017/06/30 09:38:30, msramek (slow) wrote:
> > I'm not sure what I'm looking at.
> > 
> > It's certainly better to update the documentation to match the current
> > implementation, but wouldn't it be better to simply add "chromeos" (as in,
> this
> > time for real)? Same way as Windows was once added as a new platform for
this
> > API.
> > 
> > I don't see any good reason to artificially restrict this API to a subset of
> > platform where it is useful.
> 
> I'm happy to change this either way.  My default was to remove the CrOS
> platforms because I wasn't sure it was desired on CrOS, and it is clearly not
> being used. :)  But if we would prefer enabling it on CrOS, that's fine with
me
> too.

Friendly ping here - do either of you have a preference between
a) Making the docs match reality; and
b) Making reality match the docs?

(My slight preference is for a), but just because it seems less risky)

I'd like to get us into a cohesive state. :)

[msramek, 2017-07-14 09:06:29]

My preference is that reality matches docs. That's certainly better long term.

In short term, this might mean that for some users, an extension will suddenly
take effect where it hasn't before. But presumably if the user installed an
extension that promised to manipulate this setting, and gave it the
corresponding permission, it shouldn't be that surprising (-> it was surprising
before). I'm not sure there's any migration path we could take.

[Devlin, 2017-07-17 21:34:43]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-17 21:34:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-17 22:41:40]

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

[commit-bot: I haz the power, 2017-07-17 22:41:42]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2017-07-18 00:56:00]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-18 00:56:09]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-18 02:43:30]

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

[commit-bot: I haz the power, 2017-07-18 02:43:31]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2017-07-18 20:02:33]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-18 20:03:19]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2017-07-18 22:32:00]

Description was changed from

==========
[Extensions] Exclude privacy.websites.protectedContentEnabled on CrOS

privacy.websites.protectedContentEnabled is enabled per-platform, and
the platform list includes "cros" and "cros touch". But the
determination of the platforms in binding.js includes "chromeos" and
"chromeos touch". As a result, the protectedContentEnabled feature
is never actually available on a CrOS platform (despite being documented
as such).

It appears that, since it's addition in
https://crrev.com/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86, this has
been unavailable on ChromeOS: See
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
and
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
where the patch was added.

Remove the "cros" and "cros touch" entries from the platforms limitation
of privacy.websites.protectedContentEnabled, and update the
documentation. In practice, this should have no effect, but will at
least make the schema and the documentation match reality.

In addition, remove the "chromeos touch" platform from the list of
possibilities, since it's never used as a differentiator. This makes the
platforms used in the schema match those in the feature files.

BUG=TBD
==========

to

==========
[Extensions] Fix platforms for privacy.websites.protectedContentEnabled 

privacy.websites.protectedContentEnabled is enabled per-platform, and
the platform list includes "cros" and "cros touch". But the
determination of the platforms in binding.js includes "chromeos" and
"chromeos touch". As a result, the protectedContentEnabled feature
is never actually available on a CrOS platform (despite being documented
as such).

It appears that, since it's addition in
https://crrev.com/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86, this has
been unavailable on ChromeOS: See
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
and
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
where the patch was added.

Update the platforms listed to be those that are used in the features
system.

Also update the browser test to correctly check the
privacy.websites.protectedContentEnabled feature on all platforms, as
well as expanding it to test setting preferences.

BUG=746064
==========

[Devlin, 2017-07-18 23:02:37]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-18 23:02:46]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2017-07-19 00:11:21]

On 2017/07/14 09:06:29, msramek (slow) wrote:
> My preference is that reality matches docs. That's certainly better long term.
> 
> In short term, this might mean that for some users, an extension will suddenly
> take effect where it hasn't before. But presumably if the user installed an
> extension that promised to manipulate this setting, and gave it the
> corresponding permission, it shouldn't be that surprising (-> it was
surprising
> before). I'm not sure there's any migration path we could take.

Done.  Please take another look.

[commit-bot: I haz the power, 2017-07-19 00:17:31]

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

[commit-bot: I haz the power, 2017-07-19 00:17:33]

Dry run: This issue passed the CQ dry run.

[msramek, 2017-07-19 00:44:07]

LGTM, thanks!

https://codereview.chromium.org/2966613002/diff/80001/chrome/test/data/extens...
File chrome/test/data/extensions/api_test/preference/standard/test.js (right):

https://codereview.chromium.org/2966613002/diff/80001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/preference/standard/test.js:54:
navigator.userAgent.indexOf('CrOS') == -1) {
nit: String.prototype.includes()

https://codereview.chromium.org/2966613002/diff/80001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/preference/standard/test.js:69: }, '`' +
prefName + '` is expected to be default.');
optional: '...default, which is ' + defaultValue

[Devlin, 2017-07-19 16:06:08]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-19 16:06:19]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2017-07-19 17:43:23]

https://codereview.chromium.org/2966613002/diff/80001/chrome/test/data/extens...
File chrome/test/data/extensions/api_test/preference/standard/test.js (right):

https://codereview.chromium.org/2966613002/diff/80001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/preference/standard/test.js:54:
navigator.userAgent.indexOf('CrOS') == -1) {
On 2017/07/19 00:44:07, msramek (slow) wrote:
> nit: String.prototype.includes()

Done.

https://codereview.chromium.org/2966613002/diff/80001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/preference/standard/test.js:69: }, '`' +
prefName + '` is expected to be default.');
On 2017/07/19 00:44:07, msramek (slow) wrote:
> optional: '...default, which is ' + defaultValue

Done.

[Devlin, 2017-07-19 17:43:41]

The CQ bit was unchecked by rdevlin.cronin@chromium.org

[Devlin, 2017-07-19 17:43:44]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2017-07-19 17:43:45]

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

[commit-bot: I haz the power, 2017-07-19 17:43:58]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lazyboy, 2017-07-19 17:54:19]

lgtm

[commit-bot: I haz the power, 2017-07-19 18:46:28]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1500486224737180,
"parent_rev": "1a346ab70b8ef1a08bf4b6863e12d38f24592040", "commit_rev":
"a53669d6cce2e536ae2e98f9df42d89d6c9bb902"}

[commit-bot: I haz the power, 2017-07-19 18:46:46]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1500486224737180,
"parent_rev": "765c9514c367fc1e60c51c74e69aaeedb350f53a", "commit_rev":
"392484cfd0bb44ee1606071fbe39c23b5934798f"}

[commit-bot: I haz the power, 2017-07-19 18:47:02]

Description was changed from

==========
[Extensions] Fix platforms for privacy.websites.protectedContentEnabled 

privacy.websites.protectedContentEnabled is enabled per-platform, and
the platform list includes "cros" and "cros touch". But the
determination of the platforms in binding.js includes "chromeos" and
"chromeos touch". As a result, the protectedContentEnabled feature
is never actually available on a CrOS platform (despite being documented
as such).

It appears that, since it's addition in
https://crrev.com/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86, this has
been unavailable on ChromeOS: See
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
and
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
where the patch was added.

Update the platforms listed to be those that are used in the features
system.

Also update the browser test to correctly check the
privacy.websites.protectedContentEnabled feature on all platforms, as
well as expanding it to test setting preferences.

BUG=746064
==========

to

==========
[Extensions] Fix platforms for privacy.websites.protectedContentEnabled 

privacy.websites.protectedContentEnabled is enabled per-platform, and
the platform list includes "cros" and "cros touch". But the
determination of the platforms in binding.js includes "chromeos" and
"chromeos touch". As a result, the protectedContentEnabled feature
is never actually available on a CrOS platform (despite being documented
as such).

It appears that, since it's addition in
https://crrev.com/cc3a5c8ca42579c0f9f710c18f51c566b4c75e86, this has
been unavailable on ChromeOS: See
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
and
https://chromium.googlesource.com/chromium/src/+/cc3a5c8ca42579c0f9f710c18f51...
where the patch was added.

Update the platforms listed to be those that are used in the features
system.

Also update the browser test to correctly check the
privacy.websites.protectedContentEnabled feature on all platforms, as
well as expanding it to test setting preferences.

BUG=746064

Review-Url: https://codereview.chromium.org/2966613002
Cr-Commit-Position: refs/heads/master@{#487931}
Committed:
https://chromium.googlesource.com/chromium/src/+/392484cfd0bb44ee1606071fbe39...
==========

[commit-bot: I haz the power, 2017-07-19 18:47:04]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/392484cfd0bb44ee1606071fbe39...